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