> -----Ursprüngliche Nachricht-----
> Von: Dirk-Willem van Gulik
> Gesendet: Freitag, 8. Februar 2008 16:32
> An: [email protected]
> 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
