You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rb...@apache.org on 2002/04/05 19:50:37 UTC

cvs commit: httpd-2.0/modules/generators mod_autoindex.c

rbb         02/04/05 09:50:37

  Modified:    modules/generators mod_autoindex.c
  Log:
  This is a HACK!  The problem is that the fast_internal_redirect is
  removing the OLD_WRITE filter. Obviously that is wrong.  For right now,
  the fix is to hack around the problem and just make it work.  Long term,
  we need to find a real solution to this, but this gets autoindex working
  today.
  
  Revision  Changes    Path
  1.103     +25 -1     httpd-2.0/modules/generators/mod_autoindex.c
  
  Index: mod_autoindex.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/generators/mod_autoindex.c,v
  retrieving revision 1.102
  retrieving revision 1.103
  diff -u -r1.102 -r1.103
  --- mod_autoindex.c	20 Mar 2002 17:41:54 -0000	1.102
  +++ mod_autoindex.c	5 Apr 2002 17:50:37 -0000	1.103
  @@ -1000,6 +1000,7 @@
       apr_table_setn(hdrs, "Accept", "text/html, text/plain");
       apr_table_unset(hdrs, "Accept-Encoding");
   
  +
       if ((header_fname != NULL) && r->args) {
           header_fname = apr_pstrcat(r->pool, header_fname, "?", r->args, NULL);
       }
  @@ -1017,13 +1018,30 @@
           if (rr->content_type != NULL) {
               if (!strcasecmp(ap_field_noparam(r->pool, rr->content_type),
                               "text/html")) {
  -                /* Hope everything will work... */
  +                ap_filter_t *f;
  +               /* Hope everything will work... */
                   emit_amble = 0;
                   emit_H1 = 0;
   
                   if (! suppress_amble) {
                       emit_preamble(r, title);
                   }
  +                /* This is a hack, but I can't find any better way to do this.
  +                 * The problem is that we have already created the sub-request,
  +                 * but we just inserted the OLD_WRITE filter, and the 
  +                 * sub-request needs to pass its data through the OLD_WRITE
  +                 * filter, or things go horribly wrong (missing data, data in
  +                 * the wrong order, etc).  To fix it, if you create a 
  +                 * sub-request and then insert the OLD_WRITE filter before you
  +                 * run the request, you need to make sure that the sub-request
  +                 * data goes through the OLD_WRITE filter.  Just steal this 
  +                 * code.  The long-term solution is to remove the ap_r*
  +                 * functions.
  +                 */
  +                for (f=rr->output_filters; 
  +                     f->frec != ap_subreq_core_filter_handle; f = f->next);
  +                f->next = r->output_filters; 
  +
                   /*
                    * If there's a problem running the subrequest, display the
                    * preamble if we didn't do it before -- the header file
  @@ -1111,6 +1129,12 @@
           if (rr->content_type != NULL) {
               if (!strcasecmp(ap_field_noparam(r->pool, rr->content_type),
                               "text/html")) {
  +                ap_filter_t *f;
  +                for (f=rr->output_filters; 
  +                     f->frec != ap_subreq_core_filter_handle; f = f->next);
  +                f->next = r->output_filters; 
  +
  +
                   if (ap_run_sub_req(rr) == OK) {
                       /* worked... */
                       suppress_sig = 1;
  
  
  

RE: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Ryan Bloom <rb...@covalent.net>.
> > >         The problem is that the fast_internal_redirect is removing
the
> OLD_WRITE filter.
> >
> > I'm going to try it on my box without this patch, and with no
Multiviews
> (to get
> > rid of fast_internal_redirects for HEADER and README).  If that
works
> with HEAD
> > as well as it did in 2.0.32, great!
> 
> HEAD does work fine on my box without Multiviews, as you predicted.
But
> doing
> the Right Thing w.r.t. OLD_WRITE and fast_internal_redirect just
doesn't
> sound
> like rocket science.

I have a hack in there for now.  Feel free to look at how to solve it
for real.  I beat my head against it for a few hours this week.  I'll
look at it again after the 2.0.35 GA is released.

Ryan



Re: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Greg Ames <gr...@remulak.net>.
Greg Ames wrote:
> 
> rbb@apache.org wrote:

> >         The problem is that the fast_internal_redirect is removing the OLD_WRITE filter.
> 
> I'm going to try it on my box without this patch, and with no Multiviews (to get
> rid of fast_internal_redirects for HEADER and README).  If that works with HEAD
> as well as it did in 2.0.32, great!  

HEAD does work fine on my box without Multiviews, as you predicted.  But doing
the Right Thing w.r.t. OLD_WRITE and fast_internal_redirect just doesn't sound
like rocket science.

Greg

RE: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Ryan Bloom <rb...@covalent.net>.
> Ryan Bloom wrote:
> >
> > > rbb@apache.org wrote:
> > > >
> > > > rbb         02/04/05 09:50:37
> > > >
> > > >   Modified:    modules/generators mod_autoindex.c
> > > >   Log:
> > > >   This is a HACK!
> > >
> > > Why would it be difficult for the core to preserve OLD_WRITE in
the
> > subreq
> > > filter chain?  We knew how to do that in 2.0.32.  One would hope
we
> > get
> > > smarter
> > > as we get more experienced.
> >
> > Because in pre-2.0.32 code, fast_internal_redirects didn't remove
all of
> > the RESOURCE filters.  It does now, because that is the correct
thing to
> > do.  The problem is that there is one RESOURCE filters that we want
to
> > keep, OLD_WRITE.
> 
> OK, so maybe we proved that OLD_WRITE isn't an ordinary RESOURCE
filter.
> Calling it something else might do the trick.

The problem is that it MUST be a RESOURCE filter, or it will be put in
the wrong location in the filter stack.

Ryan



Re: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Greg Ames <gr...@remulak.net>.
Ryan Bloom wrote:
> 
> > rbb@apache.org wrote:
> > >
> > > rbb         02/04/05 09:50:37
> > >
> > >   Modified:    modules/generators mod_autoindex.c
> > >   Log:
> > >   This is a HACK!
> >
> > Why would it be difficult for the core to preserve OLD_WRITE in the
> subreq
> > filter chain?  We knew how to do that in 2.0.32.  One would hope we
> get
> > smarter
> > as we get more experienced.
> 
> Because in pre-2.0.32 code, fast_internal_redirects didn't remove all of
> the RESOURCE filters.  It does now, because that is the correct thing to
> do.  The problem is that there is one RESOURCE filters that we want to
> keep, OLD_WRITE. 

OK, so maybe we proved that OLD_WRITE isn't an ordinary RESOURCE filter. 
Calling it something else might do the trick.

Greg

RE: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Ryan Bloom <rb...@covalent.net>.
> rbb@apache.org wrote:
> >
> > rbb         02/04/05 09:50:37
> >
> >   Modified:    modules/generators mod_autoindex.c
> >   Log:
> >   This is a HACK!
> 
> Why would it be difficult for the core to preserve OLD_WRITE in the
subreq
> filter chain?  We knew how to do that in 2.0.32.  One would hope we
get
> smarter
> as we get more experienced.

Because in pre-2.0.32 code, fast_internal_redirects didn't remove all of
the RESOURCE filters.  It does now, because that is the correct thing to
do.  The problem is that there is one RESOURCE filters that we want to
keep, OLD_WRITE.  I had thought about just having the core do this, but
that gets ugly quickly, because the bug is incredibly specific to having
a fast_internal_redirect inside of a content generator.

Ryan



Re: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Greg Ames <gr...@remulak.net>.
rbb@apache.org wrote:
> 
> rbb         02/04/05 09:50:37
> 
>   Modified:    modules/generators mod_autoindex.c
>   Log:
>   This is a HACK! 

Why would it be difficult for the core to preserve OLD_WRITE in the subreq
filter chain?  We knew how to do that in 2.0.32.  One would hope we get smarter
as we get more experienced.

>         The problem is that the fast_internal_redirect is removing the OLD_WRITE filter. 

I'm going to try it on my box without this patch, and with no Multiviews (to get
rid of fast_internal_redirects for HEADER and README).  If that works with HEAD
as well as it did in 2.0.32, great!  If not, I think we need to do better.

