This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 130d36d849 Fix a couple of issues with QSA/QSD handling and associated
tests
130d36d849 is described below
commit 130d36d8492ef9e4eb22952c17c92423cb35fd06
Author: Mark Thomas <[email protected]>
AuthorDate: Fri Aug 29 14:42:14 2025 +0100
Fix a couple of issues with QSA/QSD handling and associated tests
- Original query string was incorrectly retained when there was no new
query string and QSD was set
- QSD did not take precedence when QSA and QSD were both set
Rename variables for consistency / ease of maintenance
---
.../catalina/valves/rewrite/RewriteValve.java | 35 ++++---
.../apache/catalina/startup/TomcatBaseTest.java | 2 +-
.../catalina/valves/rewrite/TestRewriteValve.java | 107 ++++++++++++++++++++-
webapps/docs/changelog.xml | 4 +
4 files changed, 131 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/valves/rewrite/RewriteValve.java
b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
index baf4129c6b..39dac746f5 100644
--- a/java/org/apache/catalina/valves/rewrite/RewriteValve.java
+++ b/java/org/apache/catalina/valves/rewrite/RewriteValve.java
@@ -326,7 +326,7 @@ public class RewriteValve extends ValveBase {
// As long as MB isn't a char sequence or affiliated, this has to
be converted to a string
Charset uriCharset = request.getConnector().getURICharset();
- String originalQueryStringEncoded = request.getQueryString();
+ String queryStringOriginalEncoded = request.getQueryString();
MessageBytes urlMB = context ? request.getRequestPathMB() :
request.getDecodedRequestURIMB();
urlMB.toChars();
CharSequence urlDecoded = urlMB.getCharChunk();
@@ -427,10 +427,10 @@ public class RewriteValve extends ValveBase {
StringBuilder urlStringEncoded =
new
StringBuilder(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded,
uriCharset));
- if (!qsd && originalQueryStringEncoded != null &&
!originalQueryStringEncoded.isEmpty()) {
+ if (!qsd && queryStringOriginalEncoded != null &&
!queryStringOriginalEncoded.isEmpty()) {
if (rewrittenQueryStringRewriteEncoded == null) {
urlStringEncoded.append('?');
-
urlStringEncoded.append(originalQueryStringEncoded);
+
urlStringEncoded.append(queryStringOriginalEncoded);
} else {
if (qsa) {
// if qsa is specified append the query
@@ -438,7 +438,7 @@ public class RewriteValve extends ValveBase {
urlStringEncoded.append(
REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
urlStringEncoded.append('&');
-
urlStringEncoded.append(originalQueryStringEncoded);
+
urlStringEncoded.append(queryStringOriginalEncoded);
} else if (index == urlStringEncoded.length() - 1)
{
// if the ? is the last character delete it,
its only purpose was to
// prevent the rewrite module from appending
the query string
@@ -554,24 +554,31 @@ public class RewriteValve extends ValveBase {
// Step 3. Complete the 2nd stage to encoding.
chunk.append(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded,
uriCharset));
- // Decoded and normalized URI
- // Rewriting may have denormalized the URL
- urlStringRewriteEncoded =
RequestUtil.normalize(urlStringRewriteEncoded);
+ // Rewriting may have denormalized the URL and added
encoded characters
+ // Decode then normalize
+ String urlStringRewriteDecoded =
URLDecoder.decode(urlStringRewriteEncoded, uriCharset);
+ urlStringRewriteDecoded =
RequestUtil.normalize(urlStringRewriteDecoded);
request.getCoyoteRequest().decodedURI().setChars(MessageBytes.EMPTY_CHAR_ARRAY,
0, 0);
chunk =
request.getCoyoteRequest().decodedURI().getCharChunk();
if (context) {
// This is decoded and normalized
chunk.append(request.getServletContext().getContextPath());
}
- chunk.append(URLDecoder.decode(urlStringRewriteEncoded,
uriCharset));
- // Set the new Query if there is one
- if (queryStringRewriteEncoded != null) {
+ chunk.append(urlStringRewriteDecoded);
+ // Set the new Query String
+ if (queryStringRewriteEncoded == null) {
+ // No new query string. Therefore the original is
retained unless QSD is defined.
+ if (qsd) {
+
request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY,
0, 0);
+ }
+ } else {
+ // New query string. Therefore the original is dropped
unless QSA is defined (and QSD is not).
request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY,
0, 0);
chunk =
request.getCoyoteRequest().queryString().getCharChunk();
chunk.append(REWRITE_QUERY_ENCODER.encode(queryStringRewriteEncoded,
uriCharset));
- if (qsa && originalQueryStringEncoded != null &&
!originalQueryStringEncoded.isEmpty()) {
+ if (qsa && queryStringOriginalEncoded != null &&
!queryStringOriginalEncoded.isEmpty()) {
chunk.append('&');
- chunk.append(originalQueryStringEncoded);
+ chunk.append(queryStringOriginalEncoded);
}
}
// Set the new host if it changed
@@ -666,6 +673,10 @@ public class RewriteValve extends ValveBase {
while (flagsTokenizer.hasMoreElements()) {
parseRuleFlag(line, rule, flagsTokenizer.nextToken());
}
+ // If QSD and QSA are present, QSD always takes precedence
+ if (rule.isQsdiscard()) {
+ rule.setQsappend(false);
+ }
}
return rule;
} else if (token.equals("RewriteMap")) {
diff --git a/test/org/apache/catalina/startup/TomcatBaseTest.java
b/test/org/apache/catalina/startup/TomcatBaseTest.java
index a655b28cf4..9ed9e59294 100644
--- a/test/org/apache/catalina/startup/TomcatBaseTest.java
+++ b/test/org/apache/catalina/startup/TomcatBaseTest.java
@@ -543,7 +543,7 @@ public abstract class TomcatBaseTest extends
LoggingBaseTest {
value.append(';');
}
}
- out.println("PARAM/" + name + ": " + value);
+ out.println("PARAM:" + name + ": " + value);
}
out.println("SESSION-REQUESTED-ID: " +
diff --git a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
index ce2f7c5d5c..beb6835d29 100644
--- a/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
+++ b/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
@@ -301,17 +301,112 @@ public class TestRewriteValve extends TomcatBaseTest {
}
@Test
- public void testQueryString() throws Exception {
+ public void testQueryStringTargetOnly() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?je=2", "/b/id=1", "/c/id=1",
"je=2");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQSA() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?je=2 [QSA]", "/b/id=1",
"/c/id=1", "je=2");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?je=2 [QSD]", "/b/id=1",
"/c/id=1", "je=2");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQSAQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?je=2 [QSA,QSD]", "/b/id=1",
"/c/id=1", "je=2");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQS() throws Exception {
doTestRewrite("RewriteRule ^/b/(.*) /c?$1", "/b/id=1", "/c", "id=1");
}
+ @Test
+ public void testQueryStringTargetOnlyQSAQS() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c?$1 [QSA]", "/b/id=1", "/c",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQSDQS() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c?$1 [QSD]", "/b/id=1", "/c",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringTargetOnlyQSAQSDQS() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c?$1 [QSA,QSD]", "/b/id=1", "/c",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringSourceOnly() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1", "/b/d?id=1", "/c/d",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringSourceOnlyQSA() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [QSA]", "/b/d?id=1", "/c/d",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringSourceOnlyQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [QSD]", "/b/d?id=1", "/c/d",
null);
+ }
+
+ @Test
+ public void testQueryStringSourceOnlyQSAQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [QSA,QSD]", "/b/d?id=1",
"/c/d", null);
+ }
+
+ @Test
+ public void testQueryStringSourceAndTarget() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?id=1", "/b/d?je=2", "/c/d",
"id=1");
+ }
+
+ @Test
+ public void testQueryStringSourceAndTargetQSA() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?id=1 [QSA]", "/b/d?je=2",
"/c/d", "id=1&je=2");
+ }
+
+ @Test
+ public void testQueryStringSourceAndTargetQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?id=1 [QSD]", "/b/d?je=2",
"/c/d", "id=1");
+ }
+
+ @Test
+ public void testQueryStringSourceAndTargetQSAQSD() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?id=1 [QSA,QSD]",
"/b/d?je=2", "/c/d", "id=1");
+ }
+
+ @Test
+ public void testQueryStringEncoded01() throws Exception {
+ doTestRewrite("RewriteCond %{QUERY_STRING} a=(.*)\nRewriteRule ^/b.*$
/%1 [QSD]", "/b?a=c", "/c", null);
+ }
+
+ @Test
+ public void testQueryStringEncoded02() throws Exception {
+ doTestRewrite("RewriteCond %{QUERY_STRING} a=(.*)\nRewriteRule ^/b.*$
/z/%1 [QSD]", "/b?a=%2e%2e%2fc%2faAbB", "/z/%2e%2e%2fc%2faAbB", null);
+ }
+
@Test
public void testQueryStringRemove() throws Exception {
- doTestRewrite("RewriteRule ^/b/(.*) /c/$1?", "/b/d?=1", "/c/d", null);
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?", "/b/d?id=1", "/c/d",
null);
}
@Test
public void testQueryStringRemove02() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [QSD]", "/b/d?id=1", "/c/d",
null);
+ }
+
+ @Test
+ public void testQueryStringRemoveInvalid() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?", "/b/d?=1", "/c/d", null);
+ }
+
+ @Test
+ public void testQueryStringRemoveInvalid02() throws Exception {
doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [QSD]", "/b/d?=1", "/c/d",
null);
}
@@ -616,7 +711,7 @@ public class TestRewriteValve extends TomcatBaseTest {
public void testFlagsNC() throws Exception {
// https://bz.apache.org/bugzilla/show_bug.cgi?id=60116
doTestRewrite("RewriteCond %{QUERY_STRING} a=([a-z]*) [NC]\n" +
"RewriteRule .* - [E=X-Test:%1]", "/c?a=aAa",
- "/c", null, "aAa");
+ "/c", "a=aAa", "aAa");
}
@Test
@@ -806,12 +901,16 @@ public class TestRewriteValve extends TomcatBaseTest {
// were written into the request target
Assert.assertEquals(400, rc);
} else {
+ // If there is an expected URI, the request should be successful
+ Assert.assertEquals(200, rc);
String body = res.toString();
RequestDescriptor requestDesc = SnoopResult.parse(body);
String requestURI = requestDesc.getRequestInfo("REQUEST-URI");
Assert.assertEquals(expectedURI, requestURI);
- if (expectedQueryString != null) {
+ if (expectedQueryString == null) {
+ Assert.assertTrue(requestDesc.getParams().isEmpty());
+ } else {
String queryString =
requestDesc.getRequestInfo("REQUEST-QUERY-STRING");
Assert.assertEquals(expectedQueryString, queryString);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d4f9d123d6..89b4bbcd17 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -117,6 +117,10 @@
when the store was used with the <code>PersistentValve</code>. Based on
pull request <pr>882</pr> by Aaron Ogburn. (markt)
</fix>
+ <fix>
+ Fix handling of <code>QSA</code> and <code>QSD</code> flags in
+ <code>RewriteValve</code>. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
