httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Plüm, Rüdiger, VF-Group <ruediger.pl...@vodafone.com>
Subject Re: ap_sub_req_lookup_* loosing filter context
Date Fri, 08 Feb 2008 16:45:45 GMT
 

> -----Ursprüngliche Nachricht-----
> Von: Dirk-Willem van Gulik 
> Gesendet: Freitag, 8. Februar 2008 16:32
> An: dev@httpd.apache.org
> Betreff: ap_sub_req_lookup_* loosing filter context
> 
> In a number of places we are usingL
> 
> 		... = ap_sub_req_lookup_uri(..., r, NULL);
> 		... = ap_sub_req_lookup_file(..., r, NULL);
> 		... = ap_sub_req_lookup_dirent...., NULL);
> 		... = ap_sub_req_lookup_uri( ... ,NULL);
> 
> Where null is the output filter to preserve, if any (at NULL  
> proto_output_filter is used).
> 
> We do this to see what a sub request would look like; and then later  
> on we clean it up; perhaps after capturing some result, 
> modified path,  
> filling out some SSI etc (e.g. mod_include, mod_isapi, 
> mod_auto_index,  
> mod_cgid, mod_cern, mod_mime_magic or mod_rewrite). And then we  
> discart the sub request and continue with the real request.
> 
> However we also occasionally do NOT do this - but make the thus  
> created sub request as the main request  i.e. some redirect where we  
> do not discard the sub_req -- but continue to operate on it 
> (e.g. the  
> core of mod_dir, mod_negotiation) - and pass it to a  
> internal(fast)redirect.
> 
> Am I right to understand that this is not correct ?
> 
> As far as I can see (see patch below) - there is no harmful side  
> effect from fixing this - while it does seem to fix mod_cache when  
> dealing with content remappings done by mod_dir and mod_negotation.
> 
> Thoughts ?
> 
> Dw
> 
> Index: modules/mappers/mod_negotiation.c
> ===================================================================
> --- modules/mappers/mod_negotiation.c	(revision 618646)
> +++ modules/mappers/mod_negotiation.c	(working copy)
> @@ -1165,8 +1165,10 @@
> 
>           /* Double check, we still don't multi-resolve non-ordinary  
> files
>            */
> -        if (sub_req->finfo.filetype != APR_REG)
> +        if (sub_req->finfo.filetype != APR_REG) {
> +	    /* XXX sub req not destroyed */
>               continue;
> +	}
> 
>           /* If it has a handler, we'll pretend it's a CGI script,
>            * since that's a good indication of the sort of thing it
> @@ -3122,7 +3124,7 @@
>            * a sub_req structure yet.  Get one now.
>            */
> 
> -        sub_req = ap_sub_req_lookup_file(best->file_name, r, NULL);
> +        sub_req = ap_sub_req_lookup_file(best->file_name, r, r- 
>  >output_filter);
>           if (sub_req->status != HTTP_OK) {
>               res = sub_req->status;
>               ap_destroy_sub_req(sub_req);

I guess this happens in at least one more location at mod_negotiation
that would also need fixing (in setup_choice_response).

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.

Regards

Rüdiger


Mime
View raw message