> > 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.
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.
>
> > Index: modules/generators/mod_autoindex.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/modules/generators/mod_autoindex.c,v
> > retrieving revision 1.102
> > diff -u -d -b -w -u -r1.102 mod_autoindex.c
> > --- modules/generators/mod_autoindex.c 20 Mar 2002 17:41:54 -0000
> > 1.102
> > +++ modules/generators/mod_autoindex.c 5 Apr 2002 06:26:08 -0000
> > @@ -1017,6 +1017,7 @@
> > if (rr->content_type != NULL) {
> > if (!strcasecmp(ap_field_noparam(r->pool,
> > rr->content_type),
> > "text/html")) {
> > + ap_filter_t *f;
> > /* Hope everything will work... */
> > emit_amble = 0;
> > emit_H1 = 0;
> > @@ -1024,6 +1025,22 @@
> > 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
>
> I didn't realize this was the only problem. I thought there was still
> an issue of getting the subrequest filter in place in certain
> conditions.
I have been unable to reproduce that. If I can get the permissions to
debug the server running on daedalus, I can most likely find and fix
that later today. My problem right now is that I'm not seeing the same
things you are in the tests.
Finally, I would actually like proof that this does solve the problem we
are seeing. Have we tested this patch on daedalus yet?
> (also, can you fix your e-mail client to not wrap patches at column
72?)
Yeah, LookOut really sucks, but I am working on it.
Ryan
