This patch fixes a problem in mod_rewrite. The following config with the
RewriteRule in per-directory context works fine for query strings:
<Directory "/">
RewriteEngine On
RewriteRule ^foo http://hostname/bar [R=301,L]
</Directory>
But the same RewriteRule in per-server context doesn't work:
<VirtualHost *:80>
RewriteEngine On
RewriteRule ^/foo http://hostname/bar [R=301,L]
</VirtualHost>
The second example leads to double URL-escaping of the query string:
GET /foo?q=%25
301 Moved Permanently
Location: http://hostname/bar?q=%25%25
This is PR 50447 [1], and the issue was fixed for per-directory contexts in
r1044673 [2]. However, RewriteRules in server context are handled differently
in mod_rewrite, and their handling wasn't updated. The attached patch applies
the same fix for such rules. The second patch adds a regression test for the
issue in t/modules/rewrite.t. Both of the patches are against trunk.
[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=50447
[2] https://svn.apache.org/r1044673
Regards,
Evgeny Kotkov
mod_rewrite: Fix double escaping of the query string for RewriteRules in
server context.
See PR 50447 and r1044673 that fixed this for directory contexts.
Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c (revision 1735001)
+++ modules/mappers/mod_rewrite.c (working copy)
@@ -4548,6 +4548,7 @@ static int hook_uri2file(request_rec *r)
unsigned int port;
int rulestatus;
void *skipdata;
+ const char *oargs;
/*
* retrieve the config structures
@@ -4598,6 +4599,12 @@ static int hook_uri2file(request_rec *r)
}
/*
+ * remember the original query string for later check, since we don't
+ * want to apply URL-escaping when no substitution has changed it.
+ */
+ oargs = r->args;
+
+ /*
* add the SCRIPT_URL variable to the env. this is a bit complicated
* due to the fact that apache uses subrequests and internal redirects
*/
@@ -4731,11 +4738,21 @@ static int hook_uri2file(request_rec *r)
/* append the QUERY_STRING part */
if (r->args) {
+ char *escaped_args = NULL;
+ int noescape = (rulestatus == ACTION_NOESCAPE ||
+ (oargs && !strcmp(r->args, oargs)));
+
r->filename = apr_pstrcat(r->pool, r->filename, "?",
- (rulestatus == ACTION_NOESCAPE)
+ noescape
? r->args
- : ap_escape_uri(r->pool, r->args),
+ : (escaped_args =
+ ap_escape_uri(r->pool,
r->args)),
NULL);
+
+ rewritelog((r, 1, NULL, "%s %s to query string for redirect
%s",
+ noescape ? "copying" : "escaping",
+ r->args ,
+ noescape ? "" : escaped_args));
}
/* determine HTTP redirect response code */
mod_rewrite: Add regression tests for PR 50447 (double URL-escaping of
the query string).
Index: t/conf/extra.conf.in
===================================================================
--- t/conf/extra.conf.in (revision 1735001)
+++ t/conf/extra.conf.in (working copy)
@@ -234,6 +234,9 @@
## Proxy and QSA
RewriteRule ^proxy-qsa.html$
http://@SERVERNAME@:@PORT@/modules/cgi/env.pl?foo=bar [QSA,L,P]
+ ## Redirect, directory context
+ RewriteRule ^redirect-dir.html$ http://@SERVERNAME@:@PORT@/foobar.html
[L,R=301]
+
</Directory>
### Proxy pass-through to env.pl
@@ -243,6 +246,9 @@
RewriteCond %{QUERY_STRING} horse=trigger
RewriteRule ^/modules/rewrite/proxy3/(.*)$
http://@SERVERNAME@:@PORT@/modules/cgi/$1 [L,P]
+ ### Redirect, server context
+ RewriteRule ^/modules/rewrite/redirect.html$
http://@SERVERNAME@:@PORT@/foobar.html [L,R=301]
+
<VirtualHost cve_2011_3368_rewrite>
DocumentRoot @SERVERROOT@/htdocs/modules/proxy
RewriteEngine On
Index: t/modules/rewrite.t
===================================================================
--- t/modules/rewrite.t (revision 1735001)
+++ t/modules/rewrite.t (working copy)
@@ -12,11 +12,20 @@ use Apache::TestUtil;
my @map = qw(txt rnd prg); #dbm XXX: howto determine dbm support is available?
my @num = qw(1 2 3 4 5 6);
my @url = qw(forbidden gone perm temp);
-my @todo = ();
+my @todo;
my $r;
-plan tests => @map * @num + 11, todo => \@todo, need_module 'rewrite';
+if (!have_min_apache_version('2.5')) {
+ # PR 50447, server context
+ push @todo, 26
+}
+if (!have_min_apache_version('2.4')) {
+ # PR 50447, directory context (r1044673)
+ push @todo, 24
+}
+plan tests => @map * @num + 15, todo => \@todo, need_module 'rewrite';
+
foreach (@map) {
foreach my $n (@num) {
## throw $_ into upper case just so we can test out internal
@@ -58,6 +67,19 @@ $r = GET_BODY("/modules/rewrite/qsa.html?baz=bee")
chomp $r;
ok t_cmp($r, qr/\nQUERY_STRING = foo=bar\&baz=bee\n/s, "query-string append
test");
+# PR 50447 (double URL-escaping of the query string)
+my $hostport = Apache::TestRequest::hostport();
+
+$r = GET("/modules/rewrite/redirect-dir.html?q=%25", redirect_ok => 0);
+ok t_cmp($r->code, 301, "per-dir redirect response code is OK");
+ok t_cmp($r->header("Location"), "http://$hostport/foobar.html?q=%25";,
+ "per-dir query-string escaping is OK");
+
+$r = GET("/modules/rewrite/redirect.html?q=%25", redirect_ok => 0);
+ok t_cmp($r->code, 301, "redirect response code is OK");
+ok t_cmp($r->header("Location"), "http://$hostport/foobar.html?q=%25";,
+ "query-string escaping is OK");
+
if (have_module('mod_proxy')) {
$r = GET_BODY("/modules/rewrite/proxy.html");
chomp $r;
