Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <485C2B17.4000406@apache.org>
Date: Sat, 21 Jun 2008 00:11:35 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.13) Gecko/20080313 SeaMonkey/1.1.9
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r670061 - in /httpd/httpd/trunk: CHANGES
 modules/mappers/mod_rewrite.c
References: <20080620211234.C885A23889BB@eris.apache.org>
In-Reply-To: <20080620211234.C885A23889BB@eris.apache.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit


On 06/20/2008 11:12 PM, tdonovan@apache.org wrote:
> Author: tdonovan
> Date: Fri Jun 20 14:12:34 2008
> New Revision: 670061
> 
> URL: http://svn.apache.org/viewvc?rev=670061&view=rev
> Log:
> After r649840, mod_proxy_http will no longer append a query string from r->args if "no-canon".
> 
> Moved the NOESCAPE test down after PATH_INFO, and preserve the query string in r->filename if  
> NOESCAPE (which implies "no-canon")
> 
> Previously this was only done for CONNECT requests, where (r->uri == r->unparsed_uri)
> see mod_proxy:proxy_detect
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> 
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=670061&r1=670060&r2=670061&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Jun 20 14:12:34 2008
> @@ -2,6 +2,9 @@
>  Changes with Apache 2.3.0
>  [ When backported to 2.2.x, remove entry from this file ]
>  
> +  *) mod_rewrite: Preserve the query string with [proxy,noescape]. PR 45247
> +     [Tom Donovan]
> +
>    *) mod_proxy_http: Do not forward requests with 'Expect: 100-continue' to
>       known HTTP/1.0 servers. Return 'Expectation failed' (417) instead.
>       [Ruediger Pluem]
> 
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=670061&r1=670060&r2=670061&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Fri Jun 20 14:12:34 2008
> @@ -4453,10 +4453,6 @@
>                  return HTTP_FORBIDDEN;
>              }
>  
> -            if (rulestatus == ACTION_NOESCAPE) {
> -                apr_table_setn(r->notes, "proxy-nocanon", "1");
> -            }
> -
>              /* make sure the QUERY_STRING and
>               * PATH_INFO parts get incorporated
>               */
> @@ -4464,11 +4460,16 @@
>                  r->filename = apr_pstrcat(r->pool, r->filename,
>                                            r->path_info, NULL);
>              }
> -            if (r->args != NULL &&
> -                r->uri == r->unparsed_uri) {
> -                /* see proxy_http:proxy_http_canon() */
> -                r->filename = apr_pstrcat(r->pool, r->filename,
> -                                          "?", r->args, NULL);
> +            if (rulestatus == ACTION_NOESCAPE) {
> +                /* make sure that mod_proxy_http doesn't canonicalize the URI,
> +                 * and preserve any (possibly qsappend'd) query string in the
> +                 * filename for mod_proxy_http:proxy_http_canon()
> +                 */
> +                apr_table_setn(r->notes, "proxy-nocanon", "1");
> +                if (r->args != NULL) {
> +                    r->filename = apr_pstrcat(r->pool, r->filename,
> +                                              "?", r->args, NULL);
> +                }
>              }

I think this will break in the case that we have a forward proxy. So my proposal would be:

Leave as is (or better was)

 > -            if (rulestatus == ACTION_NOESCAPE) {
 > -                apr_table_setn(r->notes, "proxy-nocanon", "1");
 > -            }
 > -

Change

 > -            if (r->args != NULL &&
 > -                r->uri == r->unparsed_uri) {
 > -                /* see proxy_http:proxy_http_canon() */
 > -                r->filename = apr_pstrcat(r->pool, r->filename,
 > -                                          "?", r->args, NULL);


to

 > -            if (r->args != NULL &&
 > -                ((r->uri == r->unparsed_uri) || (rulestatus == ACTION_NOESCAPE))) {
 > -                /* see proxy_http:proxy_http_canon() */
 > -                r->filename = apr_pstrcat(r->pool, r->filename,
 > -                                          "?", r->args, NULL);


or even better to

 > -            if (r->args != NULL &&
 > -                ((r->proxyreq == PROXYREQ_PROXY) || (rulestatus == ACTION_NOESCAPE))) {
 > -                /* see proxy_http:proxy_http_canon() */
 > -                r->filename = apr_pstrcat(r->pool, r->filename,
 > -                                          "?", r->args, NULL);


Regards

Rüdiger