Greg

p.s. why do I care?  I'm working on a SpecWeb99 module for Apache 2.0.  The way
I see it working is using ap_rputs to generate a little bit of dynamic html,
then running a subrequest to send a file, then using ap_rputs to generate a
small trailer.  That would work in 2.0.32.  IMO it should also work in a GA
release.

RE: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Ryan Bloom <rb...@covalent.net>.
>   Modified:    modules/generators mod_autoindex.c
>   Log:
>   This is a HACK!  The problem is that the fast_internal_redirect is
>   removing the OLD_WRITE filter. Obviously that is wrong.  For right
now,
>   the fix is to hack around the problem and just make it work.  Long
term,
>   we need to find a real solution to this, but this gets autoindex
working
>   today.

This fixes the final problems with mod_autoindex.  I agree that this is
not the correct long-term solution.  I am deadly serious when I say I
want a GA release today.

The only way that 2.0 has ever made it out the door is for somebody to
stand up and say that they want this more than life itself.  I am saying
that today.  Let's ship 2.0.34 (bumped to HEAD) today, and if we need a
2.0.35 next week to fix some bugs, then we will do that next week.

This thing has been five years in the making already, and it's time to
go GA!

Ryan

 


RE: cvs commit: httpd-2.0/modules/generators mod_autoindex.c

Posted by Ryan Bloom <rb...@covalent.net>.
>   Modified:    modules/generators mod_autoindex.c
>   Log:
>   This is a HACK!  The problem is that the fast_internal_redirect is
>   removing the OLD_WRITE filter. Obviously that is wrong.  For right
now,
>   the fix is to hack around the problem and just make it work.  Long
term,
>   we need to find a real solution to this, but this gets autoindex
working
>   today.

This fixes the final problems with mod_autoindex.  I agree that this is
not the correct long-term solution.  I am deadly serious when I say I
want a GA release today.

The only way that 2.0 has ever made it out the door is for somebody to
stand up and say that they want this more than life itself.  I am saying
that today.  Let's ship 2.0.34 (bumped to HEAD) today, and if we need a
2.0.35 next week to fix some bugs, then we will do that next week.

This thing has been five years in the making already, and it's time to
go GA!

Ryan