Author: markt
Date: Wed Sep 21 13:27:10 2016
New Revision: 1761727

URL: http://svn.apache.org/viewvc?rev=1761727&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=60013
Refactor the previous fix to align the behaviour of the Rewrite Valve with 
mod_rewite. As part of this, provide an implementation for the B and NE flags 
and improve the handling for the QSA flag.
Includes multiple test cases by Santhana Preethiand a patch by Tiago Oliveira.

Modified:
    tomcat/tc8.5.x/trunk/   (props changed)
    tomcat/tc8.5.x/trunk/build.xml
    tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Resolver.java
    
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/ResolverImpl.java
    
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteRule.java
    
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
    
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java
    
tomcat/tc8.5.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
    tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc8.5.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Sep 21 13:27:10 2016
@@ -1 +1 @@
-/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737903,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501
 
,1741677,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745083,1745142-1745143,1745145,1745177,1745179-1745180,1745227,1745248,1745254,1745337,1745467,1745473,1745576,1745735,1745744,1746304,1746306-1746307,1746319,1746327,1746338,1746340-1746341,1746344,1746427,1746441,1746473,1746490,1746492,1746495-1746496,1746499-1746501,1746503-1746507,1746509,1746549,1746551,1746554,1746556,1746558,1746584,1746620,1746649,1746724,1746939,1746989,1747014,1747028,1747035,1747210,1747225,1747234,1747253,1747404,1747506,1747536,1747
 
924,1747980,1747993,1748001,1748253,1748452,1748547,1748629,1748676,1748715,1749287,1749296,1749328,1749373,1749465,1749506,1749508,1749665-1749666,1749763,1749865-1749866,1749898,1749978,1749980,1750011,1750015,1750056,1750480,1750617,1750634,1750692,1750697,1750700,1750703,1750707,1750714,1750718,1750723,1750774,1750899,1750975,1750995,1751061,1751097,1751173,1751438,1751447,1751463,1751702,1752212,1752737,1752745,1753078,1753080,1753358,1753363,1754111,1754140-1754141,1754281,1754310,1754445,1754467,1754494,1754496,1754528,1754532-1754533,1754613,1754714,1754874,1754941,1754944,1754950-1754951,1755005,1755007,1755009,1755132,1755180-1755181,1755185,1755190,1755204-1755206,1755208,1755214,1755224,1755227,1755230,1755629,1755646-1755647,1755650,1755653,1755675,1755680,1755683,1755693,1755717,1755731-1755737,1755812,1755828,1755884,1755890,1755918-1755919,1755942,1755958,1755960,1755970,1755993,1756013,1756019,1756039,1756056,1756083-1756114,1756175,1756288-1756289,1756408-1756410,1
 
756778,1756798,1756878,1756898,1756939,1757123-1757124,1757126,1757128,1757132-1757133,1757136,1757145,1757167-1757168,1757175,1757180,1757182,1757195,1757271,1757278,1757347,1757353-1757354,1757363,1757374,1757399,1757406,1757408,1757485,1757495,1757499,1757527,1757578,1757684,1757722,1757727,1757853,1757903,1757997,1758072-1758075,1758078-1758079,1758292,1758369,1758423,1758425-1758427,1758430,1758459,1758483,1758486-1758487,1758499,1758525,1758556,1758582,1758584,1758588,1758842,1759019,1759212,1759224,1759227,1759252,1759274,1759611,1760005,1760300,1760446,1760454,1760640,1760648,1761422,1761550,1761553,1761572,1761574,1761628,1761682
+/tomcat/trunk:1734785,1734799,1734845,1734928,1735041,1735044,1735480,1735577,1735597,1735599-1735600,1735615,1736145,1736162,1736209,1736280,1736297,1736299,1736489,1736646,1736703,1736836,1736849,1737104-1737105,1737112,1737117,1737119-1737120,1737155,1737157,1737192,1737280,1737339,1737632,1737664,1737715,1737748,1737785,1737834,1737860,1737903,1737959,1738005,1738007,1738014-1738015,1738018,1738022,1738039,1738043,1738059-1738060,1738147,1738149,1738174-1738175,1738261,1738589,1738623-1738625,1738643,1738816,1738850,1738855,1738946-1738948,1738953-1738954,1738979,1738982,1739079-1739081,1739087,1739113,1739153,1739172,1739176,1739191,1739474,1739726,1739762,1739775,1739814,1739817-1739818,1739975,1740131,1740324,1740465,1740495,1740508-1740509,1740520,1740535,1740707,1740803,1740810,1740969,1740980,1740991,1740997,1741015,1741033,1741036,1741058,1741060,1741080,1741147,1741159,1741164,1741173,1741181,1741190,1741197,1741202,1741208,1741213,1741221,1741225,1741232,1741409,1741501
 
