Apache Tomcat: Bypass of rules in Rewrite Valve

@@ -552,7 +552,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
                         // This is decoded and normalized
                         chunk.append(request.getServletContext().getContextPath());
                     }
-                    chunk.append(URLDecoder.decode(urlStringRewriteEncoded, uriCharset));
+                    chunk.append(URLDecoder.decode(urlStringRewriteEncoded, uriCharset.name()));
                     // Set the new Query if there is one
                     if (queryStringRewriteEncoded != null) {
                         request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);

@@ -650,18 +650,17 @@ protected boolean postParseRequest(org.apache.coyote.Request req, Request reques
             } else {
                 /*
                  * The URI is chars or String, and has been sent using an in-memory protocol handler. The following
-                 * assumptions are made: - req.requestURI() has been set to the 'original' non-decoded, non-normalized
-                 * URI - req.decodedURI() has been set to the decoded, normalized form of req.requestURI() -
-                 * 'suspicious' URI filtering - if required - has already been performed
+                 * assumptions are made:
+                 *
+                 * - req.requestURI() has been set to the 'original' non-decoded, non-normalized URI that includes path
+                 * parameters (if any)
+                 *
+                 * - req.decodedURI() has been set to the decoded, normalized form of req.requestURI() with any path
+                 * parameters removed
+                 *
+                 * - 'suspicious' URI filtering, if required, has already been performed
                  */
                 decodedURI.toChars();
-                // Remove all path parameters; any needed path parameter should be set
-                // using the request object rather than passing it in the URL
-                CharChunk uriCC = decodedURI.getCharChunk();
-                int semicolon = uriCC.indexOf(';');
-                if (semicolon > 0) {
-                    decodedURI.setChars(uriCC.getBuffer(), uriCC.getStart(), semicolon);
-                }
             }
         }
 

