On 02/08/2008 11:34 PM, Dirk-Willem van Gulik wrote:
> On Feb 8, 2008, at 5:45 PM, Plüm, Rüdiger, VF-Group wrote:
> 
>> OTOH a comment from make_sub_request makes me think whether doing this
>> is the
>> correct thing:
>>
>>        /* If NULL - we are expecting to be internal_fast_redirect'ed
>>         * to this subrequest - or this request will never be invoked.
>>         * Ignore the original request filter stack entirely, and
>>         * drill the input and output stacks back to the connection.
>>         */
>>
>> I guess ap_internal_fast_redirect is always called before we call
>> ap_invoke_handler (which calls ap_run_insert_filter). So normally we
>> should not have setup
>> anything before the proto filters, except in the mod_cache situation
>> where ap_run_insert_filter
>> is run during the quick handler phase, by mod_cache's quick handler
>> and thus we have those filters
>> setup already.
> 
> 
> Well not sure -- but what you see in terms of installed handlers is this:
> 
> Thu Feb 07 14:12:32 2008] [debug] mod_cache.c(144): Adding CACHE_SAVE
> filter for /foo/
> [Thu Feb 07 14:12:32 2008] [debug] mod_cache.c(151): Adding
> CACHE_REMOVE_URL filter for /foo/
> mod_dir.c:111 Filters: r->output_filters:->cache_save ->byterange
> ->content_length ->http_header ->http_outerror ->cache_remove_url ->core
> ... the check
> mod_dir.c:111 Filters: r->output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> .. the redirect
> mod_dir.c:205 Filters: r->output_filters:->cache_save ->byterange
> ->content_length ->http_header ->http_outerror ->cache_remove_url ->core
> mod_dir.c:206 Filters: rr->output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> mod_dir: Calling Redir
> Filters: r->output_filters:->byterange ->content_length ->http_header
> ->http_outerror ->cache_remove_url ->core
> Filters: rr->output_filters:->byterange ->content_length ->http_header
> ->http_outerror ->cache_remove_url ->core
> Filters: r->proto_output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> Filters: rr->proto_output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> Filters: rr->proto_output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> Filters: r->proto_output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> 
> mod_dir.c:211 Filters: r->output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> mod_dir.c:212 Filters: rr->output_filters:->byterange ->content_length
> ->http_header ->http_outerror ->cache_remove_url ->core
> [Thu Feb 07 14:12:32 2008] [debug] mod_cache.c(898):
> cache_remove_url_filter
> [Thu Feb 07 14:12:32 2008] [debug] mod_cache.c(918): cache:
> ap_remove_output_filter cache_remove_url
> 
> So the 'redirect' is the actual closing request -- and the first iterm
> -- cache_save is 'lost'.

Yes, I agree with you, but I did not express this clearly enough in my previous
mail. The comment in make_sub_request made me think about this deeper. But in
the next paragraph I thought out loud and wrote down why the comment in 
make_sub_request
seems to make the *wrong* assumption (that ap_run_insert_filter has not been 
run yet).
I assume that this comment is older than mod_cache and the quick handler stuff
so it was correct back then. So I think it is correct to move forward with
your fixes in trunk and let us see if this breaks something.

Regards

Rüdiger

Reply via email to