Author: markt
Date: Mon Apr 24 12:07:39 2017
New Revision: 1792460

URL: http://svn.apache.org/viewvc?rev=1792460&view=rev
Log:
Review those places where Tomcat re-encodes a URI or URI component and ensure 
that that correct encoding (path differs from query string) is applied and that 
the encoding is applied consistently.

Modified:
    tomcat/trunk/java/org/apache/catalina/manager/HTMLManagerServlet.java
    tomcat/trunk/java/org/apache/catalina/ssi/SSIMediator.java
    tomcat/trunk/java/org/apache/catalina/util/URLEncoder.java
    tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
    tomcat/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/manager/HTMLManagerServlet.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/manager/HTMLManagerServlet.java?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/manager/HTMLManagerServlet.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/manager/HTMLManagerServlet.java Mon 
Apr 24 12:07:39 2017
@@ -78,7 +78,6 @@ public final class HTMLManagerServlet ex
 
     private static final long serialVersionUID = 1L;
 
-    static final URLEncoder URL_ENCODER;
     static final String APPLICATION_MESSAGE = "message";
     static final String APPLICATION_ERROR = "error";
 
@@ -86,12 +85,6 @@ public final class HTMLManagerServlet ex
     static final String sessionDetailJspPath = 
"/WEB-INF/jsp/sessionDetail.jsp";
     static final String connectorCiphersJspPath = 
"/WEB-INF/jsp/connectorCiphers.jsp";
 
-    static {
-        URL_ENCODER = new URLEncoder();
-        // '/' should not be encoded in context paths
-        URL_ENCODER.addSafeCharacter('/');
-    }
-
     private boolean showProxySessions = false;
 
     // --------------------------------------------------------- Public Methods
@@ -430,10 +423,10 @@ public final class HTMLManagerServlet ex
 
                 StringBuilder tmp = new StringBuilder();
                 tmp.append("path=");
-                tmp.append(URL_ENCODER.encode(displayPath, "UTF-8"));
+                tmp.append(URLEncoder.DEFAULT.encode(displayPath, "UTF-8"));
                 if (ctxt.getWebappVersion().length() > 0) {
                     tmp.append("&version=");
-                    tmp.append(URL_ENCODER.encode(ctxt.getWebappVersion(), 
"UTF-8"));
+                    
tmp.append(URLEncoder.DEFAULT.encode(ctxt.getWebappVersion(), "UTF-8"));
                 }
                 String pathVersion = tmp.toString();
 
@@ -445,8 +438,8 @@ public final class HTMLManagerServlet ex
                 }
 
                 args = new Object[7];