,1741677,1741892,1741896,1741984,1742023,1742042,1742071,1742090,1742093,1742101,1742105,1742111,1742139,1742146,1742148,1742166,1742181,1742184,1742187,1742246,1742248-1742251,1742263-1742264,1742268,1742276,1742369,1742387,1742448,1742509-1742512,1742917,1742919,1742933,1742975-1742976,1742984,1742986,1743019,1743115,1743117,1743124-1743125,1743134,1743425,1743554,1743679,1743696-1743698,1743700-1743701,1744058,1744064-1744065,1744125,1744194,1744229,1744270,1744323,1744432,1744684,1744697,1744705,1744713,1744760,1744786,1745083,1745142-1745143,1745145,1745177,1745179-1745180,1745227,1745248,1745254,1745337,1745467,1745473,1745576,1745735,1745744,1746304,1746306-1746307,1746319,1746327,1746338,1746340-1746341,1746344,1746427,1746441,1746473,1746490,1746492,1746495-1746496,1746499-1746501,1746503-1746507,1746509,1746549,1746551,1746554,1746556,1746558,1746584,1746620,1746649,1746724,1746939,1746989,1747014,1747028,1747035,1747210,1747225,1747234,1747253,1747404,1747506,1747536,1747
 
924,1747980,1747993,1748001,1748253,1748452,1748547,1748629,1748676,1748715,1749287,1749296,1749328,1749373,1749465,1749506,1749508,1749665-1749666,1749763,1749865-1749866,1749898,1749978,1749980,1750011,1750015,1750056,1750480,1750617,1750634,1750692,1750697,1750700,1750703,1750707,1750714,1750718,1750723,1750774,1750899,1750975,1750995,1751061,1751097,1751173,1751438,1751447,1751463,1751702,1752212,1752737,1752745,1753078,1753080,1753358,1753363,1754111,1754140-1754141,1754281,1754310,1754445,1754467,1754494,1754496,1754528,1754532-1754533,1754613,1754714,1754874,1754941,1754944,1754950-1754951,1755005,1755007,1755009,1755132,1755180-1755181,1755185,1755190,1755204-1755206,1755208,1755214,1755224,1755227,1755230,1755629,1755646-1755647,1755650,1755653,1755675,1755680,1755683,1755693,1755717,1755731-1755737,1755812,1755828,1755884,1755890,1755918-1755919,1755942,1755958,1755960,1755970,1755993,1756013,1756019,1756039,1756056,1756083-1756114,1756175,1756288-1756289,1756408-1756410,1
 
756778,1756798,1756878,1756898,1756939,1757123-1757124,1757126,1757128,1757132-1757133,1757136,1757145,1757167-1757168,1757175,1757180,1757182,1757195,1757271,1757278,1757347,1757353-1757354,1757363,1757374,1757399,1757406,1757408,1757485,1757495,1757499,1757527,1757578,1757684,1757722,1757727,1757853,1757903,1757997,1758072-1758075,1758078-1758079,1758292,1758369,1758423,1758425-1758427,1758430,1758459,1758483,1758486-1758487,1758499,1758525,1758556,1758582,1758584,1758588,1758842,1759019,1759212,1759224,1759227,1759252,1759274,1759513-1759516,1759611,1760005,1760022,1760300,1760397,1760446,1760454,1760640,1760648,1761057,1761422,1761491,1761498,1761500-1761501,1761550,1761553,1761572,1761574,1761625-1761626,1761628,1761682

