Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: local policy)
Message-Id: <316ED1A0-7A9E-499C-AB6E-04F12FCD503D@jaguNET.com>
From: Jim Jagielski <jim@jaguNET.com>
To: dev@httpd.apache.org
In-Reply-To: <D182F681-F751-472C-8C56-12E09F995367@webweaving.org>
Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0 (Apple Message framework v915)
Subject: Re: ap_sub_req_lookup_* loosing filter context
Date: Mon, 11 Feb 2008 08:42:47 -0500
References: <335D1A4B-25E2-4FF1-8CDF-5010A7FBD293@webweaving.org>
 <99EA83DCDE961346AFA9B5EC33FEC08B46461D@VF-MBX11.internal.vodafone.com>
 <54D6F4D6-1E26-4931-9E0B-BEC9EB903D96@webweaving.org>
 <47ACE0B3.5020600@apache.org>
 <490684CD-E424-4163-9F9E-69DE491BAD95@webweaving.org>
 <D182F681-F751-472C-8C56-12E09F995367@webweaving.org>

We should likely "correct" the comment in make_sub_request
at the same time ;)

On Feb 8, 2008, at 6:33 PM, Dirk-Willem van Gulik wrote:

>
> On Feb 9, 2008, at 12:18 AM, Dirk-Willem van Gulik wrote:
>
>>
>> On Feb 9, 2008, at 12:07 AM, Ruediger Pluem wrote:
>>
>>>>> I guess this happens in at least one more location at  
>>>>> mod_negotiation
>>>>> that would also need fixing (in setup_choice_response).
>>>>
>>>> Below are all the only two places I could find. Let me know which  
>>>> ones I
>>>> missed !
>>>
>>> IMHO this one is missing:
>>>
>>> Index: mod_negotiation.c
>>> ===================================================================
>>> --- mod_negotiation.c   (Revision 619126)
>>> +++ mod_negotiation.c   (Arbeitskopie)
>>> @@ -2712,7 +2712,7 @@
>>>   if (!variant->sub_req) {
>>>       int status;
>>>
>>> -        sub_req = ap_sub_req_lookup_file(variant->file_name, r,  
>>> NULL);
>>> +        sub_req = ap_sub_req_lookup_file(variant->file_name, r, r- 
>>> >output_filters));
>>>       status = sub_req->status;
>>>
>>>       if (status != HTTP_OK &&
>>>
>>
>> Ah - excelent -- and i think that fixed a bug I had as well.
>
>
> It did. I'll probably commit, with a large "Warning - this may  
> subtly break things", below when I am a bit more awake :) Bed time  
> here.
>
> Dw.
>
> 4x4:httpd-trunk-filters dirkx$ svn diff CHANGES modules/mappers/
> Index: CHANGES
> ===================================================================
> --- CHANGES	(revision 620036)
> +++ CHANGES	(working copy)
> @@ -2,6 +2,12 @@
> Changes with Apache 2.3.0
> [ When backported to 2.2.x, remove entry from this file ]
>
> +  *) mod_dir, mod_negotiation: pass the output filter information
> +     to newly created sub requests; as these are later on used
> +     as true requests with an internal redirect. This allows for
> +     mod_cache et.al. to trap the results of the redirect.
> +     [Dirk-Willem van Gulik, Ruediger Pluem]
> +
>   *) ab: Use a 64 bit unsigned int instead of a signed long to count  
> the
>      bytes transferred to avoid integer overflows. PR 44346  
> [Ruediger Pluem]
>
> Index: modules/mappers/mod_dir.c
> ===================================================================
> --- modules/mappers/mod_dir.c	(revision 620036)
> +++ modules/mappers/mod_dir.c	(working copy)
> @@ -176,7 +176,7 @@
>             name_ptr = apr_pstrcat(r->pool, name_ptr, "?", r->args,  
> NULL);
>         }
>
> -        rr = ap_sub_req_lookup_uri(name_ptr, r, NULL);
> +        rr = ap_sub_req_lookup_uri(name_ptr, r, r->output_filters);
>
>         /* The sub request lookup is very liberal, and the core  
> map_to_storage
>          * handler will almost always result in HTTP_OK as /foo/ 
> index.html
> Index: modules/mappers/mod_negotiation.c
> ===================================================================
> --- modules/mappers/mod_negotiation.c	(revision 620036)
> +++ 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 -- may be a bug/unintentional ? */
>             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
> @@ -2712,7 +2714,7 @@
>     if (!variant->sub_req) {
>         int status;
>
> -        sub_req = ap_sub_req_lookup_file(variant->file_name, r,  
> NULL);
> +        sub_req = ap_sub_req_lookup_file(variant->file_name, r, r- 
> >output_filters);
>         status = sub_req->status;
>
>         if (status != HTTP_OK &&
> @@ -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_filters);
>         if (sub_req->status != HTTP_OK) {
>             res = sub_req->status;
>             ap_destroy_sub_req(sub_req);
>