-                args[0] = "<a href=\"" + URL_ENCODER.encode(contextPath + "/", 
"UTF-8")
-                        + "\">" + RequestUtil.filter(displayPath) + "</a>";
+                args[0] = "<a href=\"" + URLEncoder.DEFAULT.encode(contextPath 
+ "/", "UTF-8") +
+                        "\">" + RequestUtil.filter(displayPath) + "</a>";
                 if ("".equals(ctxt.getWebappVersion())) {
                     args[1] = noVersion;
                 } else {

Modified: tomcat/trunk/java/org/apache/catalina/ssi/SSIMediator.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ssi/SSIMediator.java?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ssi/SSIMediator.java (original)
+++ tomcat/trunk/java/org/apache/catalina/ssi/SSIMediator.java Mon Apr 24 
12:07:39 2017
@@ -43,7 +43,6 @@ public class SSIMediator {
     protected static final String DEFAULT_CONFIG_ERR_MSG = "[an error occurred 
while processing this directive]";
     protected static final String DEFAULT_CONFIG_TIME_FMT = "%A, %d-%b-%Y %T 
%Z";
     protected static final String DEFAULT_CONFIG_SIZE_FMT = "abbrev";
-    protected static final URLEncoder urlEncoder;
     protected String configErrMsg = DEFAULT_CONFIG_ERR_MSG;
     protected String configTimeFmt = DEFAULT_CONFIG_TIME_FMT;
     protected String configSizeFmt = DEFAULT_CONFIG_SIZE_FMT;
@@ -52,22 +51,6 @@ public class SSIMediator {
     protected final long lastModifiedDate;
     protected Strftime strftime;
     protected final SSIConditionalState conditionalState = new 
SSIConditionalState();
-    static {
-        //We try to encode only the same characters that apache does
-        urlEncoder = new URLEncoder();
-        urlEncoder.addSafeCharacter(',');
-        urlEncoder.addSafeCharacter(':');
-        urlEncoder.addSafeCharacter('-');
-        urlEncoder.addSafeCharacter('_');
-        urlEncoder.addSafeCharacter('.');
-        urlEncoder.addSafeCharacter('*');
-        urlEncoder.addSafeCharacter('/');
-        urlEncoder.addSafeCharacter('!');
-        urlEncoder.addSafeCharacter('~');
-        urlEncoder.addSafeCharacter('\'');
-        urlEncoder.addSafeCharacter('(');
-        urlEncoder.addSafeCharacter(')');
-    }
 
 
     public SSIMediator(SSIExternalResolver ssiExternalResolver,
@@ -296,7 +279,7 @@ public class SSIMediator {
     protected String encode(String value, String encoding) {
         String retVal = null;
         if (encoding.equalsIgnoreCase("url")) {
-            retVal = urlEncoder.encode(value, "UTF-8");
+            retVal = URLEncoder.DEFAULT.encode(value, "UTF-8");
         } else if (encoding.equalsIgnoreCase("none")) {
             retVal = value;
         } else if (encoding.equalsIgnoreCase("entity")) {

Modified: tomcat/trunk/java/org/apache/catalina/util/URLEncoder.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/URLEncoder.java?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/util/URLEncoder.java (original)
+++ tomcat/trunk/java/org/apache/catalina/util/URLEncoder.java Mon Apr 24 
12:07:39 2017
@@ -39,17 +39,69 @@ public class URLEncoder {
      'A', 'B', 'C', 'D', 'E', 'F'};
 
     public static final URLEncoder DEFAULT = new URLEncoder();
+    public static final URLEncoder QUERY = new URLEncoder();
+
     static {
-        DEFAULT.addSafeCharacter('~');
+        /*
+         * Encoder for 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
         DEFAULT.addSafeCharacter('-');
-        DEFAULT.addSafeCharacter('_');
         DEFAULT.addSafeCharacter('.');
+        DEFAULT.addSafeCharacter('_');
+        DEFAULT.addSafeCharacter('~');
+        // Add the sub-delims
+        DEFAULT.addSafeCharacter('!');
+        DEFAULT.addSafeCharacter('$');
+        DEFAULT.addSafeCharacter('&');
+        DEFAULT.addSafeCharacter('\'');
+        DEFAULT.addSafeCharacter('(');
+        DEFAULT.addSafeCharacter(')');
         DEFAULT.addSafeCharacter('*');
+        DEFAULT.addSafeCharacter('+');
+        DEFAULT.addSafeCharacter(',');
+        DEFAULT.addSafeCharacter(';');
+        DEFAULT.addSafeCharacter('=');
+        // Add the remaining literals
+        DEFAULT.addSafeCharacter(':');
+        DEFAULT.addSafeCharacter('@');
+        // Add '/' so it isn't encoded when we encode a path
         DEFAULT.addSafeCharacter('/');
+
+        /*
+         * Encoder for query strings
+         * 
https://www.w3.org/TR/html5/forms.html#application/x-www-form-urlencoded-encoding-algorithm
+         * 0x20 ' ' -> '+'
+         * 0x2A, 0x2D, 0x2E, 0x30 to 0x39, 0x41 to 0x5A, 0x5F, 0x61 to 0x7A 
as-is
+         * '*',  '-',  '.',  '0'  to '9',  'A'  to 'Z',  '_',  'a'  to 'z'
+         * Also '=' and '&' are not encoded
+         * Everything else %nn encoded
+         */
+        // Special encoding for space
+        QUERY.setEncodeSpaceAsPlus(true);
+        // Alpha and digit are safe by default
+        // Add the other permitted characters
+        QUERY.addSafeCharacter('*');
+        QUERY.addSafeCharacter('-');
+        QUERY.addSafeCharacter('.');
+        QUERY.addSafeCharacter('_');
+        QUERY.addSafeCharacter('=');
+        QUERY.addSafeCharacter('&');
     }
 
     //Array containing the safe characters set.
-    protected final BitSet safeCharacters = new BitSet(256);
+    private final BitSet safeCharacters = new BitSet(256);
+
+    private boolean encodeSpaceAsPlus = false;
+
 
     public URLEncoder() {
         for (char i = 'a'; i <= 'z'; i++) {
@@ -63,11 +115,17 @@ public class URLEncoder {
         }
     }
 
+
     public void addSafeCharacter( char c ) {
         safeCharacters.set( c );
     }
 
 
+    public void setEncodeSpaceAsPlus(boolean encodeSpaceAsPlus) {
+        this.encodeSpaceAsPlus = encodeSpaceAsPlus;
+    }
+
+
     /**
      * URL encodes the provided path using the given encoding.
      *
@@ -92,6 +150,8 @@ public class URLEncoder {
             int c = path.charAt(i);
             if (safeCharacters.get(c)) {
                 rewrittenPath.append((char)c);
+            } else if (encodeSpaceAsPlus && c == ' ') {
+                rewrittenPath.append('+');
             } else {
                 // convert to external encoding before hex conversion
                 try {

Modified: tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/valves/rewrite/RewriteValve.java Mon 
Apr 24 12:07:39 2017
@@ -55,45 +55,6 @@ import org.apache.tomcat.util.http.Reque
 
 public class RewriteValve extends ValveBase {
 
-    static URLEncoder ENCODER = new URLEncoder();
-    static {
-        /*
-         * 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('/');
-    }
-
-
     /**
      * The rewrite rules that the valve will use.
      */
@@ -399,7 +360,7 @@ public class RewriteValve extends ValveB
                     }
 
                     StringBuffer urlStringEncoded =
-                            new StringBuffer(ENCODER.encode(urlStringDecoded, 
uriEncoding));
+                            new 
StringBuffer(URLEncoder.DEFAULT.encode(urlStringDecoded, uriEncoding));
                     if (originalQueryStringEncoded != null &&
                             originalQueryStringEncoded.length() > 0) {
                         if (rewrittenQueryStringDecoded == null) {
@@ -409,8 +370,8 @@ public class RewriteValve extends ValveB
                             if (qsa) {
                                 // if qsa is specified append the query
                                 urlStringEncoded.append('?');
-                                urlStringEncoded.append(
-                                        
ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
+                                
urlStringEncoded.append(URLEncoder.QUERY.encode(
+                                        rewrittenQueryStringDecoded, 
uriEncoding));
                                 urlStringEncoded.append('&');
                                 
urlStringEncoded.append(originalQueryStringEncoded);
                             } else if (index == urlStringEncoded.length() - 1) 
{
@@ -419,14 +380,14 @@ public class RewriteValve extends ValveB
                                 urlStringEncoded.deleteCharAt(index);
                             } else {
                                 urlStringEncoded.append('?');
-                                urlStringEncoded.append(
-                                        
ENCODER.encode(rewrittenQueryStringDecoded, uriEncoding));
+                                
urlStringEncoded.append(URLEncoder.QUERY.encode(
+                                        rewrittenQueryStringDecoded, 
uriEncoding));
                             }
                         }
                     } else if (rewrittenQueryStringDecoded != null) {
                         urlStringEncoded.append('?');
                         urlStringEncoded.append(
-                                ENCODER.encode(rewrittenQueryStringDecoded, 
uriEncoding));
+                                
URLEncoder.QUERY.encode(rewrittenQueryStringDecoded, uriEncoding));
                     }
 
                     // Insert the context if
@@ -524,7 +485,7 @@ public class RewriteValve extends ValveB
                         // This is neither decoded nor normalized
                         chunk.append(contextPath);
                     }
-                    chunk.append(ENCODER.encode(urlStringDecoded, 
uriEncoding));
+                    chunk.append(URLEncoder.DEFAULT.encode(urlStringDecoded, 
uriEncoding));
                     request.getCoyoteRequest().requestURI().toChars();
                     // Decoded and normalized URI
                     // Rewriting may have denormalized the URL
@@ -543,7 +504,7 @@ public class RewriteValve extends ValveB
                         
request.getCoyoteRequest().queryString().setString(null);
                         chunk = 
request.getCoyoteRequest().queryString().getCharChunk();
                         chunk.recycle();
-                        chunk.append(ENCODER.encode(queryStringDecoded, 
uriEncoding));
+                        
chunk.append(URLEncoder.QUERY.encode(queryStringDecoded, uriEncoding));
                         if (qsa && originalQueryStringEncoded != null &&
                                 originalQueryStringEncoded.length() > 0) {
                             chunk.append('&');

Modified: tomcat/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/valves/rewrite/Substitution.java Mon 
Apr 24 12:07:39 2017
@@ -20,6 +20,8 @@ import java.util.ArrayList;
 import java.util.Map;
 import java.util.regex.Matcher;
 
+import org.apache.catalina.util.URLEncoder;
+
 public class Substitution {
 
     public abstract class SubstitutionElement {
@@ -49,7 +51,7 @@ public class Substitution {
                 //       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(result, 
resolver.getUriEncoding());
+                return URLEncoder.DEFAULT.encode(result, 
resolver.getUriEncoding());
             } else {
                 return result;
             }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1792460&r1=1792459&r2=1792460&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Apr 24 12:07:39 2017
@@ -54,6 +54,15 @@
       </add>
     </changelog>
   </subsection>
+  <subsection name="Catalina">
+    <changelog>
+      <fix>
+        Review those places where Tomcat re-encodes a URI or URI component and
+        ensure that that correct encoding (path differs from query string) is
+        applied and that the encoding is applied consistently. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Jasper">
     <changelog>
       <fix>



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

Reply via email to