This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2ee6b5c9f1cbe8b675bbc79f5edde915a66df020 Author: Mark Thomas <[email protected]> AuthorDate: Tue Nov 3 16:07:39 2026 +0000 Refactor AsyncContextImpl.dispatch() to better handle %nn encoded URIs --- .../org/apache/catalina/core/AsyncContextImpl.java | 29 +++------ java/org/apache/catalina/util/RequestUtil.java | 43 +++++++------ .../apache/catalina/core/TestAsyncContextImpl.java | 71 ++++++++++++++++++++++ 3 files changed, 105 insertions(+), 38 deletions(-) diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java index 57e238d0ed..655215eed3 100644 --- a/java/org/apache/catalina/core/AsyncContextImpl.java +++ b/java/org/apache/catalina/core/AsyncContextImpl.java @@ -18,7 +18,6 @@ package org.apache.catalina.core; import java.io.IOException; import java.io.Serial; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -42,7 +41,7 @@ import org.apache.catalina.Context; import org.apache.catalina.Host; import org.apache.catalina.Valve; import org.apache.catalina.connector.Request; -import org.apache.catalina.util.URLEncoder; +import org.apache.catalina.util.RequestUtil; import org.apache.coyote.ActionCode; import org.apache.coyote.AsyncContextCallback; import org.apache.coyote.RequestInfo; @@ -220,29 +219,19 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { @Override public void dispatch() { - String path; - String pathInfo; Request request = this.request; // Calls check() so local copies are validated - ServletRequest servletRequest = getRequest(); - if (servletRequest instanceof HttpServletRequest sr) { - path = sr.getServletPath(); - pathInfo = sr.getPathInfo(); - } else { - path = request.getServletPath(); - pathInfo = request.getPathInfo(); - } - if (pathInfo != null && !pathInfo.isEmpty()) { - path = path + pathInfo; + HttpServletRequest hsr = getRequest() instanceof HttpServletRequest h ? h : request; + String path = hsr.getRequestURI(); + String cpath = RequestUtil.getContextPathEncoded(request.getConnector(), hsr); + if (cpath.length() > 1) { + path = path.substring(cpath.length()); } /* - * This is a dispatch of the original request path. That path will always be URI-decoded. That will cause - * problems in ServletContext.getRequestDispatcher() if the decoded URI contains a literal '?' as it will be - * treated as a query delimiter. Therefore, ignore context.getDispatchersUseEncodedPaths() here and always - * encode. Also need to mark this request as ignoring context.getDispatchersUseEncodedPaths() so it is handled - * correctly in ServletContext.getRequestDispatcher(). + * This is a dispatch of the original request path. That path (obtained from request.getRequestURI()) will + * always be URL-encoded. Therefore, ignore context.getDispatchersUseEncodedPaths() here and mark as always + * encoded so it is handled correctly in ServletContext.getRequestDispatcher(). */ - path = URLEncoder.DEFAULT.encode(path, StandardCharsets.UTF_8); try { uriEncoded.set(Boolean.TRUE); dispatch(path); diff --git a/java/org/apache/catalina/util/RequestUtil.java b/java/org/apache/catalina/util/RequestUtil.java index 332e93afce..74ef26f423 100644 --- a/java/org/apache/catalina/util/RequestUtil.java +++ b/java/org/apache/catalina/util/RequestUtil.java @@ -22,7 +22,6 @@ import java.util.Enumeration; import jakarta.servlet.http.HttpServletRequest; -import org.apache.catalina.Context; import org.apache.catalina.connector.Connector; import org.apache.catalina.connector.Request; import org.apache.tomcat.util.buf.EncodedSolidusHandling; @@ -208,32 +207,28 @@ public final class RequestUtil { } - public static String getContextPathUndecoded(Request request) { + /** + * Obtains the original, encoded, non-normalized (including sequences of multiple '/') context path as provided by + * the user agent. + * + * @param connector The Connector on which the request was received + * @param httpServletRequest The request from which the original context path should be extracted + * + * @return The original, encoded, non-normalized context path + */ + public static String getContextPathEncoded(Connector connector, HttpServletRequest httpServletRequest) { - int lastSlash = request.getMappingData().contextSlashCount; + String canonicalContextPath = httpServletRequest.getServletContext().getContextPath(); + int lastSlash = slashCount(canonicalContextPath); // Special case handling for the root context if (lastSlash == 0) { return ""; } - String uri = request.getRequestURI(); - Context context = request.getContext(); - String canonicalContextPath = request.getServletContext().getContextPath(); - Connector connector = request.getConnector(); + String uri = httpServletRequest.getRequestURI(); EncodedSolidusHandling encodedSolidusHandling = connector.getEncodedSolidusHandlingInternal(); int pos = 0; - if (!context.getAllowMultipleLeadingForwardSlashInPath()) { - // Ensure that the returned value only starts with a single '/'. - // This prevents the value being misinterpreted as a protocol-relative - // URI if used with sendRedirect(). - do { - pos++; - } while (pos < uri.length() && uri.charAt(pos) == '/'); - pos--; - uri = uri.substring(pos); - } - char[] uriChars = uri.toCharArray(); // Need at least the number of slashes in the context path while (lastSlash > 0) { @@ -324,4 +319,16 @@ public final class RequestUtil { } return -1; } + + + private static int slashCount(String uri) { + int result = 0; + int start = 0; + while ((start = uri.indexOf('/', start)) > -1) { + start++; + result++; + } + + return result; + } } diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java index dadf500f06..fd1bb765dc 100644 --- a/test/org/apache/catalina/core/TestAsyncContextImpl.java +++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java @@ -66,11 +66,13 @@ import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; import org.apache.catalina.valves.TesterAccessLogValve; import org.apache.catalina.valves.ValveBase; +import org.apache.juli.logging.LogFactory; import org.apache.tomcat.unittest.TesterContext; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.EncodedSolidusHandling; import org.apache.tomcat.util.collections.CaseInsensitiveKeyMap; import org.apache.tomcat.util.descriptor.web.ErrorPage; +import org.apache.tomcat.util.scan.StandardJarScanner; import org.easymock.EasyMock; public class TestAsyncContextImpl extends TomcatBaseTest { @@ -2520,6 +2522,73 @@ public class TestAsyncContextImpl extends TomcatBaseTest { } + /* + * A %3b in the request URI is an encoded semi-colon that is part of the path, not a path parameter delimiter. After + * AsyncContext.dispatch() the servlet should see exactly the same path info as it saw for the original request. + */ + @Test + public void testAsyncDispatchEncodedSemicolon() throws Exception { + Tomcat tomcat = getTomcatInstance(); + Context context = tomcat.addContext("/test", null); + // Disable class path scanning - it slows the tests down by almost an order of magnitude + ((StandardJarScanner) context.getJarScanner()).setScanClassPath(false); + + Wrapper w = Tomcat.addServlet(context, "semicolon", new AsyncDispatchPathInfoServlet()); + w.setAsyncSupported(true); + context.addServletMapping("/semicolon/*", "semicolon"); + tomcat.start(); + + ByteChunk responseBody = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort() + "/a/b/../../test/semicolon/foo%3bbar", responseBody, null); + + Assert.assertEquals(200, rc); + // Same pathInfo before and after the dispatch + Assert.assertEquals("/foo;bar|/foo;bar", responseBody.toString()); + } + + + /* + * Multiple leading slashes in the request URI must not break AsyncContext.dispatch(). The servlet should see + * exactly the same path info before and after the dispatch. + */ + @Test + public void testAsyncDispatchMultipleLeadingSlashes() throws Exception { + Tomcat tomcat = getTomcatInstance(); + Context context = tomcat.addContext("/test", null); + // Disable class path scanning - it slows the tests down by almost an order of magnitude + ((StandardJarScanner) context.getJarScanner()).setScanClassPath(false); + + Wrapper w = Tomcat.addServlet(context, "slashes", new AsyncDispatchPathInfoServlet()); + w.setAsyncSupported(true); + context.addServletMapping("/slashes/*", "slashes"); + tomcat.start(); + + ByteChunk responseBody = new ByteChunk(); + int rc = getUrl("http://localhost:"; + getPort() + "//test/slashes/foo", responseBody, null); + + Assert.assertEquals(200, rc); + Assert.assertEquals("/foo|/foo", responseBody.toString()); + } + + + private static class AsyncDispatchPathInfoServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + if (req.getAttribute("first") == null) { + req.setAttribute("first", req.getPathInfo()); + AsyncContext ac = req.startAsync(); + ac.dispatch(); + } else { + resp.setContentType("text/plain"); + resp.getWriter().print(req.getAttribute("first") + "|" + req.getPathInfo()); + } + } + } + + private static class AsyncDispatchUrlWithSpacesServlet extends HttpServlet { private static final long serialVersionUID = 1L; @@ -2666,6 +2735,8 @@ public class TestAsyncContextImpl extends TomcatBaseTest { private void doTestAsyncIoEnd(boolean useThread, boolean useComplete) throws Exception { + // Make sure the logger exists before setting the level + LogFactory.getLog("org.apache.coyote"); LogManager.getLogManager().getLogger("org.apache.coyote").setLevel(Level.ALL); try { Tomcat tomcat = getTomcatInstance(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
