On Mon, Mar 04, 2002 at 09:20:03AM -0000, [EMAIL PROTECTED] 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