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/08/30 16:43:33 UTC

svn commit: r1758425 - 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: Tue Aug 30 16:43:33 2016
New Revision: 1758425

URL: http://svn.apache.org/viewvc?rev=1758425&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60013
Fix encoding issues when using the RewriteValve with UTF-8 query strings or UTF-8 redirect URLs.

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=1758425&r1=1758424&r2=1758425&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java Tue Aug 30 16:43:33 2016
@@ -53,6 +53,15 @@ import org.apache.tomcat.util.http.Reque
 
 public class RewriteValve extends ValveBase {
 
+    private static final URLEncoder QUERY_STRING_ENCODER;
+
+    static {
+        QUERY_STRING_ENCODER = new URLEncoder();
+        QUERY_STRING_ENCODER.addSafeCharacter('=');
+        QUERY_STRING_ENCODER.addSafeCharacter('&');
+    }
+
+
     /**
      * The rewrite rules that the valve will use.
      */
@@ -323,15 +332,26 @@ public class RewriteValve extends ValveB
                 // - 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 queryString = request.getQueryString();
+                    String originalQueryString = request.getQueryString();
                     StringBuffer urlString = new StringBuffer(url);
-                    if (queryString != null && queryString.length() > 0) {
-                        int index = urlString.indexOf("?");
+                    int index = urlString.indexOf("?");
+                    String encodedUrl;
+                    if (index == -1) {
+                        encodedUrl = URLEncoder.DEFAULT.encode(urlString.toString(), "UTF-8");
+                        urlString.setLength(0);
+                        urlString.append(encodedUrl);
+                    } else {
+                        encodedUrl = URLEncoder.DEFAULT.encode(
+                                urlString.substring(0, index), "UTF-8");
+                        urlString.delete(0, index);
+                        urlString.insert(0, encodedUrl);
+                    }
+                    if (originalQueryString != null && originalQueryString.length() > 0) {
                         if (index != -1) {
                             // if qsa is specified append the query
                             if (rule.isQsappend()) {
                                 urlString.append('&');
-                                urlString.append(queryString);
+                                urlString.append(originalQueryString);
                             }
                             // if the ? is the last character delete it, its only purpose was to
                             // prevent the rewrite module from appending the query string
@@ -340,9 +360,10 @@ public class RewriteValve extends ValveB
                             }
                         } else {
                             urlString.append('?');
-                            urlString.append(queryString);
+                            urlString.append(originalQueryString);
                         }
                     }
+
                     // Insert the context if
                     // 1. this valve is associated with a context
                     // 2. the url starts with a leading slash
@@ -451,10 +472,13 @@ public class RewriteValve extends ValveB
                     request.getCoyoteRequest().decodedURI().toChars();
                     // Set the new Query if there is one
                     if (queryString != null) {
+                        // TODO: This isn't perfect. There are some edge cases
+                        //       that can only be handled if RewriteValve works
+                        //       with the original (undecoded) URI
                         request.getCoyoteRequest().queryString().setString(null);
                         chunk = request.getCoyoteRequest().queryString().getCharChunk();
                         chunk.recycle();
-                        chunk.append(queryString);
+                        chunk.append(QUERY_STRING_ENCODER.encode(queryString, "UTF-8"));
                         request.getCoyoteRequest().queryString().toChars();
                     }
                     // Set the new host if it changed

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=1758425&r1=1758424&r2=1758425&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java (original)
+++ tomcat/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java Tue Aug 30 16:43:33 2016
@@ -114,6 +114,31 @@ public class TestRewriteValve extends To
         doTestRewrite(config, request, expectedURI, null);
     }
 
+    @Test
+    public void testNonAsciiPath() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*) /c/$1", "/b/%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95",
+                "/c/%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
+    }
+
+    @Test
+    public void testNonAsciiPathRedirect() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*) /c/$1 [R]",
+                "/b/%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95",
+                "/c/%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
+    }
+
+    @Test
+    public void testQueryString() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*) /c?$1", "/b/id=1", "/c", "id=1");
+    }
+
+    @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",
+                "/c", "id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
+    }
+
+
     private void doTestRewrite(String config, String request, String expectedURI,
             String expectedQueryString) throws Exception {
 
@@ -127,13 +152,11 @@ public class TestRewriteValve extends To
 
         rewriteValve.setConfiguration(config);
 
-        // Note: URLPatterns should be URL encoded
-        //       (http://svn.apache.org/r285186)
         Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
-        ctx.addServletMapping("/a/%255A", "snoop");
-        ctx.addServletMapping("/c/*", "snoop");
+        ctx.addServletMappingDecoded("/a/%5A", "snoop");
+        ctx.addServletMappingDecoded("/c/*", "snoop");
         Tomcat.addServlet(ctx, "default", new DefaultServlet());
-        ctx.addServletMapping("/", "default");
+        ctx.addServletMappingDecoded("/", "default");
 
         tomcat.start();
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1758425&r1=1758424&r2=1758425&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug 30 16:43:33 2016
@@ -168,6 +168,9 @@
         placed into the <code>FAILED</code> state. (markt)
       </fix>
       <fix>
+        <bug>60013</bug>: 
+      </fix>
+      <fix>
         <bug>60022</bug>: Improve handling when a WAR file and/or the associated
         exploded directory are symlinked into the <code>appBase</code>. (markt)
       </fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org