You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by td...@apache.org on 2008/06/20 23:12:34 UTC
svn commit: r670061 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_rewrite.c
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);
+ }
}
/* now make sure the request gets handled by the proxy handler */
Re: svn commit: r670061 - in /httpd/httpd/trunk: CHANGES
modules/mappers/mod_rewrite.c
Posted by Tom Donovan <do...@bellatlantic.net>.
Ruediger Pluem wrote:
>
>
> 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".
...
>
> 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");
> > - }
> > -
>
...
> 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);
>
>
Understood. Changed it to:
((r->proxyreq == PROXYREQ_PROXY) || (rulestatus == ACTION_NOESCAPE))
Regards,
-tom-
Re: svn commit: r670061 - in /httpd/httpd/trunk: CHANGES modules/mappers/mod_rewrite.c
Posted by Ruediger Pluem <rp...@apache.org>.
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