(This is really from me :) )

"Ryan Bloom" <[EMAIL PROTECTED]> 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 | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Reply via email to