@@ -21,6 +21,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -65,6 +66,24 @@
  */
 public class RewriteValve extends ValveBase {
 
+    private static final URLEncoder REWRITE_DEFAULT_ENCODER;
+    private static final URLEncoder REWRITE_QUERY_ENCODER;
+
+    static {
+        /*
+         * See the detailed explanation of encoding/decoding during URL re-writing in the invoke() method.
+         *
+         * These encoders perform the second stage of encoding, after re-writing has completed. These rewrite specific
+         * encoders treat '%' as a safe character so that URLs and query strings already processed by encodeForRewrite()
+         * do not end up with double encoding of '%' characters.
+         */
+        REWRITE_DEFAULT_ENCODER = (URLEncoder) URLEncoder.DEFAULT.clone();
+        REWRITE_DEFAULT_ENCODER.addSafeCharacter('%');
+
+        REWRITE_QUERY_ENCODER = (URLEncoder) URLEncoder.QUERY.clone();
+        REWRITE_QUERY_ENCODER.addSafeCharacter('%');
+    }
+
     /**
      * The rewrite rules that the valve will use.
      */
@@ -296,22 +315,51 @@ public void invoke(Request request, Response response) throws IOException, Servl
 
             invoked.set(Boolean.TRUE);
 
-            // As long as MB isn't a char sequence or affiliated, this has to be
-            // converted to a string
+            // 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();
             MessageBytes urlMB = context ? request.getRequestPathMB() : request.getDecodedRequestURIMB();
             urlMB.toChars();
             CharSequence urlDecoded = urlMB.getCharChunk();
+
+            /*
+             * The URL presented to the rewrite valve is the URL that is used for request mapping. That URL has been
+             * processed to: remove path parameters; remove the query string; decode; and normalize the URL. It may
+             * contain literal '%', '?' and/or ';' characters at this point.
+             *
+             * The re-write rules need to be able to process URLs with literal '?' characters and add query strings
+             * without the two becoming confused. The re-write rules also need to be able to insert literal '%'
+             * characters without them being confused with %nn encoding.
+             *
+             * The re-write rules cannot insert path parameters.
+             *
+             * To meet these requirement, the URL is processed as follows.
+             *
+             * Step 1. The URL is partially re-encoded by encodeForRewrite(). This method encodes any literal '%', ';'
+             * and/or '?' characters in the URL using the standard %nn form.
+             *
+             * Step 2. The re-write processing runs with the provided re-write rules against the partially encoded URL.
+             * If a re-write rule needs to insert a literal '%', ';' or '?', it must do so in %nn encoded form.
+             *
+             * Step 3. The URL (and query string if present) is re-encoded using the re-write specific encoders
+             * (REWRITE_DEFAULT_ENCODER and REWRITE_QUERY_ENCODER) that behave the same was as the standard encoders
+             * apart from '%' being treated as a safe character. This prevents double encoding of any '%' characters
+             * present in the URL from steps 1 or 2.
+             */
+
+            // Step 1. Encode URL for processing by the re-write rules.
+            CharSequence urlRewriteEncoded = encodeForRewrite(urlDecoded);
             CharSequence host = request.getServerName();
             boolean rewritten = false;
             boolean done = false;
             boolean qsa = false;
             boolean qsd = false;
             boolean valveSkip = false;
+
+            // Step 2. Process the URL using the re-write rules.
             for (int i = 0; i < rules.length; i++) {
                 RewriteRule rule = rules[i];
-                CharSequence test = (rule.isHost()) ? host : urlDecoded;
+                CharSequence test = (rule.isHost()) ? host : urlRewriteEncoded;
                 CharSequence newtest = rule.evaluate(test, resolver);
                 if (newtest != null && !Objects.equals(test.toString(), newtest.toString())) {
                     if (containerLog.isTraceEnabled()) {
@@ -321,7 +369,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
                     if (rule.isHost()) {
                         host = newtest;
                     } else {
-                        urlDecoded = newtest;
+                        urlRewriteEncoded = newtest;
                     }
                     rewritten = true;
                 }
@@ -358,28 +406,30 @@ public void invoke(Request request, Response response) throws IOException, Servl
                 if (rule.isRedirect() && newtest != null) {
                     // 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;
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    int index = urlStringRewriteEncoded.indexOf('?');
+                    String rewrittenQueryStringRewriteEncoded;
                     if (index == -1) {
-                        rewrittenQueryStringDecoded = null;
+                        rewrittenQueryStringRewriteEncoded = null;
                     } else {
-                        rewrittenQueryStringDecoded = urlStringDecoded.substring(index + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, index);
+                        rewrittenQueryStringRewriteEncoded = urlStringRewriteEncoded.substring(index + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, index);
                     }
 
+                    // Step 3. Complete the 2nd stage to encoding.
                     StringBuilder urlStringEncoded =
-                            new StringBuilder(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+                            new StringBuilder(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded, uriCharset));
+
                     if (!qsd && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
-                        if (rewrittenQueryStringDecoded == null) {
+                        if (rewrittenQueryStringRewriteEncoded == null) {
                             urlStringEncoded.append('?');
                             urlStringEncoded.append(originalQueryStringEncoded);
                         } else {
                             if (qsa) {
                                 // if qsa is specified append the query
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                                 urlStringEncoded.append('&');
                                 urlStringEncoded.append(originalQueryStringEncoded);
                             } else if (index == urlStringEncoded.length() - 1) {
@@ -388,13 +438,14 @@ public void invoke(Request request, Response response) throws IOException, Servl
                                 urlStringEncoded.deleteCharAt(index);
                             } else {
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                             }
                         }
-                    } else if (rewrittenQueryStringDecoded != null) {
+                    } else if (rewrittenQueryStringRewriteEncoded != null) {
                         urlStringEncoded.append('?');
-                        urlStringEncoded.append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                        urlStringEncoded
+                                .append(REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                     }
 
                     // Insert the context if
@@ -469,12 +520,12 @@ public void invoke(Request request, Response response) throws IOException, Servl
             if (rewritten) {
                 if (!done) {
                     // See if we need to replace the query string
-                    String urlStringDecoded = urlDecoded.toString();
-                    String queryStringDecoded = null;
-                    int queryIndex = urlStringDecoded.indexOf('?');
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    String queryStringRewriteEncoded = null;
+                    int queryIndex = urlStringRewriteEncoded.indexOf('?');
                     if (queryIndex != -1) {
-                        queryStringDecoded = urlStringDecoded.substring(queryIndex + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, queryIndex);
+                        queryStringRewriteEncoded = urlStringRewriteEncoded.substring(queryIndex + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, queryIndex);
                     }
                     // Save the current context path before re-writing starts
                     String contextPath = null;
@@ -488,22 +539,24 @@ public void invoke(Request request, Response response) throws IOException, Servl
                         // This is neither decoded nor normalized
                         chunk.append(contextPath);
                     }
-                    chunk.append(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+
+                    // 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
-                    urlStringDecoded = RequestUtil.normalize(urlStringDecoded);
+                    urlStringRewriteEncoded = RequestUtil.normalize(urlStringRewriteEncoded);
                     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(urlStringDecoded);
+                    chunk.append(URLDecoder.decode(urlStringRewriteEncoded, uriCharset));
                     // Set the new Query if there is one
-                    if (queryStringDecoded != null) {
+                    if (queryStringRewriteEncoded != null) {
                         request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
                         chunk = request.getCoyoteRequest().queryString().getCharChunk();
-                        chunk.append(URLEncoder.QUERY.encode(queryStringDecoded, uriCharset));
+                        chunk.append(REWRITE_QUERY_ENCODER.encode(queryStringRewriteEncoded, uriCharset));
                         if (qsa && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
                             chunk.append('&');
                             chunk.append(originalQueryStringEncoded);
@@ -790,4 +843,31 @@ protected static void parseRuleFlag(String line, RewriteRule rule, String flag)
             throw new IllegalArgumentException(sm.getString("rewriteValve.invalidFlags", line, flag));
         }
     }
+
+
+    private CharSequence encodeForRewrite(CharSequence input) {
+        StringBuilder result = null;
+        int pos = 0;
+        int mark = 0;
+        while (pos < input.length()) {
+            char c = input.charAt(pos);
+            if (c == '%' || c == ';' || c == '?') {
+                if (result == null) {
+                    result = new StringBuilder((int) (input.length() * 1.1));
+                }
+                result.append(input.subSequence(mark, pos));
+                result.append('%');
+                result.append(Character.forDigit((c >> 4) & 0xF, 16));
+                result.append(Character.forDigit(c & 0xF, 16));
+                mark = pos + 1;
+            }
+            pos++;
+        }
+        if (result != null) {
+            result.append(input.subSequence(mark, input.length()));
+            return result;
+        } else {
+            return input;
+        }
+    }
 }

@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.HttpURLConnection;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -63,7 +64,7 @@ public void testNoRewrite() throws Exception {
 
     @Test
     public void testBackslashPercentSign() throws Exception {
-        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%255A");
+        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%5A");
     }
 
     @Test
@@ -142,7 +143,7 @@ public void testRewriteMap09() throws Exception {
 
     @Test
     public void testRewriteMap10() throws Exception {
-        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%20aa");
     }
 
     @Test
@@ -346,16 +347,16 @@ public void testNonAsciiQueryStringAndPathAndRedirect() throws Exception {
     public void testNonAsciiQueryStringWithB() throws Exception {
         doTestRewrite("RewriteRule ^/b/(.*)/id=(.*) /c?filename=$1&id=$2 [B]",
                 "/b/file01/id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95", "/c",
-                "filename=file01&id=%25E5%259C%25A8%25E7%25BA%25BF%25E6%25B5%258B%25E8%25AF%2595");
+                "filename=file01&id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
     }
 
 
     @Test
     public void testNonAsciiQueryStringAndPathAndRedirectWithB() throws Exception {
         // Note the double encoding of the result (httpd produces the same result)
         doTestRewrite("RewriteRule ^/b/(.*)/(.*)/id=(.*) /c/$1?filename=$2&id=$3 [B,R]",
-                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%25E5%259C%25A8%25E7%25BA%25BF",
-                "filename=file01&id=%25E6%25B5%258B%25E8%25AF%2595");
+                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%E5%9C%A8%E7%BA%BF",
+                "filename=file01&id=%E6%B5%8B%E8%AF%95");
     }
 
 
@@ -371,7 +372,7 @@ public void testUtf8WithBothQsFlagsNone() throws Exception {
     public void testUtf8WithBothQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -387,7 +388,7 @@ public void testUtf8WithBothQsFlagsR() throws Exception {
     public void testUtf8WithBothQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -413,7 +414,7 @@ public void testUtf8WithBothQsFlagsRBNE() throws Exception {
     public void testUtf8WithBothQsFlagsBQSA() throws Exception {
         // 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=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -429,7 +430,7 @@ public void testUtf8WithBothQsFlagsRQSA() throws Exception {
     public void testUtf8WithBothQsFlagsRBQSA() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,QSA]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1&di=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -461,7 +462,7 @@ public void testUtf8WithOriginalQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -476,7 +477,7 @@ public void testUtf8WithOriginalQsFlagsR() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -510,8 +511,8 @@ public void testUtf8WithRewriteQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -534,8 +535,8 @@ public void testUtf8WithBothQsFlagsQSA() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -575,7 +576,7 @@ public void testUtf8FlagsNone() throws Exception {
     @Test
     public void testUtf8FlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -589,7 +590,7 @@ public void testUtf8FlagsR() throws Exception {
     @Test
     public void testUtf8FlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -784,6 +785,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
         rewriteValve.setConfiguration(config);
 
         Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/a/Z", "snoop");
         ctx.addServletMappingDecoded("/a/%5A", "snoop");
         ctx.addServletMappingDecoded("/c/*", "snoop");
         ctx.addServletMappingDecoded("/W/*", "snoop");
@@ -929,4 +931,87 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
             }
         }
     }
+
+
+    @Test
+    public void testEncodedUriSimple() throws Exception {
+        doTestRewriteWithEncoding("aaa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark01() throws Exception {
+        doTestRewriteWithEncoding("a%3fa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark02() throws Exception {
+        doTestRewriteWithEncoding("%3faa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark03() throws Exception {
+        doTestRewriteWithEncoding("aa%3f");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMarkAndQueryString() throws Exception {
+        doTestRewriteWithEncoding("a%3fa?b=c", "a%3fa", "b=c");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon01() throws Exception {
+        doTestRewriteWithEncoding("a%3ba");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon02() throws Exception {
+        doTestRewriteWithEncoding("%3baa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon03() throws Exception {
+        doTestRewriteWithEncoding("aa%3b");
+    }
+
+
+    private void doTestRewriteWithEncoding(String segment) throws Exception {
+        doTestRewriteWithEncoding(segment, segment, null);
+    }
+
+    private void doTestRewriteWithEncoding(String segment, String expectedSegment, String expectedQueryString)
+            throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        RewriteValve rewriteValve = new RewriteValve();
+        tomcat.getHost().getPipeline().addValve(rewriteValve);
+
+        rewriteValve.setConfiguration("RewriteRule ^/source/(.*)$ /target/$1");
+
+        Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/target/*", "snoop");
+
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort() + "/source/" + segment, res, false);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+
+        res.setCharset(StandardCharsets.UTF_8);
+        String body = res.toString();
+        Assert.assertTrue(body, body.contains("REQUEST-URI: /target/" + expectedSegment));
+        Assert.assertTrue(body, body.contains("PATH-INFO: /" +
+                URLDecoder.decode(expectedSegment, StandardCharsets.UTF_8)));
+        Assert.assertTrue(body, body.contains("REQUEST-QUERY-STRING: " + expectedQueryString));
+    }
 }

@@ -171,6 +171,11 @@
         Fix stack trace trimming in <code>ErrorReportValve</code> after removal
         of the security manager support. (remm)
       </fix>
+      <fix>
+        Improve the handling of <code>%nn</code> URL encoding in the
+        RewriteValve and document how <code>%nn</code> URL encoding may be used
+        with rewrite rules. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -56,6 +56,28 @@
 
 </section>
 
+<section name="Using rewrite rules with special characters">
+
+  <p>The URL presented to the rewrite valve is the same URL used for request
+  mapping with any literal <code>'%'</code>, <code>';'</code> and/or
+  <code>'?'</code> characters encoded in <code>%nn</code> form.</p>
+
+  <p>A rewrite rule that wishes to insert a literal <code>'%'</code>,
+  <code>';'</code>, <code>'?'</code>, <code>'&amp;'</code> or <code>'='</code>
+  character should do so in <code>%nn</code> form. Other characters maybe
+  inserted in either literal or <code>%nn</code> form.</p>
+
+  <p>This enables the rewrite rules to:
+  <ul>
+  <li>process URLs containing literal <code>'?'</code> characters;</li>
+  <li>add a query string;</li>
+  <li>insert a literal <code>'%'</code> character without it being confused with
+  <code>%nn</code> encoding.</li>
+  </ul>
+  </p>
+
+</section>
+
 <section name="Directives">
 
   <p>The rewrite.config file contains a list of directives which closely

@@ -653,17 +653,17 @@ protected boolean postParseRequest(org.apache.coyote.Request req, Request reques
             } else {
                 /*
                  * The URI is chars or String, and has been sent using an in-memory protocol handler. The following
-                 * assumptions are made: - req.requestURI() has been set to the 'original' non-decoded, non-normalized
-                 * URI - req.decodedURI() has been set to the decoded, normalized form of req.requestURI()
+                 * assumptions are made:
+                 *
+                 * - req.requestURI() has been set to the 'original' non-decoded, non-normalized URI that includes path
+                 * parameters (if any)
+                 *
+                 * - req.decodedURI() has been set to the decoded, normalized form of req.requestURI() with any path
+                 * parameters removed
+                 *
+                 * - 'suspicious' URI filtering, if required, has already been performed
                  */
                 decodedURI.toChars();
-                // Remove all path parameters; any needed path parameter should be set
-                // using the request object rather than passing it in the URL
-                CharChunk uriCC = decodedURI.getCharChunk();
-                int semicolon = uriCC.indexOf(';');
-                if (semicolon > 0) {
-                    decodedURI.setChars(uriCC.getBuffer(), uriCC.getStart(), semicolon);
-                }
             }
         }
 

@@ -21,6 +21,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -65,6 +66,24 @@
  */
 public class RewriteValve extends ValveBase {
 
+    private static final URLEncoder REWRITE_DEFAULT_ENCODER;
+    private static final URLEncoder REWRITE_QUERY_ENCODER;
+
+    static {
+        /*
+         * See the detailed explanation of encoding/decoding during URL re-writing in the invoke() method.
+         *
+         * These encoders perform the second stage of encoding, after re-writing has completed. These rewrite specific
+         * encoders treat '%' as a safe character so that URLs and query strings already processed by encodeForRewrite()
+         * do not end up with double encoding of '%' characters.
+         */
+        REWRITE_DEFAULT_ENCODER = (URLEncoder) URLEncoder.DEFAULT.clone();
+        REWRITE_DEFAULT_ENCODER.addSafeCharacter('%');
+
+        REWRITE_QUERY_ENCODER = (URLEncoder) URLEncoder.QUERY.clone();
+        REWRITE_QUERY_ENCODER.addSafeCharacter('%');
+    }
+
     /**
      * The rewrite rules that the valve will use.
      */
@@ -297,22 +316,51 @@ public void invoke(Request request, Response response) throws IOException, Servl
 
             invoked.set(Boolean.TRUE);
 
-            // As long as MB isn't a char sequence or affiliated, this has to be
-            // converted to a string
+            // 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();
             MessageBytes urlMB = context ? request.getRequestPathMB() : request.getDecodedRequestURIMB();
             urlMB.toChars();
             CharSequence urlDecoded = urlMB.getCharChunk();
+
+            /*
+             * The URL presented to the rewrite valve is the URL that is used for request mapping. That URL has been
+             * processed to: remove path parameters; remove the query string; decode; and normalize the URL. It may
+             * contain literal '%', '?' and/or ';' characters at this point.
+             *
+             * The re-write rules need to be able to process URLs with literal '?' characters and add query strings
+             * without the two becoming confused. The re-write rules also need to be able to insert literal '%'
+             * characters without them being confused with %nn encoding.
+             *
+             * The re-write rules cannot insert path parameters.
+             *
+             * To meet these requirement, the URL is processed as follows.
+             *
+             * Step 1. The URL is partially re-encoded by encodeForRewrite(). This method encodes any literal '%', ';'
+             * and/or '?' characters in the URL using the standard %nn form.
+             *
+             * Step 2. The re-write processing runs with the provided re-write rules against the partially encoded URL.
+             * If a re-write rule needs to insert a literal '%', ';' or '?', it must do so in %nn encoded form.
+             *
+             * Step 3. The URL (and query string if present) is re-encoded using the re-write specific encoders
+             * (REWRITE_DEFAULT_ENCODER and REWRITE_QUERY_ENCODER) that behave the same was as the standard encoders
+             * apart from '%' being treated as a safe character. This prevents double encoding of any '%' characters
+             * present in the URL from steps 1 or 2.
+             */
+
+            // Step 1. Encode URL for processing by the re-write rules.
+            CharSequence urlRewriteEncoded = encodeForRewrite(urlDecoded);
             CharSequence host = request.getServerName();
             boolean rewritten = false;
             boolean done = false;
             boolean qsa = false;
             boolean qsd = false;
             boolean valveSkip = false;
+
+            // Step 2. Process the URL using the re-write rules.
             for (int i = 0; i < rules.length; i++) {
                 RewriteRule rule = rules[i];
-                CharSequence test = (rule.isHost()) ? host : urlDecoded;
+                CharSequence test = (rule.isHost()) ? host : urlRewriteEncoded;
                 CharSequence newtest = rule.evaluate(test, resolver);
                 if (newtest != null && !Objects.equals(test.toString(), newtest.toString())) {
                     if (containerLog.isTraceEnabled()) {
@@ -322,7 +370,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
                     if (rule.isHost()) {
                         host = newtest;
                     } else {
-                        urlDecoded = newtest;
+                        urlRewriteEncoded = newtest;
                     }
                     rewritten = true;
                 }
@@ -359,28 +407,30 @@ public void invoke(Request request, Response response) throws IOException, Servl
                 if (rule.isRedirect() && newtest != null) {
                     // 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;
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    int index = urlStringRewriteEncoded.indexOf('?');
+                    String rewrittenQueryStringRewriteEncoded;
                     if (index == -1) {
-                        rewrittenQueryStringDecoded = null;
+                        rewrittenQueryStringRewriteEncoded = null;
                     } else {
-                        rewrittenQueryStringDecoded = urlStringDecoded.substring(index + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, index);
+                        rewrittenQueryStringRewriteEncoded = urlStringRewriteEncoded.substring(index + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, index);
                     }
 
+                    // Step 3. Complete the 2nd stage to encoding.
                     StringBuilder urlStringEncoded =
-                            new StringBuilder(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+                            new StringBuilder(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded, uriCharset));
+
                     if (!qsd && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
-                        if (rewrittenQueryStringDecoded == null) {
+                        if (rewrittenQueryStringRewriteEncoded == null) {
                             urlStringEncoded.append('?');
                             urlStringEncoded.append(originalQueryStringEncoded);
                         } else {
                             if (qsa) {
                                 // if qsa is specified append the query
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                                 urlStringEncoded.append('&');
                                 urlStringEncoded.append(originalQueryStringEncoded);
                             } else if (index == urlStringEncoded.length() - 1) {
@@ -389,13 +439,14 @@ public void invoke(Request request, Response response) throws IOException, Servl
                                 urlStringEncoded.deleteCharAt(index);
                             } else {
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                             }
                         }
-                    } else if (rewrittenQueryStringDecoded != null) {
+                    } else if (rewrittenQueryStringRewriteEncoded != null) {
                         urlStringEncoded.append('?');
-                        urlStringEncoded.append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                        urlStringEncoded
+                                .append(REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                     }
 
                     // Insert the context if
@@ -470,12 +521,12 @@ public void invoke(Request request, Response response) throws IOException, Servl
             if (rewritten) {
                 if (!done) {
                     // See if we need to replace the query string
-                    String urlStringDecoded = urlDecoded.toString();
-                    String queryStringDecoded = null;
-                    int queryIndex = urlStringDecoded.indexOf('?');
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    String queryStringRewriteEncoded = null;
+                    int queryIndex = urlStringRewriteEncoded.indexOf('?');
                     if (queryIndex != -1) {
-                        queryStringDecoded = urlStringDecoded.substring(queryIndex + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, queryIndex);
+                        queryStringRewriteEncoded = urlStringRewriteEncoded.substring(queryIndex + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, queryIndex);
                     }
                     // Save the current context path before re-writing starts
                     String contextPath = null;
@@ -489,22 +540,24 @@ public void invoke(Request request, Response response) throws IOException, Servl
                         // This is neither decoded nor normalized
                         chunk.append(contextPath);
                     }
-                    chunk.append(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+
+                    // 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
-                    urlStringDecoded = RequestUtil.normalize(urlStringDecoded);
+                    urlStringRewriteEncoded = RequestUtil.normalize(urlStringRewriteEncoded);
                     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(urlStringDecoded);
+                    chunk.append(URLDecoder.decode(urlStringRewriteEncoded, uriCharset));
                     // Set the new Query if there is one
-                    if (queryStringDecoded != null) {
+                    if (queryStringRewriteEncoded != null) {
                         request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
                         chunk = request.getCoyoteRequest().queryString().getCharChunk();
-                        chunk.append(URLEncoder.QUERY.encode(queryStringDecoded, uriCharset));
+                        chunk.append(REWRITE_QUERY_ENCODER.encode(queryStringRewriteEncoded, uriCharset));
                         if (qsa && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
                             chunk.append('&');
                             chunk.append(originalQueryStringEncoded);
@@ -799,4 +852,31 @@ protected static void parseRuleFlag(String line, RewriteRule rule, String flag)
             throw new IllegalArgumentException(sm.getString("rewriteValve.invalidFlags", line, flag));
         }
     }
+
+
+    private CharSequence encodeForRewrite(CharSequence input) {
+        StringBuilder result = null;
+        int pos = 0;
+        int mark = 0;
+        while (pos < input.length()) {
+            char c = input.charAt(pos);
+            if (c == '%' || c == ';' || c == '?') {
+                if (result == null) {
+                    result = new StringBuilder((int) (input.length() * 1.1));
+                }
+                result.append(input.subSequence(mark, pos));
+                result.append('%');
+                result.append(Character.forDigit((c >> 4) & 0xF, 16));
+                result.append(Character.forDigit(c & 0xF, 16));
+                mark = pos + 1;
+            }
+            pos++;
+        }
+        if (result != null) {
+            result.append(input.subSequence(mark, input.length()));
+            return result;
+        } else {
+            return input;
+        }
+    }
 }

@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.HttpURLConnection;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -63,7 +64,7 @@ public void testNoRewrite() throws Exception {
 
     @Test
     public void testBackslashPercentSign() throws Exception {
-        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%255A");
+        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%5A");
     }
 
     @Test
@@ -142,7 +143,7 @@ public void testRewriteMap09() throws Exception {
 
     @Test
     public void testRewriteMap10() throws Exception {
-        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%20aa");
     }
 
     @Test
@@ -346,16 +347,16 @@ public void testNonAsciiQueryStringAndPathAndRedirect() throws Exception {
     public void testNonAsciiQueryStringWithB() throws Exception {
         doTestRewrite("RewriteRule ^/b/(.*)/id=(.*) /c?filename=$1&id=$2 [B]",
                 "/b/file01/id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95", "/c",
-                "filename=file01&id=%25E5%259C%25A8%25E7%25BA%25BF%25E6%25B5%258B%25E8%25AF%2595");
+                "filename=file01&id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
     }
 
 
     @Test
     public void testNonAsciiQueryStringAndPathAndRedirectWithB() throws Exception {
         // Note the double encoding of the result (httpd produces the same result)
         doTestRewrite("RewriteRule ^/b/(.*)/(.*)/id=(.*) /c/$1?filename=$2&id=$3 [B,R]",
-                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%25E5%259C%25A8%25E7%25BA%25BF",
-                "filename=file01&id=%25E6%25B5%258B%25E8%25AF%2595");
+                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%E5%9C%A8%E7%BA%BF",
+                "filename=file01&id=%E6%B5%8B%E8%AF%95");
     }
 
 
@@ -371,7 +372,7 @@ public void testUtf8WithBothQsFlagsNone() throws Exception {
     public void testUtf8WithBothQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -387,7 +388,7 @@ public void testUtf8WithBothQsFlagsR() throws Exception {
     public void testUtf8WithBothQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -413,7 +414,7 @@ public void testUtf8WithBothQsFlagsRBNE() throws Exception {
     public void testUtf8WithBothQsFlagsBQSA() throws Exception {
         // 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=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -429,7 +430,7 @@ public void testUtf8WithBothQsFlagsRQSA() throws Exception {
     public void testUtf8WithBothQsFlagsRBQSA() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,QSA]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1&di=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -461,7 +462,7 @@ public void testUtf8WithOriginalQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -476,7 +477,7 @@ public void testUtf8WithOriginalQsFlagsR() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -510,8 +511,8 @@ public void testUtf8WithRewriteQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -534,8 +535,8 @@ public void testUtf8WithBothQsFlagsQSA() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -575,7 +576,7 @@ public void testUtf8FlagsNone() throws Exception {
     @Test
     public void testUtf8FlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -589,7 +590,7 @@ public void testUtf8FlagsR() throws Exception {
     @Test
     public void testUtf8FlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -784,6 +785,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
         rewriteValve.setConfiguration(config);
 
         Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/a/Z", "snoop");
         ctx.addServletMappingDecoded("/a/%5A", "snoop");
         ctx.addServletMappingDecoded("/c/*", "snoop");
         ctx.addServletMappingDecoded("/W/*", "snoop");
@@ -929,4 +931,87 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
             }
         }
     }
+
+
+    @Test
+    public void testEncodedUriSimple() throws Exception {
+        doTestRewriteWithEncoding("aaa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark01() throws Exception {
+        doTestRewriteWithEncoding("a%3fa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark02() throws Exception {
+        doTestRewriteWithEncoding("%3faa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark03() throws Exception {
+        doTestRewriteWithEncoding("aa%3f");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMarkAndQueryString() throws Exception {
+        doTestRewriteWithEncoding("a%3fa?b=c", "a%3fa", "b=c");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon01() throws Exception {
+        doTestRewriteWithEncoding("a%3ba");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon02() throws Exception {
+        doTestRewriteWithEncoding("%3baa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon03() throws Exception {
+        doTestRewriteWithEncoding("aa%3b");
+    }
+
+
+    private void doTestRewriteWithEncoding(String segment) throws Exception {
+        doTestRewriteWithEncoding(segment, segment, null);
+    }
+
+    private void doTestRewriteWithEncoding(String segment, String expectedSegment, String expectedQueryString)
+            throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        RewriteValve rewriteValve = new RewriteValve();
+        tomcat.getHost().getPipeline().addValve(rewriteValve);
+
+        rewriteValve.setConfiguration("RewriteRule ^/source/(.*)$ /target/$1");
+
+        Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/target/*", "snoop");
+
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort() + "/source/" + segment, res, false);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+
+        res.setCharset(StandardCharsets.UTF_8);
+        String body = res.toString();
+        Assert.assertTrue(body, body.contains("REQUEST-URI: /target/" + expectedSegment));
+        Assert.assertTrue(body, body.contains("PATH-INFO: /" +
+                URLDecoder.decode(expectedSegment, StandardCharsets.UTF_8)));
+        Assert.assertTrue(body, body.contains("REQUEST-QUERY-STRING: " + expectedQueryString));
+    }
 }

@@ -161,6 +161,11 @@
         <code>JsonErrorReportValve</code> equivalent to the one from
         <code>ErrorReportValve</code>. (remm)
       </fix>
+      <fix>
+        Improve the handling of <code>%nn</code> URL encoding in the
+        RewriteValve and document how <code>%nn</code> URL encoding may be used
+        with rewrite rules. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -56,6 +56,28 @@
 
 </section>
 
+<section name="Using rewrite rules with special characters">
+
+  <p>The URL presented to the rewrite valve is the same URL used for request
+  mapping with any literal <code>'%'</code>, <code>';'</code> and/or
+  <code>'?'</code> characters encoded in <code>%nn</code> form.</p>
+
+  <p>A rewrite rule that wishes to insert a literal <code>'%'</code>,
+  <code>';'</code>, <code>'?'</code>, <code>'&amp;'</code> or <code>'='</code>
+  character should do so in <code>%nn</code> form. Other characters maybe
+  inserted in either literal or <code>%nn</code> form.</p>
+
+  <p>This enables the rewrite rules to:
+  <ul>
+  <li>process URLs containing literal <code>'?'</code> characters;</li>
+  <li>add a query string;</li>
+  <li>insert a literal <code>'%'</code> character without it being confused with
+  <code>%nn</code> encoding.</li>
+  </ul>
+  </p>
+
+</section>
+
 <section name="Directives">
 
   <p>The rewrite.config file contains a list of directives which closely

@@ -653,18 +653,17 @@ protected boolean postParseRequest(org.apache.coyote.Request req, Request reques
             } else {
                 /*
                  * The URI is chars or String, and has been sent using an in-memory protocol handler. The following
-                 * assumptions are made: - req.requestURI() has been set to the 'original' non-decoded, non-normalized
-                 * URI - req.decodedURI() has been set to the decoded, normalized form of req.requestURI() -
-                 * 'suspicious' URI filtering - if required - has already been performed
+                 * assumptions are made:
+                 *
+                 * - req.requestURI() has been set to the 'original' non-decoded, non-normalized URI that includes path
+                 * parameters (if any)
+                 *
+                 * - req.decodedURI() has been set to the decoded, normalized form of req.requestURI() with any path
+                 * parameters removed
+                 *
+                 * - 'suspicious' URI filtering, if required, has already been performed
                  */
                 decodedURI.toChars();
-                // Remove all path parameters; any needed path parameter should be set
-                // using the request object rather than passing it in the URL
-                CharChunk uriCC = decodedURI.getCharChunk();
-                int semicolon = uriCC.indexOf(';');
-                if (semicolon > 0) {
-                    decodedURI.setChars(uriCC.getBuffer(), uriCC.getStart(), semicolon);
-                }
             }
         }
 

@@ -21,6 +21,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.net.URLDecoder;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -65,6 +66,24 @@
  */
 public class RewriteValve extends ValveBase {
 
+    private static final URLEncoder REWRITE_DEFAULT_ENCODER;
+    private static final URLEncoder REWRITE_QUERY_ENCODER;
+
+    static {
+        /*
+         * See the detailed explanation of encoding/decoding during URL re-writing in the invoke() method.
+         *
+         * These encoders perform the second stage of encoding, after re-writing has completed. These rewrite specific
+         * encoders treat '%' as a safe character so that URLs and query strings already processed by encodeForRewrite()
+         * do not end up with double encoding of '%' characters.
+         */
+        REWRITE_DEFAULT_ENCODER = (URLEncoder) URLEncoder.DEFAULT.clone();
+        REWRITE_DEFAULT_ENCODER.addSafeCharacter('%');
+
+        REWRITE_QUERY_ENCODER = (URLEncoder) URLEncoder.QUERY.clone();
+        REWRITE_QUERY_ENCODER.addSafeCharacter('%');
+    }
+
     /**
      * The rewrite rules that the valve will use.
      */
@@ -297,22 +316,51 @@ public void invoke(Request request, Response response) throws IOException, Servl
 
             invoked.set(Boolean.TRUE);
 
-            // As long as MB isn't a char sequence or affiliated, this has to be
-            // converted to a string
+            // 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();
             MessageBytes urlMB = context ? request.getRequestPathMB() : request.getDecodedRequestURIMB();
             urlMB.toChars();
             CharSequence urlDecoded = urlMB.getCharChunk();
+
+            /*
+             * The URL presented to the rewrite valve is the URL that is used for request mapping. That URL has been
+             * processed to: remove path parameters; remove the query string; decode; and normalize the URL. It may
+             * contain literal '%', '?' and/or ';' characters at this point.
+             *
+             * The re-write rules need to be able to process URLs with literal '?' characters and add query strings
+             * without the two becoming confused. The re-write rules also need to be able to insert literal '%'
+             * characters without them being confused with %nn encoding.
+             *
+             * The re-write rules cannot insert path parameters.
+             *
+             * To meet these requirement, the URL is processed as follows.
+             *
+             * Step 1. The URL is partially re-encoded by encodeForRewrite(). This method encodes any literal '%', ';'
+             * and/or '?' characters in the URL using the standard %nn form.
+             *
+             * Step 2. The re-write processing runs with the provided re-write rules against the partially encoded URL.
+             * If a re-write rule needs to insert a literal '%', ';' or '?', it must do so in %nn encoded form.
+             *
+             * Step 3. The URL (and query string if present) is re-encoded using the re-write specific encoders
+             * (REWRITE_DEFAULT_ENCODER and REWRITE_QUERY_ENCODER) that behave the same was as the standard encoders
+             * apart from '%' being treated as a safe character. This prevents double encoding of any '%' characters
+             * present in the URL from steps 1 or 2.
+             */
+
+            // Step 1. Encode URL for processing by the re-write rules.
+            CharSequence urlRewriteEncoded = encodeForRewrite(urlDecoded);
             CharSequence host = request.getServerName();
             boolean rewritten = false;
             boolean done = false;
             boolean qsa = false;
             boolean qsd = false;
             boolean valveSkip = false;
+
+            // Step 2. Process the URL using the re-write rules.
             for (int i = 0; i < rules.length; i++) {
                 RewriteRule rule = rules[i];
-                CharSequence test = (rule.isHost()) ? host : urlDecoded;
+                CharSequence test = (rule.isHost()) ? host : urlRewriteEncoded;
                 CharSequence newtest = rule.evaluate(test, resolver);
                 if (newtest != null && !Objects.equals(test.toString(), newtest.toString())) {
                     if (containerLog.isTraceEnabled()) {
@@ -322,7 +370,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
                     if (rule.isHost()) {
                         host = newtest;
                     } else {
-                        urlDecoded = newtest;
+                        urlRewriteEncoded = newtest;
                     }
                     rewritten = true;
                 }
@@ -359,28 +407,30 @@ public void invoke(Request request, Response response) throws IOException, Servl
                 if (rule.isRedirect() && newtest != null) {
                     // 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;
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    int index = urlStringRewriteEncoded.indexOf('?');
+                    String rewrittenQueryStringRewriteEncoded;
                     if (index == -1) {
-                        rewrittenQueryStringDecoded = null;
+                        rewrittenQueryStringRewriteEncoded = null;
                     } else {
-                        rewrittenQueryStringDecoded = urlStringDecoded.substring(index + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, index);
+                        rewrittenQueryStringRewriteEncoded = urlStringRewriteEncoded.substring(index + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, index);
                     }
 
+                    // Step 3. Complete the 2nd stage to encoding.
                     StringBuilder urlStringEncoded =
-                            new StringBuilder(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+                            new StringBuilder(REWRITE_DEFAULT_ENCODER.encode(urlStringRewriteEncoded, uriCharset));
+
                     if (!qsd && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
-                        if (rewrittenQueryStringDecoded == null) {
+                        if (rewrittenQueryStringRewriteEncoded == null) {
                             urlStringEncoded.append('?');
                             urlStringEncoded.append(originalQueryStringEncoded);
                         } else {
                             if (qsa) {
                                 // if qsa is specified append the query
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                                 urlStringEncoded.append('&');
                                 urlStringEncoded.append(originalQueryStringEncoded);
                             } else if (index == urlStringEncoded.length() - 1) {
@@ -389,13 +439,14 @@ public void invoke(Request request, Response response) throws IOException, Servl
                                 urlStringEncoded.deleteCharAt(index);
                             } else {
                                 urlStringEncoded.append('?');
-                                urlStringEncoded
-                                        .append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                                urlStringEncoded.append(
+                                        REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                             }
                         }
-                    } else if (rewrittenQueryStringDecoded != null) {
+                    } else if (rewrittenQueryStringRewriteEncoded != null) {
                         urlStringEncoded.append('?');
-                        urlStringEncoded.append(URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriCharset));
+                        urlStringEncoded
+                                .append(REWRITE_QUERY_ENCODER.encode(rewrittenQueryStringRewriteEncoded, uriCharset));
                     }
 
                     // Insert the context if
@@ -470,12 +521,12 @@ public void invoke(Request request, Response response) throws IOException, Servl
             if (rewritten) {
                 if (!done) {
                     // See if we need to replace the query string
-                    String urlStringDecoded = urlDecoded.toString();
-                    String queryStringDecoded = null;
-                    int queryIndex = urlStringDecoded.indexOf('?');
+                    String urlStringRewriteEncoded = urlRewriteEncoded.toString();
+                    String queryStringRewriteEncoded = null;
+                    int queryIndex = urlStringRewriteEncoded.indexOf('?');
                     if (queryIndex != -1) {
-                        queryStringDecoded = urlStringDecoded.substring(queryIndex + 1);
-                        urlStringDecoded = urlStringDecoded.substring(0, queryIndex);
+                        queryStringRewriteEncoded = urlStringRewriteEncoded.substring(queryIndex + 1);
+                        urlStringRewriteEncoded = urlStringRewriteEncoded.substring(0, queryIndex);
                     }
                     // Save the current context path before re-writing starts
                     String contextPath = null;
@@ -489,22 +540,24 @@ public void invoke(Request request, Response response) throws IOException, Servl
                         // This is neither decoded nor normalized
                         chunk.append(contextPath);
                     }
-                    chunk.append(URLEncoder.DEFAULT.encode(urlStringDecoded, uriCharset));
+
+                    // 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
-                    urlStringDecoded = RequestUtil.normalize(urlStringDecoded);
+                    urlStringRewriteEncoded = RequestUtil.normalize(urlStringRewriteEncoded);
                     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(urlStringDecoded);
+                    chunk.append(URLDecoder.decode(urlStringRewriteEncoded, uriCharset));
                     // Set the new Query if there is one
-                    if (queryStringDecoded != null) {
+                    if (queryStringRewriteEncoded != null) {
                         request.getCoyoteRequest().queryString().setChars(MessageBytes.EMPTY_CHAR_ARRAY, 0, 0);
                         chunk = request.getCoyoteRequest().queryString().getCharChunk();
-                        chunk.append(URLEncoder.QUERY.encode(queryStringDecoded, uriCharset));
+                        chunk.append(REWRITE_QUERY_ENCODER.encode(queryStringRewriteEncoded, uriCharset));
                         if (qsa && originalQueryStringEncoded != null && !originalQueryStringEncoded.isEmpty()) {
                             chunk.append('&');
                             chunk.append(originalQueryStringEncoded);
@@ -799,4 +852,31 @@ protected static void parseRuleFlag(String line, RewriteRule rule, String flag)
             throw new IllegalArgumentException(sm.getString("rewriteValve.invalidFlags", line, flag));
         }
     }
+
+
+    private CharSequence encodeForRewrite(CharSequence input) {
+        StringBuilder result = null;
+        int pos = 0;
+        int mark = 0;
+        while (pos < input.length()) {
+            char c = input.charAt(pos);
+            if (c == '%' || c == ';' || c == '?') {
+                if (result == null) {
+                    result = new StringBuilder((int) (input.length() * 1.1));
+                }
+                result.append(input.subSequence(mark, pos));
+                result.append('%');
+                result.append(Character.forDigit((c >> 4) & 0xF, 16));
+                result.append(Character.forDigit(c & 0xF, 16));
+                mark = pos + 1;
+            }
+            pos++;
+        }
+        if (result != null) {
+            result.append(input.subSequence(mark, input.length()));
+            return result;
+        } else {
+            return input;
+        }
+    }
 }

@@ -20,6 +20,7 @@
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.HttpURLConnection;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -63,7 +64,7 @@ public void testNoRewrite() throws Exception {
 
     @Test
     public void testBackslashPercentSign() throws Exception {
-        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%255A");
+        doTestRewrite("RewriteRule ^(.*) /a/\\%5A", "/", "/a/%5A");
     }
 
     @Test
@@ -142,7 +143,7 @@ public void testRewriteMap09() throws Exception {
 
     @Test
     public void testRewriteMap10() throws Exception {
-        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%2520aa");
+        doTestRewrite("RewriteMap lc int:escape\n" + "RewriteRule ^(.*) ${lc:$1}", "/c/a%20aa", "/c/a%20aa");
     }
 
     @Test
@@ -346,16 +347,16 @@ public void testNonAsciiQueryStringAndPathAndRedirect() throws Exception {
     public void testNonAsciiQueryStringWithB() throws Exception {
         doTestRewrite("RewriteRule ^/b/(.*)/id=(.*) /c?filename=$1&id=$2 [B]",
                 "/b/file01/id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95", "/c",
-                "filename=file01&id=%25E5%259C%25A8%25E7%25BA%25BF%25E6%25B5%258B%25E8%25AF%2595");
+                "filename=file01&id=%E5%9C%A8%E7%BA%BF%E6%B5%8B%E8%AF%95");
     }
 
 
     @Test
     public void testNonAsciiQueryStringAndPathAndRedirectWithB() throws Exception {
         // Note the double encoding of the result (httpd produces the same result)
         doTestRewrite("RewriteRule ^/b/(.*)/(.*)/id=(.*) /c/$1?filename=$2&id=$3 [B,R]",
-                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%25E5%259C%25A8%25E7%25BA%25BF",
-                "filename=file01&id=%25E6%25B5%258B%25E8%25AF%2595");
+                "/b/%E5%9C%A8%E7%BA%BF/file01/id=%E6%B5%8B%E8%AF%95", "/c/%E5%9C%A8%E7%BA%BF",
+                "filename=file01&id=%E6%B5%8B%E8%AF%95");
     }
 
 
@@ -371,7 +372,7 @@ public void testUtf8WithBothQsFlagsNone() throws Exception {
     public void testUtf8WithBothQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -387,7 +388,7 @@ public void testUtf8WithBothQsFlagsR() throws Exception {
     public void testUtf8WithBothQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1");
     }
 
 
@@ -413,7 +414,7 @@ public void testUtf8WithBothQsFlagsRBNE() throws Exception {
     public void testUtf8WithBothQsFlagsBQSA() throws Exception {
         // 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=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -429,7 +430,7 @@ public void testUtf8WithBothQsFlagsRQSA() throws Exception {
     public void testUtf8WithBothQsFlagsRBQSA() throws Exception {
         // Note %C2%A1 == \u00A1
         doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,QSA]", "/b/%C2%A1/id=%C2%A1?di=%C2%AE",
-                "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1&di=%C2%AE");
+                "/c/%C2%A1%C2%A1", "id=%C2%A1&di=%C2%AE");
     }
 
 
@@ -461,7 +462,7 @@ public void testUtf8WithOriginalQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -476,7 +477,7 @@ public void testUtf8WithOriginalQsFlagsR() throws Exception {
     @Test
     public void testUtf8WithOriginalQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%25C2%25A1",
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1",
                 "id=%C2%A1");
     }
 
@@ -510,8 +511,8 @@ public void testUtf8WithRewriteQsFlagsNone() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -534,8 +535,8 @@ public void testUtf8WithBothQsFlagsQSA() throws Exception {
     @Test
     public void testUtf8WithRewriteQsFlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1",
-                "id=%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]", "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
     }
 
 
@@ -575,7 +576,7 @@ public void testUtf8FlagsNone() throws Exception {
     @Test
     public void testUtf8FlagsB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -589,7 +590,7 @@ public void testUtf8FlagsR() throws Exception {
     @Test
     public void testUtf8FlagsRB() throws Exception {
         // Note %C2%A1 == \u00A1
-        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%25C2%25A1");
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B]", "/b/%C2%A1", "/c/%C2%A1%C2%A1");
     }
 
 
@@ -784,6 +785,7 @@ public void invoke(Request request, Response response) throws IOException, Servl
         rewriteValve.setConfiguration(config);
 
         Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/a/Z", "snoop");
         ctx.addServletMappingDecoded("/a/%5A", "snoop");
         ctx.addServletMappingDecoded("/c/*", "snoop");
         ctx.addServletMappingDecoded("/W/*", "snoop");
@@ -929,4 +931,87 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
             }
         }
     }
+
+
+    @Test
+    public void testEncodedUriSimple() throws Exception {
+        doTestRewriteWithEncoding("aaa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark01() throws Exception {
+        doTestRewriteWithEncoding("a%3fa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark02() throws Exception {
+        doTestRewriteWithEncoding("%3faa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMark03() throws Exception {
+        doTestRewriteWithEncoding("aa%3f");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedQuestionMarkAndQueryString() throws Exception {
+        doTestRewriteWithEncoding("a%3fa?b=c", "a%3fa", "b=c");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon01() throws Exception {
+        doTestRewriteWithEncoding("a%3ba");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon02() throws Exception {
+        doTestRewriteWithEncoding("%3baa");
+    }
+
+
+    @Test
+    public void testEncodedUriEncodedSemicolon03() throws Exception {
+        doTestRewriteWithEncoding("aa%3b");
+    }
+
+
+    private void doTestRewriteWithEncoding(String segment) throws Exception {
+        doTestRewriteWithEncoding(segment, segment, null);
+    }
+
+    private void doTestRewriteWithEncoding(String segment, String expectedSegment, String expectedQueryString)
+            throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("", null);
+
+        RewriteValve rewriteValve = new RewriteValve();
+        tomcat.getHost().getPipeline().addValve(rewriteValve);
+
+        rewriteValve.setConfiguration("RewriteRule ^/source/(.*)$ /target/$1");
+
+        Tomcat.addServlet(ctx, "snoop", new SnoopServlet());
+        ctx.addServletMappingDecoded("/target/*", "snoop");
+
+        tomcat.start();
+
+        ByteChunk res = new ByteChunk();
+        int rc = getUrl("http://localhost:" + getPort() + "/source/" + segment, res, false);
+
+        Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+
+        res.setCharset(StandardCharsets.UTF_8);
+        String body = res.toString();
+        Assert.assertTrue(body, body.contains("REQUEST-URI: /target/" + expectedSegment));
+        Assert.assertTrue(body, body.contains("PATH-INFO: /" +
+                URLDecoder.decode(expectedSegment, StandardCharsets.UTF_8)));
+        Assert.assertTrue(body, body.contains("REQUEST-QUERY-STRING: " + expectedQueryString));
+    }
 }

@@ -161,6 +161,11 @@
         <code>JsonErrorReportValve</code> equivalent to the one from
         <code>ErrorReportValve</code>. (remm)
       </fix>
+      <fix>
+        Improve the handling of <code>%nn</code> URL encoding in the
+        RewriteValve and document how <code>%nn</code> URL encoding may be used
+        with rewrite rules. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

@@ -56,6 +56,28 @@
 
 </section>
 
+<section name="Using rewrite rules with special characters">
+
+  <p>The URL presented to the rewrite valve is the same URL used for request
+  mapping with any literal <code>'%'</code>, <code>';'</code> and/or
+  <code>'?'</code> characters encoded in <code>%nn</code> form.</p>
+
+  <p>A rewrite rule that wishes to insert a literal <code>'%'</code>,
+  <code>';'</code>, <code>'?'</code>, <code>'&amp;'</code> or <code>'='</code>
+  character should do so in <code>%nn</code> form. Other characters maybe
+  inserted in either literal or <code>%nn</code> form.</p>
+
+  <p>This enables the rewrite rules to:
+  <ul>
+  <li>process URLs containing literal <code>'?'</code> characters;</li>
+  <li>add a query string;</li>
+  <li>insert a literal <code>'%'</code> character without it being confused with
+  <code>%nn</code> encoding.</li>
+  </ul>
+  </p>
+
+</section>
+
 <section name="Directives">
 
   <p>The rewrite.config file contains a list of directives which closely