Modified: tomcat/tc8.5.x/trunk/build.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/build.xml?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/build.xml (original)
+++ tomcat/tc8.5.x/trunk/build.xml Wed Sep 21 13:27:10 2016
@@ -1414,6 +1414,7 @@
         threads="${test.threads}" >
 
         <jvmarg value="${test.jvmarg.egd}"/>
+        <jvmarg value="-Dfile.encoding=UTF-8"/>
         <jvmarg value="-Djava.library.path=${test.apr.loc}"/>
         <jvmarg value="${test.formatter}"/>
         <jvmarg 
value="-Djava.net.preferIPv4Stack=${java.net.preferIPv4Stack}"/>

Modified: 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Resolver.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Resolver.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Resolver.java 
(original)
+++ tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Resolver.java 
Wed Sep 21 13:27:10 2016
@@ -33,4 +33,6 @@ public abstract class Resolver {
     public abstract String resolveHttp(String key);
 
     public abstract boolean resolveResource(int type, String name);
+
+    public abstract String getUriEncoding();
 }

Modified: 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/ResolverImpl.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/ResolverImpl.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/ResolverImpl.java 
(original)
+++ 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/ResolverImpl.java 
Wed Sep 21 13:27:10 2016
@@ -177,4 +177,9 @@ public class ResolverImpl extends Resolv
             return value;
         }
     }
+
+    @Override
+    public String getUriEncoding() {
+        return request.getConnector().getURIEncoding();
+    }
 }

Modified: 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteRule.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteRule.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteRule.java 
(original)
+++ 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteRule.java 
Wed Sep 21 13:27:10 2016
@@ -37,6 +37,7 @@ public class RewriteRule {
             substitution = new Substitution();
             substitution.setSub(substitutionString);
             substitution.parse(maps);
+            substitution.setEscapeBackReferences(isEscapeBackReferences());
         }
         // Parse the pattern
         int flags = 0;
@@ -151,6 +152,8 @@ public class RewriteRule {
     }
 
 
+    private boolean escapeBackReferences = false;
+
     /**
      *  This flag chains the current rule with the next rule (which itself
      *  can be chained with the following rule, etc.). This has the following
@@ -325,6 +328,13 @@ public class RewriteRule {
      */
     protected boolean type = false;
     protected String typeValue = null;
+
+    public boolean isEscapeBackReferences() {
+        return escapeBackReferences;
+    }
+    public void setEscapeBackReferences(boolean escapeBackReferences) {
+        this.escapeBackReferences = escapeBackReferences;
+    }
     public boolean isChain() {
         return chain;
     }

