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: domain of dirkx@webweaving.org
 designates 209.132.96.45 as permitted sender)
Message-Id: <D182F681-F751-472C-8C56-12E09F995367@webweaving.org>
From: Dirk-Willem van Gulik <dirkx@webweaving.org>
To: dev@httpd.apache.org
In-Reply-To: <490684CD-E424-4163-9F9E-69DE491BAD95@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: Sat, 9 Feb 2008 00:33:25 +0100
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>


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);