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

Reply via email to