Modified: 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java 
(original)
+++ 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java 
Wed Sep 21 13:27:10 2016
@@ -23,6 +23,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.StringReader;
+import java.net.URLDecoder;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Hashtable;
@@ -53,12 +54,42 @@ import org.apache.tomcat.util.http.Reque
 
 public class RewriteValve extends ValveBase {
 
-    private static final URLEncoder QUERY_STRING_ENCODER;
-
+    static URLEncoder ENCODER = new URLEncoder();
     static {
-        QUERY_STRING_ENCODER = new URLEncoder();
-        QUERY_STRING_ENCODER.addSafeCharacter('=');
-        QUERY_STRING_ENCODER.addSafeCharacter('&');
+        /*
+         * Replicates httpd's encoding
+         * Primarily aimed at encoding URI paths, so from the spec:
+         *
+         * pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
+         *
+         * unreserved  = ALPHA / DIGIT / "-" / "." / "_" / "~"
+         *
+         * sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
+         *              / "*" / "+" / "," / ";" / "="
+         */
+        // ALPHA and DIGIT are always treated as safe characters
+        // Add the remaining unreserved characters
+        ENCODER.addSafeCharacter('-');
+        ENCODER.addSafeCharacter('.');
+        ENCODER.addSafeCharacter('_');
+        ENCODER.addSafeCharacter('~');
+        // Add the sub-delims
+        ENCODER.addSafeCharacter('!');
+        ENCODER.addSafeCharacter('$');
+        ENCODER.addSafeCharacter('&');
+        ENCODER.addSafeCharacter('\'');
+        ENCODER.addSafeCharacter('(');
+        ENCODER.addSafeCharacter(')');
+        ENCODER.addSafeCharacter('*');
+        ENCODER.addSafeCharacter('+');
+        ENCODER.addSafeCharacter(',');
+        ENCODER.addSafeCharacter(';');
+        ENCODER.addSafeCharacter('=');
+        // Add the remaining literals
+        ENCODER.addSafeCharacter(':');
+        ENCODER.addSafeCharacter('@');
+        // Add '/' so it isn't encoded when we encode a path
+        ENCODER.addSafeCharacter('/');
     }
 
 
@@ -292,15 +323,19 @@ 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 url = urlMB.getCharChunk();
+            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 : url;
+                CharSequence test = (rule.isHost()) ? host : urlDecoded;
                 CharSequence newtest = rule.evaluate(test, resolver);
                 if (newtest != null && !test.equals(newtest.toString())) {
                     if (container.getLogger().isDebugEnabled()) {
@@ -310,11 +345,18 @@ public class RewriteValve extends ValveB
                     if (rule.isHost()) {
                         host = newtest;
                     } else {
-                        url = newtest;
+                        urlDecoded = newtest;
                     }
                     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
@@ -329,49 +371,66 @@ 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 originalQueryString = request.getQueryString();
-                    StringBuffer urlString = new StringBuffer(url);
-                    int index = urlString.indexOf("?");
-                    String encodedUrl;
+                    // 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;
                     if (index == -1) {
-                        encodedUrl = 
URLEncoder.DEFAULT.encode(urlString.toString(), "UTF-8");
-                        urlString.setLength(0);
-                        urlString.append(encodedUrl);
+                        rewrittenQueryStringDecoded = null;
                     } 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(originalQueryString);
-                            }
-                            // if the ? is the last character delete it, its 
only purpose was to
-                            // prevent the rewrite module from appending the 
query string
-                            else if (index == urlString.length() - 1) {
-                                urlString.deleteCharAt(index);
-                            }
+                        rewrittenQueryStringDecoded = 
urlStringDecoded.substring(index + 1);
+                        urlStringDecoded = urlStringDecoded.substring(0, 
index);
+                    }
+
+                    StringBuffer urlStringEncoded =
+                            new StringBuffer(ENCODER.encode(urlStringDecoded, 
uriEncoding));
+                    if (originalQueryStringEncoded != null &&
+                            originalQueryStringEncoded.length() > 0) {
+                        if (rewrittenQueryStringDecoded == null) {
+                            urlStringEncoded.append('?');
+                            
urlStringEncoded.append(originalQueryStringEncoded);
                         } else {
-                            urlString.append('?');
-                            urlString.append(originalQueryString);
+                            if (qsa) {
+                                // if qsa is specified append the query
+                                urlStringEncoded.append('?');
+                                urlStringEncoded.append(
+                                        
ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
+                                urlStringEncoded.append('&');
+                                
urlStringEncoded.append(originalQueryStringEncoded);
+                            } else if (index == urlStringEncoded.length() - 1) 
{
+                                // if the ? is the last character delete it, 
its only purpose was to
+                                // prevent the rewrite module from appending 
the query string
+                                urlStringEncoded.deleteCharAt(index);
+                            } else {
+                                urlStringEncoded.append('?');
+                                urlStringEncoded.append(
+                                        
ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
+                            }
                         }
+                    } else if (rewrittenQueryStringDecoded != null) {
+                        urlStringEncoded.append('?');
+                        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 && urlString.charAt(0) == '/' && 
!UriUtil.hasScheme(urlString)) {
-                        urlString.insert(0, 
request.getContext().getEncodedPath());
+                    if (context && urlStringEncoded.charAt(0) == '/' &&
+                            !UriUtil.hasScheme(urlStringEncoded)) {
+                        urlStringEncoded.insert(0, 
request.getContext().getEncodedPath());
+                    }
+                    if (rule.isNoescape()) {
+                        response.sendRedirect(
+                                URLDecoder.decode(urlStringEncoded.toString(), 
uriEncoding));
+                    } else {
+                        response.sendRedirect(urlStringEncoded.toString());
                     }
-                    response.sendRedirect(urlString.toString());
                     response.setStatus(rule.getRedirectCode());
                     done = true;
                     break;
@@ -401,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 = url.toString();
-                    if (urlString.indexOf('?') != -1 && queryString != null) {
-                        url = urlString + "&" + queryString;
-                    }
-                }
 
                 // Control flow processing
 
@@ -441,45 +492,54 @@ public class RewriteValve extends ValveB
             if (rewritten) {
                 if (!done) {
                     // See if we need to replace the query string
-                    String urlString = url.toString();
-                    String queryString = null;
-                    int queryIndex = urlString.indexOf('?');
+                    String urlStringDecoded = urlDecoded.toString();
+                    String queryStringDecoded = null;
+                    int queryIndex = urlStringDecoded.indexOf('?');
                     if (queryIndex != -1) {
-                        queryString = urlString.substring(queryIndex+1);
-                        urlString = urlString.substring(0, queryIndex);
+                        queryStringDecoded = 
urlStringDecoded.substring(queryIndex+1);
+                        urlStringDecoded = urlStringDecoded.substring(0, 
queryIndex);
                     }
-                    // Set the new 'original' URI
+                    // Save the current context path before re-writing starts
                     String contextPath = null;
                     if (context) {
                         contextPath = request.getContextPath();
                     }
+                    // Populated the encoded (i.e. undecoded) requestURI
                     request.getCoyoteRequest().requestURI().setString(null);
                     CharChunk chunk = 
request.getCoyoteRequest().requestURI().getCharChunk();
                     chunk.recycle();
                     if (context) {
+                        // This is neither decoded nor normalized
                         chunk.append(contextPath);
                     }
-                    chunk.append(URLEncoder.DEFAULT.encode(urlString, 
"UTF-8"));
+                    chunk.append(ENCODER.encode(urlStringDecoded, 
uriEncoding));
                     request.getCoyoteRequest().requestURI().toChars();
                     // Decoded and normalized URI
+                    // Rewriting may have denormalized the URL
+                    urlStringDecoded = RequestUtil.normalize(urlStringDecoded);
                     request.getCoyoteRequest().decodedURI().setString(null);
                     chunk = 
request.getCoyoteRequest().decodedURI().getCharChunk();
                     chunk.recycle();
                     if (context) {
-                        chunk.append(contextPath);
+                        // This is decoded and normalized
+                        
chunk.append(request.getServletContext().getContextPath());
                     }
-                    chunk.append(RequestUtil.normalize(urlString));
+                    chunk.append(urlStringDecoded);
                     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
+                    if (queryStringDecoded != null) {
                         
request.getCoyoteRequest().queryString().setString(null);
                         chunk = 
request.getCoyoteRequest().queryString().getCharChunk();
                         chunk.recycle();
-                        chunk.append(QUERY_STRING_ENCODER.encode(queryString, 
"UTF-8"));
-                        request.getCoyoteRequest().queryString().toChars();
+                        chunk.append(ENCODER.encode(queryStringDecoded, 
uriEncoding));
+                        if (qsa && originalQueryStringEncoded != null &&
+                                originalQueryStringEncoded.length() > 0) {
+                            chunk.append('&');
+                            chunk.append(originalQueryStringEncoded);
+                        }
+                        if (!chunk.isNull()) {
+                            request.getCoyoteRequest().queryString().toChars();
+                        }
                     }
                     // Set the new host if it changed
                     if (!host.equals(request.getServerName())) {
@@ -663,7 +723,9 @@ public class RewriteValve extends ValveB
      * @param flag The flag
      */
     protected static void parseRuleFlag(String line, RewriteRule rule, String 
flag) {
-        if (flag.equals("chain") || flag.equals("C")) {
+        if (flag.equals("B")) {
+            rule.setEscapeBackReferences(true);
+        } else if (flag.equals("chain") || flag.equals("C")) {
             rule.setChain(true);
         } else if (flag.startsWith("cookie=") || flag.startsWith("CO=")) {
             rule.setCookie(true);

Modified: 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java 
(original)
+++ 
tomcat/tc8.5.x/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java 
Wed Sep 21 13:27:10 2016
@@ -40,7 +40,15 @@ public class Substitution {
         public int n;
         @Override
         public String evaluate(Matcher rule, Matcher cond, Resolver resolver) {
-            return rule.group(n);
+            if (escapeBackReferences) {
+                // Note: This should be consistent with the way httpd behaves.
+                //       We might want to consider providing a dedicated 
decoder
+                //       with an option to add additional safe characters to
+                //       provide users with more flexibility
+                return RewriteValve.ENCODER.encode(rule.group(n), 
resolver.getUriEncoding());
+            } else {
+                return rule.group(n);
+            }
         }
     }
 
@@ -105,6 +113,11 @@ public class Substitution {
     public String getSub() { return sub; }
     public void setSub(String sub) { this.sub = sub; }
 
+    private boolean escapeBackReferences;
+    void setEscapeBackReferences(boolean escapeBackReferences) {
+        this.escapeBackReferences = escapeBackReferences;
+    }
+
     public void parse(Map<String, RewriteMap> maps) {
 
         ArrayList<SubstitutionElement> elements = new ArrayList<>();

Modified: 
tomcat/tc8.5.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- 
tomcat/tc8.5.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
 (original)
+++ 
tomcat/tc8.5.x/trunk/test/org/apache/catalina/valves/rewrite/TestRewriteValve.java
 Wed Sep 21 13:27:10 2016
@@ -27,6 +27,15 @@ import org.apache.catalina.startup.Tomca
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.tomcat.util.buf.ByteChunk;
 
+/*
+ * Implementation note:
+ *
+ * A number of these tests involve the rewrite valve returning a HTTP Location
+ * header that include un-encoded UTF-8 bytes. How the HTTP client handles 
these
+ * depends on the default character encoding configured for the JVM running the
+ * test. The tests expect the client to be configured with UTF-8 as the default
+ * encoding. Use of any other encoding is likely to lead to test failures.
+ */
 public class TestRewriteValve extends TomcatBaseTest {
 
     @Test
@@ -75,13 +84,15 @@ public class TestRewriteValve extends To
     @Test
     public void testRewriteEnvVarAndServerVar() throws Exception {
         System.setProperty("some_variable", "something");
-        doTestRewrite("RewriteRule /b/(.*).html$ 
/c/%{ENV:some_variable}%{SERVLET_PATH}", "/b/x.html", "/c/something/b/x.html");
+        doTestRewrite("RewriteRule /b/(.*).html$ 
/c/%{ENV:some_variable}%{SERVLET_PATH}",
+                "/b/x.html", "/c/something/b/x.html");
     }
 
     @Test
     public void testRewriteServerVarAndEnvVar() throws Exception {
         System.setProperty("some_variable", "something");
-        doTestRewrite("RewriteRule /b/(.*).html$ 
/c%{SERVLET_PATH}/%{ENV:some_variable}", "/b/x.html", "/c/b/x.html/something");
+        doTestRewrite("RewriteRule /b/(.*).html$ 
/c%{SERVLET_PATH}/%{ENV:some_variable}",
+                "/b/x.html", "/c/b/x.html/something");
     }
 
     @Test
@@ -90,7 +101,7 @@ public class TestRewriteValve extends To
             doTestRewrite("RewriteRule /b/(.*).html$ /c%_{SERVLET_PATH}", 
"/b/x.html", "/c");
             Assert.fail("IAE expected.");
         } catch (java.lang.IllegalArgumentException e) {
-            // excpected as %_{ is invalid
+            // expected as %_{ is invalid
         }
     }
 
@@ -100,7 +111,7 @@ public class TestRewriteValve extends To
             doTestRewrite("RewriteRule /b/(.*).html$ /c$_{SERVLET_PATH}", 
"/b/x.html", "/c");
             Assert.fail("IAE expected.");
         } catch (java.lang.IllegalArgumentException e) {
-            // excpected as $_{ is invalid
+            // expected as $_{ is invalid
         }
     }
 
@@ -129,19 +140,329 @@ 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",
+        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");
     }
 
 
     @Test
-    public void testNonAsciiQueryStringAndRedirectPath() throws Exception {
-        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/$1?$2", 
"/b/%E5%9C%A8%E7%BA%BF/id=%E6%B5%8B%E8%AF%95",
+    public void testNonAsciiQueryStringAndPath() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/$1?$2",
+                "/b/%E5%9C%A8%E7%BA%BF/id=%E6%B5%8B%E8%AF%95",
                 "/c/%E5%9C%A8%E7%BA%BF", "id=%E6%B5%8B%E8%AF%95");
     }
 
 
+    @Test
+    public void testNonAsciiQueryStringAndRedirect() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*) /c?$1 [R]",
+                "/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");
+    }
+
+
+    @Test
+    public void testNonAsciiQueryStringAndPathAndRedirect() throws Exception {
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/$1?$2 [R]",
+                "/b/%E5%9C%A8%E7%BA%BF/id=%E6%B5%8B%E8%AF%95",
+                "/c/%E5%9C%A8%E7%BA%BF", "id=%E6%B5%8B%E8%AF%95");
+    }
+
+
+    @Test
+    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");
+    }
+
+
+    @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");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsNone() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/%C2%A1%C2%A1", 
"id=%C2%A1");
+    }
+
+
+    @Test
+    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");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsR() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/%C2%A1%C2%A1", 
"id=%C2%A1");
+    }
+
+
+    @Test
+    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");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsRNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,NE]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", 
"/c/\u00C2\u00A1\u00C2\u00A1", "id=\u00C2\u00A1");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsRBNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,NE]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/\u00C2\u00A1%C2%A1", 
"id=%C2%A1");
+    }
+
+
+    @Test
+    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");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsRQSA() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,QSA]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1&di=%C2%AE");
+    }
+
+
+    @Test
+    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");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsRNEQSA() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,NE,QSA]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/\u00C2\u00A1\u00C2\u00A1",
+                "id=\u00C2\u00A1&di=\u00C2\u00AE");
+    }
+
+
+    @Test
+    public void testUtf8WithBothQsFlagsRBNEQSA() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,NE,QSA]",
+                "/b/%C2%A1/id=%C2%A1?di=%C2%AE", "/c/\u00C2\u00A1%C2%A1",
+                "id=%C2%A1&di=\u00C2\u00AE");
+    }
+
+
+    @Test
+    public void testUtf8WithOriginalQsFlagsNone() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1",
+                "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1", "id=%C2%A1");
+    }
+
+
+    @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", "id=%C2%A1");
+    }
+
+
+    @Test
+    public void testUtf8WithOriginalQsFlagsR() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R]",
+                "/b/%C2%A1?id=%C2%A1", "/c/%C2%A1%C2%A1", "id=%C2%A1");
+    }
+
+
+    @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", "id=%C2%A1");
+    }
+
+
+    @Test
+    public void testUtf8WithOriginalQsFlagsRNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,NE]",
+                "/b/%C2%A1?id=%C2%A1", "/c/\u00C2\u00A1\u00C2\u00A1", 
"id=\u00C2\u00A1");
+    }
+
+
+    @Test
+    public void testUtf8WithOriginalQsFlagsRBNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B,NE]",
+                "/b/%C2%A1?id=%C2%A1", "/c/\u00C2\u00A1%C2%A1", 
"id=\u00C2\u00A1");
+    }
+
+
+    @Test
+    public void testUtf8WithRewriteQsFlagsNone() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2",
+                "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1", "id=%C2%A1");
+    }
+
+
+    @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");
+    }
+
+
+    @Test
+    public void testUtf8WithRewriteQsFlagsR() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R]",
+                "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1", "id=%C2%A1");
+    }
+
+
+    @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
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B]",
+                "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%25C2%25A1", "id=%25C2%25A1");
+    }
+
+
+    @Test
+    public void testUtf8WithRewriteQsFlagsRNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,NE]",
+                "/b/%C2%A1/id=%C2%A1", "/c/\u00C2\u00A1\u00C2\u00A1", 
"id=\u00C2\u00A1");
+    }
+
+
+    @Test
+    public void testUtf8WithRewriteQsFlagsRBNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [R,B,NE]",
+                "/b/%C2%A1/id=%C2%A1", "/c/\u00C2\u00A1%C2%A1", "id=%C2%A1");
+    }
+
+
+    @Test
+    public void testUtf8WithRewriteQsFlagsQSA() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*)/(.*) /c/\u00A1$1?$2 [QSA]",
+                "/b/%C2%A1/id=%C2%A1", "/c/%C2%A1%C2%A1",
+                "id=%C2%A1");
+    }
+
+
+    @Test
+    public void testUtf8FlagsNone() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1", "/b/%C2%A1", 
"/c/%C2%A1%C2%A1");
+    }
+
+
+    @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");
+    }
+
+
+    @Test
+    public void testUtf8FlagsR() throws Exception {
+        // Note %C2%A1 == \u00A1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R]", "/b/%C2%A1", 
"/c/%C2%A1%C2%A1");
+    }
+
+
+    @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");
+    }
+
+
+    @Test
+    public void testUtf8FlagsRNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,NE]",
+                "/b/%C2%A1", "/c/\u00C2\u00A1\u00C2\u00A1");
+    }
+
+
+    @Test
+    public void testUtf8FlagsRBNE() throws Exception {
+        // Note %C2%A1 == \u00A1
+        // Failing to escape the redirect means UTF-8 bytes in the Location
+        // header which will be treated as if they are ISO-8859-1
+        doTestRewrite("RewriteRule ^/b/(.*) /c/\u00A1$1 [R,B,NE]",
+                "/b/%C2%A1", "/c/\u00C2\u00A1%C2%A1");
+    }
 
 
     @Test
