You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/09/12 16:00:27 UTC
svn commit: r1760397 - in /tomcat/trunk:
java/org/apache/catalina/valves/rewrite/RewriteValve.java
test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
webapps/docs/changelog.xml
Author: markt
Date: Mon Sep 12 16:00:27 2016
New Revision: 1760397
URL: http://svn.apache.org/viewvc?rev=1760397&view=rev
Log:
Follow-up for https://bz.apache.org/bugzilla/show_bug.cgi?id=60013
Fix some long lines and remove multiple calls to request.getConnector().getURIEncoding()
Improve handling for QSA.
Includes a test case provided by Santhana Preethi
Modified:
tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java?rev=1760397&r1=1760396&r2=1760397&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java Mon Sep 12 16:00:27 2016
@@ -323,12 +323,16 @@ public class RewriteValve extends ValveB
// As long as MB isn't a char sequence or affiliated, this has to be
// converted to a string
- MessageBytes urlMB = context ? request.getRequestPathMB() : request.getDecodedRequestURIMB();
+ String uriEncoding = request.getConnector().getURIEncoding();
+ String originalQueryStringEncoded = request.getQueryString();
+ MessageBytes urlMB =
+ context ? request.getRequestPathMB() : request.getDecodedRequestURIMB();
urlMB.toChars();
CharSequence urlDecoded = urlMB.getCharChunk();
CharSequence host = request.getServerName();
boolean rewritten = false;
boolean done = false;
+ boolean qsa = false;
for (int i = 0; i < rules.length; i++) {
RewriteRule rule = rules[i];
CharSequence test = (rule.isHost()) ? host : urlDecoded;
@@ -346,6 +350,13 @@ public class RewriteValve extends ValveB
rewritten = true;
}
+ // Check QSA before the final reply
+ if (!qsa && newtest != null && rule.isQsappend()) {
+ // TODO: This logic will need some tweaks if we add QSD
+ // support
+ qsa = true;
+ }
+
// Final reply
// - forbidden
@@ -360,10 +371,11 @@ public class RewriteValve extends ValveB
done = true;
break;
}
+
// - redirect (code)
if (rule.isRedirect() && newtest != null) {
- // append the query string to the url if there is one and it hasn't been rewritten
- String originalQueryStringEncoded = request.getQueryString();
+ // Append the query string to the url if there is one and it
+ // hasn't been rewritten
String urlStringDecoded = urlDecoded.toString();
int index = urlStringDecoded.indexOf("?");
String rewrittenQueryStringDecoded;
@@ -374,16 +386,19 @@ public class RewriteValve extends ValveB
urlStringDecoded = urlStringDecoded.substring(0, index);
}
- StringBuffer urlStringEncoded = new StringBuffer(ENCODER.encode(urlStringDecoded, request.getConnector().getURIEncoding()));
- if (originalQueryStringEncoded != null && originalQueryStringEncoded.length() > 0) {
+ StringBuffer urlStringEncoded =
+ new StringBuffer(ENCODER.encode(urlStringDecoded, uriEncoding));
+ if (originalQueryStringEncoded != null &&
+ originalQueryStringEncoded.length() > 0) {
if (rewrittenQueryStringDecoded == null) {
urlStringEncoded.append('?');
urlStringEncoded.append(originalQueryStringEncoded);
} else {
- if (rule.isQsappend()) {
+ if (qsa) {
// if qsa is specified append the query
urlStringEncoded.append('?');
- urlStringEncoded.append(ENCODER.encode(rewrittenQueryStringDecoded, request.getConnector().getURIEncoding()));
+ urlStringEncoded.append(
+ ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
urlStringEncoded.append('&');
urlStringEncoded.append(originalQueryStringEncoded);
} else if (index == urlStringEncoded.length() - 1) {
@@ -392,23 +407,27 @@ public class RewriteValve extends ValveB
urlStringEncoded.deleteCharAt(index);
} else {
urlStringEncoded.append('?');
- urlStringEncoded.append(ENCODER.encode(rewrittenQueryStringDecoded, request.getConnector().getURIEncoding()));
+ urlStringEncoded.append(
+ ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
}
}
} else if (rewrittenQueryStringDecoded != null) {
urlStringEncoded.append('?');
- urlStringEncoded.append(ENCODER.encode(rewrittenQueryStringDecoded, request.getConnector().getURIEncoding()));
+ urlStringEncoded.append(
+ ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
}
// Insert the context if
// 1. this valve is associated with a context
// 2. the url starts with a leading slash
// 3. the url isn't absolute
- if (context && urlStringEncoded.charAt(0) == '/' && !UriUtil.hasScheme(urlStringEncoded)) {
+ if (context && urlStringEncoded.charAt(0) == '/' &&
+ !UriUtil.hasScheme(urlStringEncoded)) {
urlStringEncoded.insert(0, request.getContext().getEncodedPath());
}
if (rule.isNoescape()) {
- response.sendRedirect(URLDecoder.decode(urlStringEncoded.toString(), request.getConnector().getURIEncoding()));
+ response.sendRedirect(
+ URLDecoder.decode(urlStringEncoded.toString(), uriEncoding));
} else {
response.sendRedirect(urlStringEncoded.toString());
}
@@ -441,14 +460,6 @@ public class RewriteValve extends ValveB
if (rule.isType() && newtest != null) {
request.setContentType(rule.getTypeValue());
}
- // - qsappend
- if (rule.isQsappend() && newtest != null) {
- String queryString = request.getQueryString();
- String urlString = urlDecoded.toString();
- if (urlString.indexOf('?') != -1 && queryString != null) {
- urlDecoded = urlString + "&" + queryString;
- }
- }
// Control flow processing
@@ -501,7 +512,7 @@ public class RewriteValve extends ValveB
// This is neither decoded nor normalized
chunk.append(contextPath);
}
- chunk.append(ENCODER.encode(urlStringDecoded, request.getConnector().getURIEncoding()));
+ chunk.append(ENCODER.encode(urlStringDecoded, uriEncoding));
request.getCoyoteRequest().requestURI().toChars();
// Decoded and normalized URI
// Rewriting may have denormalized the URL
@@ -520,8 +531,14 @@ public class RewriteValve extends ValveB
request.getCoyoteRequest().queryString().setString(null);
chunk = request.getCoyoteRequest().queryString().getCharChunk();
chunk.recycle();
- chunk.append(ENCODER.encode(queryStringDecoded, request.getConnector().getURIEncoding()));
- request.getCoyoteRequest().queryString().toChars();
+ chunk.append(ENCODER.encode(queryStringDecoded, uriEncoding));
+ if (qsa) {
+ chunk.append('&');
+ chunk.append(originalQueryStringEncoded);
+ }
+ if (!chunk.isNull()) {
+ request.getCoyoteRequest().queryString().toChars();
+ }
}
// Set the new host if it changed
if (!host.equals(request.getServerName())) {
Modified: tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java?rev=1760397&r1=1760396&r2=1760397&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java (original)
+++ tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java Mon Sep 12 16:00:27 2016
@@ -131,6 +131,11 @@ public class TestRewriteValve extends To
}
@Test
+ public void testQueryStringRemove() throws Exception {
+ doTestRewrite("RewriteRule ^/b/(.*) /c/$1?", "/b/d?=1", "/c/d", null);
+ }
+
+ @Test
public void testNonAsciiQueryString() throws Exception {
doTestRewrite("RewriteRule ^/b/(.*) /c?$1",
"/b/id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95",
@@ -237,7 +242,7 @@ public class TestRewriteValve extends To
// Note %C2%A1 == \u00A1
doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B,QSA]",
"/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/%C2%A1%25C2%25A1",
- "id=%25C2%25A1&di=%25C2%25AE");
+ "id=%25C2%25A1&di=%C2%AE");
}
@@ -357,6 +362,15 @@ public class TestRewriteValve extends To
}
+ @Test
+ public void testUtf8WithBothQsFlagsQSA() throws Exception {
+ // Note %C2%A1 == \u00A1
+ doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [QSA]",
+ "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/%C2%A1%C2%A1",
+ "id=%C2%A1&di=%C2%AE");
+ }
+
+
@Test
public void testUtf8WithRewriteQsFlagsRB() throws Exception {
// Note %C2%A1 == \u00A1
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1760397&r1=1760396&r2=1760397&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Sep 12 16:00:27 2016
@@ -55,7 +55,9 @@
<fix>
<bug>60013</bug>: Refactor the previous fix to align the behaviour of
the Rewrite Valve with mod_rewite. As part of this, provide an
- implementation for the <code>B</code> and <code>NE</code> flags. (markt)
+ implementation for the <code>B</code> and <code>NE</code> flags and
+ improve the handling for the <code>QSA</code> flag. Includes multiple
+ test cases by Santhana Preethi. (markt)
</fix>
</changelog>
</subsection>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org