Author: markt Date: Sun Jun 18 19:04:54 2017 New Revision: 1799115 URL: http://svn.apache.org/viewvc?rev=1799115&view=rev Log: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61185 When an asynchronous request is dispatched via AsyncContext.dispatch() ensure that getRequestURI() for the dispatched request matches that of the original request.
Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java?rev=1799115&r1=1799114&r2=1799115&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Sun Jun 18 19:04:54 2017 @@ -390,28 +390,35 @@ public class ApplicationContext implemen @Override - public RequestDispatcher getRequestDispatcher(String path) { + public RequestDispatcher getRequestDispatcher(final String path) { // Validate the path argument - if (path == null) + if (path == null) { return null; - if (!path.startsWith("/")) - throw new IllegalArgumentException - (sm.getString - ("applicationContext.requestDispatcher.iae", path)); - - // Get query string - String queryString = null; - String normalizedPath = path; - int pos = normalizedPath.indexOf('?'); + } + if (!path.startsWith("/")) { + throw new IllegalArgumentException( + sm.getString("applicationContext.requestDispatcher.iae", path)); + } + + // Need to separate the query string and the uri. This is required for + // the ApplicationDispatcher constructor. Mapping also requires the uri + // without the query string. + String uri; + String queryString; + int pos = path.indexOf('?'); if (pos >= 0) { - queryString = normalizedPath.substring(pos + 1); - normalizedPath = normalizedPath.substring(0, pos); + uri = path.substring(0, pos); + queryString = path.substring(pos + 1); + } else { + uri = path; + queryString = null; } - normalizedPath = RequestUtil.normalize(normalizedPath); - if (normalizedPath == null) + String normalizedPath = RequestUtil.normalize(uri); + if (normalizedPath == null) { return null; + } if (getContext().getDispatchersUseEncodedPaths()) { // Decode @@ -431,6 +438,15 @@ public class ApplicationContext implemen new IllegalArgumentException()); return null; } + + // URI needs to include the context path + uri = URLEncoder.DEFAULT.encode(getContextPath(), StandardCharsets.UTF_8) + uri; + } else { + // uri is passed to the constructor for ApplicationDispatcher and is + // ultimately used as the value for getRequestURI() which returns + // encoded values. Therefore, since the value passed in for path + // was decoded, encode uri here. + uri = URLEncoder.DEFAULT.encode(getContextPath() + uri, StandardCharsets.UTF_8); } pos = normalizedPath.length(); @@ -486,10 +502,8 @@ public class ApplicationContext implemen mappingData.recycle(); - String encodedUri = URLEncoder.DEFAULT.encode(uriCC.toString(), StandardCharsets.UTF_8); - // Construct a RequestDispatcher to process this request - return new ApplicationDispatcher(wrapper, encodedUri, wrapperPath, pathInfo, + return new ApplicationDispatcher(wrapper, uri, wrapperPath, pathInfo, queryString, mapping, null); } Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1799115&r1=1799114&r2=1799115&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Sun Jun 18 19:04:54 2017 @@ -42,7 +42,6 @@ import org.apache.catalina.Globals; 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.coyote.ActionCode; import org.apache.coyote.AsyncContextCallback; import org.apache.coyote.RequestInfo; @@ -50,6 +49,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.InstanceManager; import org.apache.tomcat.util.ExceptionUtils; +import org.apache.tomcat.util.buf.UDecoder; import org.apache.tomcat.util.res.StringManager; public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { @@ -151,21 +151,21 @@ public class AsyncContextImpl implements public void dispatch() { check(); String path; - String pathInfo; + String cpath; ServletRequest servletRequest = getRequest(); if (servletRequest instanceof HttpServletRequest) { HttpServletRequest sr = (HttpServletRequest) servletRequest; - path = sr.getServletPath(); - pathInfo = sr.getPathInfo(); + path = sr.getRequestURI(); + cpath = sr.getContextPath(); } else { - path = request.getServletPath(); - pathInfo = request.getPathInfo(); + path = request.getRequestURI(); + cpath = request.getContextPath(); } - if (pathInfo != null) { - path += pathInfo; + if (cpath.length() > 1) { + path = path.substring(cpath.length()); } - if (this.context.getDispatchersUseEncodedPaths()) { - path = URLEncoder.DEFAULT.encode(path, StandardCharsets.UTF_8); + if (!context.getDispatchersUseEncodedPaths()) { + path = UDecoder.URLDecode(path, StandardCharsets.UTF_8); } dispatch(path); } Modified: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1799115&r1=1799114&r2=1799115&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Sun Jun 18 19:04:54 2017 @@ -50,6 +50,7 @@ import static org.junit.Assert.assertNot import static org.junit.Assert.assertTrue; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.apache.catalina.Context; @@ -61,7 +62,6 @@ import org.apache.catalina.startup.Tomca import org.apache.catalina.valves.TesterAccessLogValve; import org.apache.tomcat.unittest.TesterContext; import org.apache.tomcat.util.buf.ByteChunk; -import org.apache.tomcat.util.buf.UDecoder; import org.apache.tomcat.util.descriptor.web.ErrorPage; import org.easymock.EasyMock; @@ -2545,7 +2545,6 @@ public class TestAsyncContextImpl extend req.setAttribute("count", Integer.valueOf(count)); String encodedUri = req.getRequestURI(); - String decodedUri = UDecoder.URLDecode(encodedUri); try { // Just here to trigger the error @@ -2560,7 +2559,7 @@ public class TestAsyncContextImpl extend resp.getWriter().print("OK"); } else { AsyncContext ac = req.startAsync(); - ac.dispatch(decodedUri); + ac.dispatch(encodedUri); } } } @@ -2583,7 +2582,6 @@ public class TestAsyncContextImpl extend req.setAttribute("count", Integer.valueOf(count)); String encodedUri = req.getRequestURI(); - String decodedUri = UDecoder.URLDecode(encodedUri); try { // Just here to trigger the error @@ -2597,9 +2595,63 @@ public class TestAsyncContextImpl extend resp.setContentType("text/plain"); resp.getWriter().print("OK"); } else { - RequestDispatcher rd = req.getRequestDispatcher(decodedUri); + RequestDispatcher rd = req.getRequestDispatcher(encodedUri); rd.forward(req, resp); } } } + + + @Before + public void setup() { + // Required by testBug61185() + // Does not impact other tests in this class + System.setProperty("org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH", "true"); + } + + + @Test + public void testBug61185() throws Exception { + // Setup Tomcat instance + Tomcat tomcat = getTomcatInstance(); + + // No file system docBase required + Context ctx = tomcat.addContext("", null); + + EncodedDispatchServlet encodedDispatchServlet = new EncodedDispatchServlet(); + Wrapper wrapper = Tomcat.addServlet(ctx, "encodedDispatchServlet", encodedDispatchServlet); + wrapper.setAsyncSupported(true); + ctx.addServletMappingDecoded("/*", "encodedDispatchServlet"); + + tomcat.start(); + + ByteChunk body = new ByteChunk(); + int rc = getUrl("http://localhost:" + getPort() + EncodedDispatchServlet.ENCODED_URI, body, null); + + assertEquals(HttpServletResponse.SC_OK, rc); + assertEquals("OK", body.toString()); + } + + + private static final class EncodedDispatchServlet extends HttpServlet { + + private static final long serialVersionUID = 1L; + + private static final String ENCODED_URI = "/foo/vv%2F1234/add/2"; + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + if (DispatcherType.ASYNC == req.getDispatcherType()) { + if (ENCODED_URI.equals(req.getRequestURI())) { + resp.getWriter().print("OK"); + } else { + resp.getWriter().print("FAIL"); + } + } else { + AsyncContext ac = req.startAsync(); + ac.dispatch(); + } + } + + } } Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1799115&r1=1799114&r2=1799115&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Sun Jun 18 19:04:54 2017 @@ -113,6 +113,12 @@ identify crawlers based on their IP address. Based on a patch provided by Tetradeus. (violetagg) </add> + <fix> + <bug>61185</bug>: When an asynchronous request is dispatched via + <code>AsyncContext.dispatch()</code> ensure that + <code>getRequestURI()</code> for the dispatched request matches that of + the original request. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org