@@ -153,6 +474,15 @@ public class TestRewriteValve extends To
     }
 
 
+    @Test
+    public void testHostRewrite() throws Exception {
+        // Based on report from users list that ':' was encoded and breaking
+        // the redirect
+        doTestRewrite("RewriteRule ^/b(.*) 
http://%{HTTP_HOST}:%{SERVER_PORT}/a$1 [R]",
+                "/b/%255A", "/a/%255A");
+    }
+
+
     private void doTestRewrite(String config, String request, String 
expectedURI) throws Exception {
         doTestRewrite(config, request, expectedURI, null);
     }
@@ -199,8 +529,8 @@ public class TestRewriteValve extends To
         }
 
         if (expectedAttributeValue != null) {
-            String attrbuteValue = requestDesc.getAttribute("X-Test");
-            Assert.assertEquals(expectedAttributeValue, attrbuteValue);
+            String attributeValue = requestDesc.getAttribute("X-Test");
+            Assert.assertEquals(expectedAttributeValue, attributeValue);
         }
     }
 }

Modified: tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml?rev=1761727&r1=1761726&r2=1761727&view=diff
==============================================================================
--- tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc8.5.x/trunk/webapps/docs/changelog.xml Wed Sep 21 13:27:10 2016
@@ -53,6 +53,13 @@
         class path entries. (markt)
       </add>
       <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 and
+        improve the handling for the <code>QSA</code> flag. Includes multiple
+        test cases by Santhana Preethiand a patch by Tiago Oliveira. (markt)
+      </fix>
+      <fix>
         <bug>60116</bug>: Fix a problem with the rewrite valve that caused back
         references evaluated in conditions to be forced to lower case when 
using
         the <code>NC</code> flag. (markt)




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

Reply via email to