httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@ebuilt.com>
Subject Filter Breakage is Re: cvs commit: httpd-2.0/server core.c
Date Mon, 04 Mar 2002 10:32:31 GMT
On Mon, Mar 04, 2002 at 09:20:03AM -0000, jerenkrantz@apache.org wrote:
> jerenkrantz    02/03/04 01:20:03
> 
>   Modified:    server   core.c
>   Log:
>   Ensure that net_time filter isn't added on subreqs - we assume that it is
>   added on !r->main requests.  This led to infinite loop/SEGV when dealing
>   with anything that created a subreq.
>   
>   (I don't think core_create_req is a good place for adding this filter.)

This fixes one of my problems.

I now have two further problems.

1) Since fast_redirect() is called from a hook, but all hooks are run
on this "new" request - it is possible that some hooks are left to
be run from the original request.  This causes a problem for the
AddOutputFilterByType hook (core_filters_type) since it is a fixup
as well and is run after the mod_dir/mod_negotiation hooks.  Therefore,
when using this directive, it adds each filter twice causing segfaults
and infinite loops.  Oops.

We could play with hook ordering, but I'm pretty sure that's the
wrong thing to be doing.  Of course, I'm advocating removing
fast_redirect for precisely this problem.

2) This code still can't handle filtering order properly across
sub-requests.  There are two problems with this approach.  The
first is that the lists aren't distinct and are corrupted as we
generate the subreqs.  The second problem is that I believe it is
possible for a configuration to add a protocol-level filter (say,
mod_headers or mod_deflate), and this new semantic does not
support that properly (probably due to the assumption that
protocol filters can't be inserted in a subreq).

Imagine the following hypothetical configuration:

Alias /manual "/foo/manual"
<Directory "/foo/manual">
    Options Indexes MultiViews
    AddOutputFilter INCLUDES .html
</Directory>

Let's request /manual/ from our server.  What happens?

First off we create subreqs for index.html.  That triggers the
mod_negotiation reqs.  When mod_negotiation runs, it'll actually
create multiple subreqs that are *valid* - (i.e. index.html.en,
index.html.fr, index.html.de, etc, etc.).  Therefore, the hooks
will be run on *each* of these.  It'll then pick one of these
subreqs to be our request and call fast_redirect() on it.  And,
when mod_negotiation is done, mod_dir will cast fast_redirect()
once more (to unravel our request paths).  (Hopefully, by now
Ryan sees where this is headed.)

mod_mime will dutifully add INCLUDES to each of the subreqs as
mod_negotiation runs each subreq.  Aha!  But, what happens to
proto_output_filters?  It's prev will be modified to point at
this subreq's INCLUDES filter.  But, let's say we take the 3rd
subreq (i.e. not the last subreq we ran).  So, we do our
fast_redirect() magic and we promote r->output_filters from the
*third* subreq.  proto_output_filter's prev is pointing at the
*last* subreq's INCLUDES filter.  Uh, oh.  output_filters chain
is corrupted.  Witness this:

(gdb) print r->output_filters
$122 = (struct ap_filter_t *) 0x81a8538
(gdb) print r->output_filters->next
$123 = (ap_filter_t *) 0x8166aa8
(gdb) print r->output_filters->next->prev
$124 = (ap_filter_t *) 0x81a6560

Oops.  So, when we go run ap_run_insert_filter hook (to add
the protocol-handling filters), we try to add byterange filter,
but we get screwed and we miss the following code at
util_filter.c (line 356):

if (*outf && ((*outf)->prev == first)) {
    (*outf)->prev = f;
}

So, the previous isn't updated.  No big deal, you say?  Let's
consider the second filter we add in ap_run_insert_filter -
content-length.  When, it comes in, our check to INSERT_BEFORE
returns false, so we'll insert it afterwards.  Because of our
bad prev pointers, this code (line 368) never gets called:

if (fscan->next->prev == fscan) {
    f->prev = fscan;
    fscan->next->prev = f;
    fscan->next = f;
}

That should be adding the next pointer such that fscan->next points
at our newly created filter.  Since that call fails, only the
first filter will be inserted as we go forward in the list (and
it is indeterminate as you would traverse backwards in the list).
That's one problem.  The solution here would be to make the
output_filters chain for *each* subreq to be distinct.  But, that
requires copying the lists on each subreq such that the filter
chains are distinct.

Currently, we end up with:

Includes->Byterange->Core

with HTTP and content-length being hidden in the prev of Core.  So,
mod_dir/mod_negotiation requests with AddOutputFilter[ByType] are
still broken.

The second half of the filtering problem is that we don't promote
r->proto_output_filters when doing a fast_redirect.  What if we
changed our example directive to be:

AddOutputFilter DEFLATE .html

It'd get added to the r->output_filters of the subreq and the
r->proto_output_filters would be correct as well for this
subreq.  However, when we do the promotion in fast_redirect(),
only r->output_filters is used.  So, r->output_filters is
DEFLATE->CORE.  But, r->proto_output_filters after fast_redirect
is called is: CORE *not* DEFLATE->CORE since we don't update it.
That introduces havoc when we try to add filters to the
proto_output_filters chain as we have a HTTP_HEADER filter in
our chain but it isn't pointed at by proto_filters.  Ooops.

Hope this provides some explanations as to the problems I'm
seeing.  Again, I'm not at all sold on this proto_output_filters
trick as I don't think it will solve our problem.  However, I'm
willing to wait for Ryan to fix it because he seems pretty sure
that this will work, but I'm definitely concerned about the extra
complexity we're introducing.  -- justin


Mime
View raw message