httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ryan Bloom" <...@covalent.net>
Subject RE: [PATCH] Fix mod_autoindex
Date Fri, 05 Apr 2002 16:32:31 GMT
OKAY, I have fully diagnosed the problem.

The problem only occurs if you have multi-views enabled for the HEADER
and README files.  What is killing us is that we are creating a
sub-request from within another sub-request, but we only have one
SUB_REQ_OUTPUT_FILTER on the chain.  This means that we end up removing
the filter and then we finalize the first sub_request.

I'm creating a patch now.

Ryan

----------------------------------------------
Ryan Bloom                  rbb@covalent.net
645 Howard St.              rbb@apache.org
San Francisco, CA 

> -----Original Message-----
> From: trawick@rdu88-250-035.nc.rr.com [mailto:trawick@rdu88-250-
> 035.nc.rr.com] On Behalf Of Jeff Trawick
> Sent: Friday, April 05, 2002 8:06 AM
> To: rbb@covalent.net
> Cc: dev@httpd.apache.org
> Subject: Re: [PATCH] Fix mod_autoindex
> 
> (This is really from me :) )
> 
> "Ryan Bloom" <rbb@covalent.net> writes:
> 
> > > > This is another attempt at fixing mod_autoindex.  This works for
me
> > on
> > > > my computer, and it explains the problem.  I haven't been able
to
> > > > actually reproduce the problem, but I have seen the symptoms.
Can
> > > > somebody who can reproduce PLEASE test this on their setup.  I
am
> > hoping
> > > > to setup a failure condition soon so that I can test this
myself.
> > > >
> > > > I may commit this either way, because even if this doesn't fix
the
> > > > problem from apache.org, it does solve a problem with data
ordering.
> > > >
> > > > This is the cleanest way I could find to solve the problem, but
I am
> > > > perfectly willing to listen to other ideas.
> > >
> > > Greg and I discussed this issue (create subrequest, create output
via
> > > ap_rXXX, run subrequest) a couple of days ago.  What we preferred
> > > overall was to require an app to call something like ap_rinit() if
> > > they were going to use ap_rXXX functions.  ap_rinit() would add
> > > old_write filter.  ap_rinit() would have to be called at the start
of
> > > content generation, before creating any subrequests.
> > >
> > > Yes it is ugly, but yes it is very simple to implement and keep
> > > working, and yes it is easy to give the right feedback to the app
if
> > > they forget to call ap_rinit() (return 500 and log an error if
> > > buffer_output is called with no old_write filter; currently if we
get
> > > to buffer_output with no old_write filter we add it).
> > >
> > > This seems to be a reasonable worse-is-better tradeoff.  It is
easier
> > > for an app writer to do something like call ap_rinit() than it is
to
> > > do a trick like in your patch.  It is easier for us to maintain
such
> > > an API too, which in turn makes life simpler for the app writer.
> >
> > The problem I have with that is that in 99% of the cases, the call
to
> > ap_rinit is unnecessary.  The whole point of the ap_r* functions was
to
> > provide a level of source backwards compatibility with Apache 1.3,
so
> > that developers wouldn't have to modify their modules.  If you force
> > them all to run ap_rinit before they can run ap_r*, then you have
> > completely defeated that goal.
> 
> It is a hindrance that can be solved in a couple of minutes (as long
> as they read the error_log).  This is similar in scale to the
> hindrance of needing to remove the call to ap_send_http_header().
> 
> > And, as I have said, this is an edge case.  If you look at all of
the
> > other places in the code that we create a sub-request, we always
create
> > it and run it.  This module happens to output some data in between
those
> > two operations.  I don't think we want to force all module authors
to
> > jump through hoops for an edge case.
> >
> > We can create the function, and advise its use, but we should not
> > mandate it.
> 
> A benefit of mandating it is that it ensures that a module with the
> pattern seen in mod_autoindex isn't broken (because it forgot to call
> ap_rinit()).  And the cost to the app writer is trivial.  But either
> way (mandate or not), it would seem to be a simple change for Apache
> and for the module.
> 
> --
> Jeff Trawick | trawick@attglobal.net
> Born in Roswell... married an alien...


Mime
View raw message