This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push: new ad084202eb Add attributes to Context to control decoding RequestDispatcher path ad084202eb is described below commit ad084202eb3643c5e5f679fb2a9cd8c66ad4603f Author: Mark Thomas <ma...@apache.org> AuthorDate: Thu Jan 23 12:08:56 2025 +0000 Add attributes to Context to control decoding RequestDispatcher path Adds encodedReverseSolidusHandling and encodedSolidusHandling. Both default to DECODE (the current behaviour) --- java/org/apache/catalina/Context.java | 68 +++++++++ .../apache/catalina/core/ApplicationContext.java | 3 +- java/org/apache/catalina/core/StandardContext.java | 41 ++++++ .../apache/tomcat/util/buf/LocalStrings.properties | 2 + java/org/apache/tomcat/util/buf/UDecoder.java | 99 +++++++++++-- ...estApplicationContextGetRequestDispatcherC.java | 161 +++++++++++++++++++++ webapps/docs/changelog.xml | 6 + webapps/docs/config/context.xml | 40 +++++ 8 files changed, 405 insertions(+), 15 deletions(-) diff --git a/java/org/apache/catalina/Context.java b/java/org/apache/catalina/Context.java index 1e142c3bcb..1c724e9a2a 100644 --- a/java/org/apache/catalina/Context.java +++ b/java/org/apache/catalina/Context.java @@ -25,6 +25,7 @@ import java.util.Locale; import java.util.Map; import java.util.Set; +import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContainerInitializer; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletRegistration; @@ -36,6 +37,7 @@ import org.apache.catalina.deploy.NamingResourcesImpl; import org.apache.tomcat.ContextBind; import org.apache.tomcat.InstanceManager; import org.apache.tomcat.JarScanner; +import org.apache.tomcat.util.buf.EncodedSolidusHandling; import org.apache.tomcat.util.descriptor.web.ApplicationParameter; import org.apache.tomcat.util.descriptor.web.ErrorPage; import org.apache.tomcat.util.descriptor.web.FilterDef; @@ -1887,4 +1889,70 @@ public interface Context extends Container, ContextBind { */ @Deprecated void setUseBloomFilterForArchives(boolean useBloomFilterForArchives); + + + /** + * Obtain the current configuration for the handling of encoded reverse solidus (%5c - \) characters in paths used + * to obtain {@link RequestDispatcher} instances for this {@link Context}. + * + * @return Obtain the current configuration for the handling of encoded reverse solidus characters + */ + default String getEncodedReverseSolidusHandling() { + return EncodedSolidusHandling.DECODE.getValue(); + } + + + /** + * Configure the handling for encoded reverse solidus (%5c - \) characters in paths used to obtain + * {@link RequestDispatcher} instances for this {@link Context}. + * + * @param encodedReverseSolidusHandling One of the values of {@link EncodedSolidusHandling} + */ + default void setEncodedReverseSolidusHandling(String encodedReverseSolidusHandling) { + throw new UnsupportedOperationException(); + } + + + /** + * Obtain the current configuration for the handling of encoded reverse solidus (%5c - \) characters in paths used + * to obtain {@link RequestDispatcher} instances for this {@link Context}. + * + * @return Obtain the current configuration for the handling of encoded reverse solidus characters + */ + default EncodedSolidusHandling getEncodedReverseSolidusHandlingEnum() { + return EncodedSolidusHandling.DECODE; + } + + + /** + * Obtain the current configuration for the handling of encoded solidus (%2f - /) characters in paths used to obtain + * {@link RequestDispatcher} instances for this {@link Context}. + * + * @return Obtain the current configuration for the handling of encoded solidus characters + */ + default String getEncodedSolidusHandling() { + return EncodedSolidusHandling.DECODE.getValue(); + } + + + /** + * Configure the handling for encoded solidus (%2f - /) characters in paths used to obtain {@link RequestDispatcher} + * instances for this {@link Context}. + * + * @param encodedSolidusHandling One of the values of {@link EncodedSolidusHandling} + */ + default void setEncodedSolidusHandling(String encodedSolidusHandling) { + throw new UnsupportedOperationException(); + } + + + /** + * Obtain the current configuration for the handling of encoded solidus (%2f - /) characters in paths used to obtain + * {@link RequestDispatcher} instances for this {@link Context}. + * + * @return Obtain the current configuration for the handling of encoded solidus characters + */ + default EncodedSolidusHandling getEncodedSolidusHandlingEnum() { + return EncodedSolidusHandling.DECODE; + } } diff --git a/java/org/apache/catalina/core/ApplicationContext.java b/java/org/apache/catalina/core/ApplicationContext.java index 5e21d53e5e..06ff77b702 100644 --- a/java/org/apache/catalina/core/ApplicationContext.java +++ b/java/org/apache/catalina/core/ApplicationContext.java @@ -378,7 +378,8 @@ public class ApplicationContext implements ServletContext { // Decode only if the uri derived from the provided path is expected to be encoded if (getContext().getDispatchersUseEncodedPaths()) { - uriToMap = UDecoder.URLDecode(uriToMap, StandardCharsets.UTF_8); + uriToMap = UDecoder.URLDecode(uriToMap, StandardCharsets.UTF_8, context.getEncodedSolidusHandlingEnum(), + context.getEncodedReverseSolidusHandlingEnum()); } // Then normalize diff --git a/java/org/apache/catalina/core/StandardContext.java b/java/org/apache/catalina/core/StandardContext.java index fe2dd7cbca..bfaf966140 100644 --- a/java/org/apache/catalina/core/StandardContext.java +++ b/java/org/apache/catalina/core/StandardContext.java @@ -116,6 +116,7 @@ import org.apache.tomcat.InstanceManager; import org.apache.tomcat.InstanceManagerBindings; import org.apache.tomcat.JarScanner; import org.apache.tomcat.util.ExceptionUtils; +import org.apache.tomcat.util.buf.EncodedSolidusHandling; import org.apache.tomcat.util.buf.StringUtils; import org.apache.tomcat.util.compat.JreCompat; import org.apache.tomcat.util.descriptor.XmlIdentifiers; @@ -806,9 +807,49 @@ public class StandardContext extends ContainerBase implements Context, Notificat private int notFoundClassResourceCacheSize = 1000; + private EncodedSolidusHandling encodedReverseSolidusHandling = EncodedSolidusHandling.DECODE; + + private EncodedSolidusHandling encodedSolidusHandling = EncodedSolidusHandling.DECODE; + // ----------------------------------------------------- Context Properties + @Override + public String getEncodedReverseSolidusHandling() { + return encodedReverseSolidusHandling.getValue(); + } + + + @Override + public void setEncodedReverseSolidusHandling(String encodedReverseSolidusHandling) { + this.encodedReverseSolidusHandling = EncodedSolidusHandling.fromString(encodedReverseSolidusHandling); + } + + + @Override + public EncodedSolidusHandling getEncodedReverseSolidusHandlingEnum() { + return encodedReverseSolidusHandling; + } + + + @Override + public String getEncodedSolidusHandling() { + return encodedSolidusHandling.getValue(); + } + + + @Override + public void setEncodedSolidusHandling(String encodedSolidusHandling) { + this.encodedSolidusHandling = EncodedSolidusHandling.fromString(encodedSolidusHandling); + } + + + @Override + public EncodedSolidusHandling getEncodedSolidusHandlingEnum() { + return encodedSolidusHandling; + } + + public int getNotFoundClassResourceCacheSize() { return notFoundClassResourceCacheSize; } diff --git a/java/org/apache/tomcat/util/buf/LocalStrings.properties b/java/org/apache/tomcat/util/buf/LocalStrings.properties index ed7dacc86e..0758ebd15a 100644 --- a/java/org/apache/tomcat/util/buf/LocalStrings.properties +++ b/java/org/apache/tomcat/util/buf/LocalStrings.properties @@ -49,3 +49,5 @@ uDecoder.noBackslash=The encoded backslash character is not allowed uDecoder.noSlash=The encoded slash character is not allowed uDecoder.urlDecode.conversionError=Failed to decode [{0}] using character set [{1}] uDecoder.urlDecode.missingDigit=Failed to decode [{0}] because the % character must be followed by two hexadecimal digits +uDecoder.urlDecode.rejectEncodedReverseSolidus=Failed to decode [{0}] because it contained a %5c sequence the decoder is configured to reject +uDecoder.urlDecode.rejectEncodedSolidus=Failed to decode [{0}] because it contained a %2f sequence the decoder is configured to reject diff --git a/java/org/apache/tomcat/util/buf/UDecoder.java b/java/org/apache/tomcat/util/buf/UDecoder.java index 3a1fa6edbf..da63fa030c 100644 --- a/java/org/apache/tomcat/util/buf/UDecoder.java +++ b/java/org/apache/tomcat/util/buf/UDecoder.java @@ -169,20 +169,20 @@ public final class UDecoder { } } } else if (res == '\\') { - switch (encodedReverseSolidusHandling) { - case DECODE: { - buff[idx] = (byte) res; - break; - } - case REJECT: { - throw EXCEPTION_BACKSLASH; - } - case PASS_THROUGH: { - buff[idx++] = buff[j - 2]; - buff[idx++] = buff[j - 1]; - buff[idx] = buff[j]; - } + switch (encodedReverseSolidusHandling) { + case DECODE: { + buff[idx] = (byte) res; + break; + } + case REJECT: { + throw EXCEPTION_BACKSLASH; } + case PASS_THROUGH: { + buff[idx++] = buff[j - 2]; + buff[idx++] = buff[j - 1]; + buff[idx] = buff[j]; + } + } } else if (res == '%') { /* * If encoded '/' or '\' is going to be left encoded then so must encoded '%' else the subsequent @@ -218,6 +218,24 @@ public final class UDecoder { * @exception IllegalArgumentException if a '%' character is not followed by a valid 2-digit hexadecimal number */ public static String URLDecode(String str, Charset charset) { + return URLDecode(str, charset, EncodedSolidusHandling.DECODE, EncodedSolidusHandling.DECODE); + } + + + /** + * Decode and return the specified URL-encoded String. It is assumed the string is not a query string. + * + * @param str The url-encoded string + * @param charset The character encoding to use; if null, UTF-8 is used. + * @param encodedSolidusHandling The required handling of encoded solidus (%2f - /) + * @param encodedReverseSolidusHandling The required handling of encoded reverse solidus (%5c - \) + * + * @return the decoded string + * + * @exception IllegalArgumentException if a '%' character is not followed by a valid 2-digit hexadecimal number + */ + public static String URLDecode(String str, Charset charset, EncodedSolidusHandling encodedSolidusHandling, + EncodedSolidusHandling encodedReverseSolidusHandling) { if (str == null) { return null; } @@ -266,7 +284,60 @@ public final class UDecoder { char c1 = sourceChars[ix++]; char c2 = sourceChars[ix++]; if (isHexDigit(c1) && isHexDigit(c2)) { - baos.write(x2c(c1, c2)); + int decoded = x2c(c1, c2); + switch (decoded) { + case '/': { + switch (encodedSolidusHandling) { + case DECODE: { + osw.append('/'); + break; + } + case PASS_THROUGH: { + osw.append(c); + osw.append(c1); + osw.append(c2); + break; + } + case REJECT: { + throw new IllegalArgumentException( + sm.getString("uDecoder.urlDecode.rejectEncodedSolidus", str)); + } + } + break; + } + case '\\': { + switch (encodedReverseSolidusHandling) { + case DECODE: { + osw.append('\\'); + break; + } + case PASS_THROUGH: { + osw.append(c); + osw.append(c1); + osw.append(c2); + break; + } + case REJECT: { + throw new IllegalArgumentException( + sm.getString("uDecoder.urlDecode.rejectEncodedReverseSolidus", str)); + } + } + break; + } + case '%': { + if (encodedReverseSolidusHandling == EncodedSolidusHandling.PASS_THROUGH || + encodedSolidusHandling == EncodedSolidusHandling.PASS_THROUGH) { + osw.append(c); + osw.append(c1); + osw.append(c2); + } else { + baos.write('%'); + } + break; + } + default: + baos.write(decoded); + } } else { throw new IllegalArgumentException(sm.getString("uDecoder.urlDecode.missingDigit", str)); } diff --git a/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherC.java b/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherC.java new file mode 100644 index 0000000000..ff5fd02d21 --- /dev/null +++ b/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherC.java @@ -0,0 +1,161 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.catalina.core; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Collection; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +import org.apache.catalina.Context; +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.buf.ByteChunk; + +/* + * Tests interaction of ServletContext.getRequestDispatcher() and the Connector/Context attributes + * encodedSolidusHandling and encodedReverseSolidusHandling. + */ +@RunWith(value = Parameterized.class) +public class TestApplicationContextGetRequestDispatcherC extends TomcatBaseTest { + + @Parameters(name = "{index}: pathInfoRequest[{0}], pathInfoDispatcher[{1}]") + public static Collection<Object[]> data() { + return Arrays.asList(new Object[][]{ + // Defaults + { "/a/b/c", "/a/b/c", null, null }, + { "/a%2fb%5cc/%25", "/a/b/c/%", "decode", "decode" }, + { "/a%2fb%5cc/%25", "/a/b%5cc/%25", "decode", "passthrough" }, + { "/a%2fb%5cc/%25", "/a%2fb/c/%25", "passthrough", "decode" }, + { "/a%2fb%5cc/%25", "/a%2fb%5cc/%25", "passthrough", "passthrough" }, + }); + } + + + @Parameter(0) + public String pathInfoRequest; + @Parameter(1) + public String pathInfoDispatcher; + @Parameter(2) + public String encodedSolidusHandling; + @Parameter(3) + public String encodedReverseSolidusHandling; + + + @Test + public void testSomething() throws Exception { + doTest(); + } + + + private void doTest() throws Exception { + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // Configure the Connector + tomcat.getConnector().setAllowBackslash(true); + if (encodedSolidusHandling != null) { + tomcat.getConnector().setEncodedSolidusHandling(encodedSolidusHandling); + } + if (encodedReverseSolidusHandling != null) { + tomcat.getConnector().setEncodedReverseSolidusHandling(encodedReverseSolidusHandling); + } + + // No file system docBase required + Context ctx = tomcat.addContext("/test", null); + ctx.addWelcomeFile("index.html"); + if (encodedSolidusHandling != null) { + ctx.setEncodedSolidusHandling(encodedSolidusHandling); + } + if (encodedReverseSolidusHandling != null) { + ctx.setEncodedReverseSolidusHandling(encodedReverseSolidusHandling); + } + + // Servlet that performs a dispatch to ... + Tomcat.addServlet(ctx, "rd", new Dispatch(pathInfoRequest)); + ctx.addServletMappingDecoded("/dispatch/*", "rd"); + + // ... this Servlet + Tomcat.addServlet(ctx, "target", new Target()); + ctx.addServletMappingDecoded("/target/*", "target"); + + tomcat.start(); + + StringBuilder url = new StringBuilder("http://localhost:"); + url.append(getPort()); + url.append("/test/dispatch"); + url.append(pathInfoRequest); + + ByteChunk bc = getUrl(url.toString()); + String body = bc.toString(); + + Assert.assertEquals(pathInfoDispatcher + pathInfoDispatcher, body); + } + + + private static class Dispatch extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private final String dispatchPath; + + Dispatch(String dispatchPath) { + this.dispatchPath = dispatchPath; + } + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + /* + * The original pathInfo as presented by the client and the dispatchPath are currently the same for this + * test, as is the handling configured in the Connector and Context. The test checks both the normal request + * processing and the dispatch processing for the correct handling of %2f and %5c. The test could be + * expanded so different settings are used for the Connector and the Context. + */ + // Handle an edge case - the application needs to encode any paths used to obtain a RequestDispatcher + String pathInfo = req.getPathInfo(); + if (pathInfo.endsWith("%")) { + pathInfo = pathInfo.replace("%", "%25"); + } + req.getServletContext().getRequestDispatcher("/target" + pathInfo + dispatchPath).forward(req, resp); + } + } + + + private static class Target extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.setContentType("text/plain"); + resp.setCharacterEncoding(StandardCharsets.UTF_8); + resp.getWriter().print(req.getPathInfo()); + } + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 85986319d2..5ee3a83665 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -170,6 +170,12 @@ the processing of the provided path is consistent with normal request processing. (markt) </scode> + <add> + Add <code>encodedReverseSolidusHandling</code> and + <code>encodedSolidusHandling</code> attributes to Context to provide + control over the handling of the path used to created a + <code>RequestDispatcher</code>. (markt) + </add> </changelog> </subsection> <subsection name="Coyote"> diff --git a/webapps/docs/config/context.xml b/webapps/docs/config/context.xml index 10a6752c4b..36c7ab0c3c 100644 --- a/webapps/docs/config/context.xml +++ b/webapps/docs/config/context.xml @@ -409,6 +409,46 @@ sufficient.</p> </attribute> + <attribute name="encodedReverseSolidusHandling" required="false"> + <p>When set to <code>reject</code>, an attempt to obtain a + <code>RequestDispatcher</code> for a path containing a <code>%5c</code> + sequence will trigger an <code>IllegalArgumentException</code>. When set + to <code>decode</code>, any <code>%5c</code> sequence in the path used + to obtain a <code>RequestDispatcher</code> will have that sequence + decoded to <code>\</code> (and then normalized to <code>/</code>) before + the <code>RequestDispatcher</code> is created. When set to + <code>passthrough</code>, any <code>%5c</code> sequence in the path used + to obtain a <code>RequestDispatcher</code> will remain unchanged.</p> + <p>If <code>passthrough</code> is used then it is the application's + resposibility to perform any further <code>%nn</code> decoding required. + Any <code>%25</code> sequences (encoded <code>%</code>) in the path will + also be processed with the <code>%25</code> sequence unchanged + to avoid potential corruption and/or decoding failure when the path is + subsequently <code>%nn</code> decoded by the application.</p> + <p>If not specified, the default value is <code>decode</code>.</p> + </attribute> + + <attribute name="encodedSolidusHandling" required="false"> + <p>When set to <code>reject</code>, an attempt to obtain a + <code>RequestDispatcher</code> for a path containing a <code>%2f</code> + sequence will trigger an <code>IllegalArgumentException</code>. When set + to <code>decode</code>, any <code>%2f</code> sequence in the path used + to obtain a <code>RequestDispatcher</code> will have that sequence + decoded to <code>/</code> before the <code>RequestDispatcher</code> is + created. When set to <code>passthrough</code>, any <code>%2f</code> + sequence in the path used to obtain a <code>RequestDispatcher</code> + will remain unchanged.</p> + <p>If <code>passthrough</code> is used then it is the application's + resposibility to perform any further <code>%nn</code> decoding required. + Any <code>%25</code> sequences (encoded <code>%</code>) in the path will + also be processed with the <code>%25</code> sequence unchanged + to avoid potential corruption and/or decoding failure when the path is + subsequently <code>%nn</code> decoded by the application.</p> + <p>If not specified, the default value is <code>decode</code>. This + default will change to <code>reject</code> (to align with the + <strong>Connector</strong>) in Tomcat 12.</p> + </attribute> + <attribute name="failCtxIfServletStartFails" required="false"> <p>Set to <code>true</code> to have the context fail its startup if any servlet that has load-on-startup >=0 fails its own startup.</p> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org