Author: markt
Date: Wed Aug 29 19:29:46 2018
New Revision: 1839604

URL: http://svn.apache.org/viewvc?rev=1839604&view=rev
Log:
Improve the handling of path parameters when working with RequestDispatcher 
objects.

Added:
    
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
   (with props)
    
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java
   (with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1839604&r1=1839603&r2=1839604&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Wed Aug 29 
19:29:46 2018
@@ -1360,6 +1360,25 @@ public class Request implements HttpServ
             return context.getServletContext().getRequestDispatcher(path);
         }
 
+        /*
+         * Relative to what, exactly?
+         *
+         * From the Servlet 4.0 Javadoc:
+         * - The pathname specified may be relative, although it cannot extend
+         *   outside the current servlet context.
+         * - If it is relative, it must be relative against the current servlet
+         *
+         * From Section 9.1 of the spec:
+         * - The servlet container uses information in the request object to
+         *   transform the given relative path against the current servlet to a
+         *   complete path.
+         *
+         * It is undefined whether the requestURI is used or whether 
servletPath
+         * and pathInfo are used. Given that the RequestURI includes the
+         * contextPath (and extracting that is messy) , using the servletPath 
and
+         * pathInfo looks to be the more reasonable choice.
+         */
+
         // Convert a request-relative path to a context-relative one
         String servletPath = (String) getAttribute(
                 RequestDispatcher.INCLUDE_SERVLET_PATH);

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=1839604&r1=1839603&r2=1839604&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java 
(original)
+++ tomcat/trunk/java/org/apache/catalina/core/ApplicationContext.java Wed Aug 
29 19:29:46 2018
@@ -403,9 +403,8 @@ public class ApplicationContext implemen
                     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.
+        // Same processing order as InputBuffer / CoyoteAdapter
+        // First remove query string
         String uri;
         String queryString;
         int pos = path.indexOf('?');
@@ -417,18 +416,24 @@ public class ApplicationContext implemen
             queryString = null;
         }
 
-        String normalizedPath = RequestUtil.normalize(uri);
-        if (normalizedPath == null) {
+        // Remove path parameters
+        String uriNoParams = stripPathParams(uri);
+
+        // Then normalize
+        String normalizedUri = RequestUtil.normalize(uriNoParams);
+        if (normalizedUri == null) {
             return null;
         }
 
+        // Mapping is against the normalized uri
+
         if (getContext().getDispatchersUseEncodedPaths()) {
             // Decode
-            String decodedPath = UDecoder.URLDecode(normalizedPath);
+            String decodedUri = UDecoder.URLDecode(normalizedUri);
 
             // Security check to catch attempts to encode /../ sequences
-            normalizedPath = RequestUtil.normalize(decodedPath);
-            if (!decodedPath.equals(normalizedPath)) {
+            normalizedUri = RequestUtil.normalize(decodedUri);
+            if (!decodedUri.equals(normalizedUri)) {
                 getContext().getLogger().warn(
                         sm.getString("applicationContext.illegalDispatchPath", 
path),
                         new IllegalArgumentException());
@@ -445,7 +450,7 @@ public class ApplicationContext implemen
             uri = URLEncoder.DEFAULT.encode(getContextPath() + uri, 
StandardCharsets.UTF_8);
         }
 
-        pos = normalizedPath.length();
+        pos = normalizedUri.length();
 
         // Use the thread local URI and mapping data
         DispatchData dd = dispatchData.get();
@@ -464,28 +469,12 @@ public class ApplicationContext implemen
             // Map the URI
             CharChunk uriCC = uriMB.getCharChunk();
             try {
-                uriCC.append(context.getPath(), 0, context.getPath().length());
-                /*
-                 * Ignore any trailing path params (separated by ';') for 
mapping
-                 * purposes
-                 */
-                int semicolon = normalizedPath.indexOf(';');
-                if (pos >= 0 && semicolon > pos) {
-                    semicolon = -1;
-                }
-                uriCC.append(normalizedPath, 0, semicolon > 0 ? semicolon : 
pos);
+                uriCC.append(context.getPath());
+                uriCC.append(normalizedUri);
                 service.getMapper().map(context, uriMB, mappingData);
                 if (mappingData.wrapper == null) {
                     return null;
                 }
-                /*
-                 * Append any trailing path params (separated by ';') that were
-                 * ignored for mapping purposes, so that they're reflected in 
the
-                 * RequestDispatcher's requestURI
-                 */
-                if (semicolon > 0) {
-                    uriCC.append(normalizedPath, semicolon, pos - semicolon);
-                }
             } catch (Exception e) {
                 // Should never happen
                 log(sm.getString("applicationContext.mapping.error"), e);
@@ -509,6 +498,34 @@ public class ApplicationContext implemen
     }
 
 
+    // Package private to facilitate testing
+    static String stripPathParams(String input) {
+        // Shortcut
+        if (input.indexOf(';') < 0) {
+            return input;
+        }
+
+        StringBuilder sb = new StringBuilder(input.length());
+        int pos = 0;
+        int limit = input.length();
+        while (pos < limit) {
+            int nextSemiColon = input.indexOf(';', pos);
+            if (nextSemiColon < 0) {
+                nextSemiColon = limit;
+            }
+            sb.append(input.substring(pos, nextSemiColon));
+            int followingSlash = input.indexOf('/', nextSemiColon);
+            if (followingSlash < 0) {
+                pos = limit;
+            } else {
+                pos = followingSlash;
+            }
+        }
+
+        return sb.toString();
+    }
+
+
     @Override
     public URL getResource(String path) throws MalformedURLException {
 

Added: 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java?rev=1839604&view=auto
==============================================================================
--- 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
 (added)
+++ 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
 Wed Aug 29 19:29:46 2018
@@ -0,0 +1,585 @@
+/*
+ * 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.io.PrintWriter;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Locale;
+
+import javax.servlet.AsyncContext;
+import javax.servlet.DispatcherType;
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletMapping;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.MappingMatch;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+@RunWith(value = Parameterized.class)
+public class TestApplicationContextGetRequestDispatcherB extends 
TomcatBaseTest {
+
+    @Parameters(name = "{index}: startMapping[{0}], startUri[{1}], 
dispatcherType[{2}], " +
+            "targetMapping[{3}], targetUri[{4}], useEncodedDispatchPaths[{5}], 
" +
+            "expectedRequestURI[{6}], expectedContextPath[{7}], 
expectedServletPath[{8}], " +
+            "expectedPathInfo[{9}], expectedQueryString[{10}], 
expectedMappingMatch[{11}, " +
+            "expectedMappingPattern[{12}], expectedMappingMatchValue[{13}], " +
+            "expectedMappingServletName[{14}], " +
+            "expectedDispatcherRequestURI[{15}], 
expectedDispatcherContextPath[{16}], " +
+            "expectedDispatcherServletPath[{17}], 
expectedDispatcherPathInfo[{18}], " +
+            "expectedDispatcherQueryString[{19}], 
expectedDispatcherMappingMatch[{20}]," +
+            "expectedDispatcherMappingPattern[{21}], 
expectedDispatcherMappingMatchValue[{22}]," +
+            "expectedDispatcherMappingServletName[{23}]," +
+            "expectedBody")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][]{
+            // Simple dispatch for each type
+            { "/start", "/start", DispatcherType.INCLUDE, "/target", 
"/target", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", "/start", DispatcherType.FORWARD, "/target", 
"/target", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start", DispatcherType.ASYNC, "/target", "/target", 
Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Simple dispatch with query strings
+            { "/start", "/start?abcde=fghij", DispatcherType.INCLUDE, 
"/target", "/target?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target", "/test", "/target", null, "zyxwv=utsrq",
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", "/start?abcde=fghij", DispatcherType.FORWARD, 
"/target", "/target?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, "zyxwv=utsrq",
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start?abcde=fghij", DispatcherType.ASYNC, "/target", 
"/target?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, "zyxwv=utsrq",
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Simple dispatch with trailing path parameters at start
+            { "/start", "/start;abcde=fghij", DispatcherType.INCLUDE, 
"/target", "/target", Boolean.TRUE,
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.FORWARD, 
"/target", "/target", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.ASYNC, "/target", 
"/target", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Simple dispatch with path parameters at start
+            { "/start", ";abcde=fghij/start", DispatcherType.INCLUDE, 
"/target", "/target", Boolean.TRUE,
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", ";abcde=fghij/start", DispatcherType.FORWARD, 
"/target", "/target", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", ";abcde=fghij/start", DispatcherType.ASYNC, "/target", 
"/target", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Simple dispatch with path parameters on dispatch
+            { "/start", "/start", DispatcherType.INCLUDE, "/target", 
"/target;abcde=fghij", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target;abcde=fghij", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", "/start", DispatcherType.FORWARD, "/target", 
"/target;abcde=fghij", Boolean.TRUE,
+                    "/test/target;abcde=fghij", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start", DispatcherType.ASYNC, "/target", 
"/target;abcde=fghij", Boolean.TRUE,
+                    "/test/target;abcde=fghij", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Simple dispatch with multiple path parameters on start and 
dispatch
+            { "/start", "/start;abcde=fghij", DispatcherType.INCLUDE, 
"/target", ";klmno=pqrst/target;uvwxy=z0123", Boolean.TRUE,
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/;klmno=pqrst/target;uvwxy=z0123", "/test", 
"/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.FORWARD, 
"/target", ";klmno=pqrst/target;uvwxy=z0123", Boolean.TRUE,
+                    "/test/;klmno=pqrst/target;uvwxy=z0123", "/test", 
"/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.ASYNC, "/target", 
";klmno=pqrst/target;uvwxy=z0123", Boolean.TRUE,
+                    "/test/;klmno=pqrst/target;uvwxy=z0123", "/test", 
"/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "ASYNC-IAE"},
+            // Simple dispatch with directory traversal
+            { "/start/*", "/start/foo", DispatcherType.INCLUDE, "/target", 
"../target", Boolean.TRUE,
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start/*", "/start/foo", DispatcherType.FORWARD, "/target", 
"../target", Boolean.TRUE,
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "OK"},
+            { "/start/*", "/start/foo", DispatcherType.ASYNC, "/target", 
"../target", Boolean.TRUE,
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "ASYNC-IAE"},
+            // Simple dispatch with directory traversal and path parameters
+            // Note comments in Request.getRequestDispatcher(String) that
+            // explain why the path parameter abcde=fghij is not present on the
+            // dispatched requestURI
+            { "/start/*", "/start;abcde=fghij/foo", DispatcherType.INCLUDE, 
"/target", "../target;klmno=pqrst", Boolean.TRUE,
+                    "/test/start;abcde=fghij/foo", "/test", "/start", "/foo", 
null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "/test/start/../target;klmno=pqrst", "/test", "/target", 
null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "OK"},
+            { "/start/*", "/start;abcde=fghij/foo", DispatcherType.FORWARD, 
"/target", "../target;klmno=pqrst", Boolean.TRUE,
+                    "/test/start/../target;klmno=pqrst", "/test", "/target", 
null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij/foo", "/test", "/start", "/foo", 
null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "OK"},
+            { "/start/*", "/start;abcde=fghij/foo", DispatcherType.ASYNC, 
"/target", "../target;klmno=pqrst", Boolean.TRUE,
+                    "/test/start;abcde=fghij/../target;klmno=pqrst", "/test", 
"/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start;abcde=fghij/foo", "/test", "/start", "/foo", 
null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "ASYNC-IAE"},
+            // Simple dispatch with invalid directory traversal
+            { "/start/*", "/start/foo", DispatcherType.INCLUDE, "/target", 
"../../target", Boolean.TRUE,
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "RD-NULL"},
+            { "/start/*", "/start/foo", DispatcherType.FORWARD, "/target", 
"../../target", Boolean.TRUE,
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "RD-NULL"},
+            { "/start/*", "/start/foo", DispatcherType.ASYNC, "/target", 
"../../target", Boolean.TRUE,
+                    "/test/start/../target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start/foo", "/test", "/start", "/foo", null,
+                    MappingMatch.PATH, "/start/*", "foo", "rd",
+                    "ASYNC-IAE"},
+            // Simple dispatch with invalid target
+            { "/start", "/start", DispatcherType.INCLUDE, "/target", 
"/does-not-exist", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "RD-NULL"},
+            { "/start", "/start", DispatcherType.FORWARD, "/target", 
"/does-not-exist", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "RD-NULL"},
+            { "/start", "/start", DispatcherType.ASYNC, "/target", 
"/does-not-exist", Boolean.TRUE,
+                    "/test/target", "/test", "/target", null, null,
+                    MappingMatch.EXACT, "/target", "target", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "ASYNC-RD-NULL"},
+            // Welcome files
+            { "/start", "/start", DispatcherType.INCLUDE, "*.html", "/", 
Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "OK"},
+            { "/start", "/start", DispatcherType.FORWARD, "*.html", "/", 
Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start", DispatcherType.ASYNC, "*.html", "/", 
Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Welcome files with query strings
+            { "/start", "/start?abcde=fghij", DispatcherType.INCLUDE, 
"*.html", "/?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/", "/test", "/index.html", null, "zyxwv=utsrq",
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "OK"},
+            { "/start", "/start?abcde=fghij", DispatcherType.FORWARD, 
"*.html", "/?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, "zyxwv=utsrq",
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start?abcde=fghij", DispatcherType.ASYNC, "*.html", 
"/?zyxwv=utsrq", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, "zyxwv=utsrq",
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, "abcde=fghij",
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Welcome files with trailing path parameters at start
+            { "/start", "/start;abcde=fghij", DispatcherType.INCLUDE, 
"*.html", "/", Boolean.TRUE,
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.FORWARD, 
"*.html", "/", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start;abcde=fghij", DispatcherType.ASYNC, "*.html", 
"/", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start;abcde=fghij", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Welcome files with path parameters at start
+            { "/start", ";abcde=fghij/start", DispatcherType.INCLUDE, 
"*.html", "/", Boolean.TRUE,
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "OK"},
+            { "/start", ";abcde=fghij/start", DispatcherType.FORWARD, 
"*.html", "/", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", ";abcde=fghij/start", DispatcherType.ASYNC, "*.html", 
"/", Boolean.TRUE,
+                    "/test/", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test;abcde=fghij/start", "/test;abcde=fghij", "/start", 
null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            // Welcome files with trailing path parameters on dispatch
+            { "/start", "/start", DispatcherType.INCLUDE, "*.html", 
"/;abcde=fghij", Boolean.TRUE,
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "/test/;abcde=fghij", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "OK"},
+            { "/start", "/start", DispatcherType.FORWARD, "*.html", 
"/;abcde=fghij", Boolean.TRUE,
+                    "/test/;abcde=fghij", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+            { "/start", "/start", DispatcherType.ASYNC, "*.html", 
"/;abcde=fghij", Boolean.TRUE,
+                    "/test/;abcde=fghij", "/test", "/index.html", null, null,
+                    MappingMatch.EXTENSION, "*.html", "index", "target",
+                    "/test/start", "/test", "/start", null, null,
+                    MappingMatch.EXACT, "/start", "start", "rd",
+                    "OK"},
+        });
+    }
+
+    // Inputs
+    private final String startMapping;
+    private final String startUri;
+    private final DispatcherType dispatcherType;
+    private final String targetMapping;
+    private final String targetUri;
+    private final boolean useEncodedDispatchPaths;
+    // Outputs
+    private final String expectedRequestURI;
+    private final String expectedContextPath;
+    private final String expectedServletPath;
+    private final String expectedPathInfo;
+    private final String expectedQueryString;
+    private final MappingMatch expectedMappingMatch;
+    private final String expectedMappingPattern;
+    private final String expectedMappingMatchValue;
+    private final String expectedMappingServletName;
+    private final String expectedDispatcherRequestURI;
+    private final String expectedDispatcherContextPath;
+    private final String expectedDispatcherServletPath;
+    private final String expectedDispatcherPathInfo;
+    private final String expectedDispatcherQueryString;
+    private final MappingMatch expectedDispatcherMappingMatch;
+    private final String expectedDispatcherMappingPattern;
+    private final String expectedDispatcherMappingMatchValue;
+    private final String expectedDispatcherMappingServletName;
+    private final String expectedBody;
+
+
+    public TestApplicationContextGetRequestDispatcherB(String startMapping, 
String startUri,
+            DispatcherType dispatcherType, String targetMapping, String 
targetUri,
+            boolean useEncodedDispatchPaths,
+            String expectedRequestURI, String expectedContextPath, String 
expectedServletPath,
+            String expectedPathInfo, String expectedQueryString, MappingMatch 
expectedMappingMatch,
+            String expectedMappingPattern, String expectedMappingMatchValue,
+            String expectedMappingServletName,
+            String expectedDispatcherRequestURI, String 
expectedDispatcherContextPath,
+            String expectedDispatcherServletPath, String 
expectedDispatcherPathInfo,
+            String expectedDispatcherQueryString, MappingMatch 
expectedDispatcherMappingMatch,
+            String expectedDispatcherMappingPattern, String 
expectedDispatcherMappingMatchValue,
+            String expectedDispatcherMappingServletName,
+            String expectedBody) {
+        this.startMapping = startMapping;
+        this.startUri = startUri;
+        this.dispatcherType = dispatcherType;
+        this.targetMapping = targetMapping;
+        this.targetUri = targetUri;
+        this.useEncodedDispatchPaths = useEncodedDispatchPaths;
+        this.expectedRequestURI = expectedRequestURI;
+        this.expectedContextPath = expectedContextPath;
+        this.expectedServletPath = expectedServletPath;
+        this.expectedPathInfo = expectedPathInfo;
+        this.expectedQueryString = expectedQueryString;
+        this.expectedMappingMatch = expectedMappingMatch;
+        this.expectedMappingPattern = expectedMappingPattern;
+        this.expectedMappingMatchValue = expectedMappingMatchValue;
+        this.expectedMappingServletName = expectedMappingServletName;
+        this.expectedDispatcherRequestURI = expectedDispatcherRequestURI;
+        this.expectedDispatcherContextPath = expectedDispatcherContextPath;
+        this.expectedDispatcherServletPath = expectedDispatcherServletPath;
+        this.expectedDispatcherPathInfo = expectedDispatcherPathInfo;
+        this.expectedDispatcherQueryString = expectedDispatcherQueryString;
+        this.expectedDispatcherMappingMatch = expectedDispatcherMappingMatch;
+        this.expectedDispatcherMappingPattern = 
expectedDispatcherMappingPattern;
+        this.expectedDispatcherMappingMatchValue = 
expectedDispatcherMappingMatchValue;
+        this.expectedDispatcherMappingServletName = 
expectedDispatcherMappingServletName;
+        this.expectedBody = expectedBody;
+     }
+
+
+    @Test
+    public void doTest() throws Exception {
+
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+
+        // No file system docBase required
+        Context ctx = tomcat.addContext("/test", null);
+        ctx.setDispatchersUseEncodedPaths(useEncodedDispatchPaths);
+        ctx.addWelcomeFile("index.html");
+
+        // Add a target servlet to dispatch to
+        Tomcat.addServlet(ctx, "target", new Target());
+        ctx.addServletMappingDecoded(targetMapping, "target");
+
+        Wrapper w = Tomcat.addServlet(ctx, "rd", new Dispatch());
+        w.setAsyncSupported(true);
+        ctx.addServletMappingDecoded(startMapping, "rd");
+
+        tomcat.start();
+
+        StringBuilder url = new StringBuilder("http://localhost:";);
+        url.append(getPort());
+        url.append("/test");
+        url.append(startUri);
+
+        ByteChunk bc = getUrl(url.toString());
+        String body = bc.toString();
+
+        Assert.assertEquals(expectedBody, body);
+    }
+
+
+    private class Dispatch extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+
+            if (dispatcherType == DispatcherType.INCLUDE) {
+                RequestDispatcher rd = req.getRequestDispatcher(targetUri);
+                if (rd == null) {
+                    writeResponse(resp, "RD-NULL");
+                } else {
+                    rd.include(req, resp);
+                }
+            } else if (dispatcherType == DispatcherType.FORWARD) {
+                RequestDispatcher rd = req.getRequestDispatcher(targetUri);
+                if (rd == null) {
+                    writeResponse(resp, "RD-NULL");
+                } else {
+                    rd.forward(req, resp);
+                }
+            } else if (dispatcherType == DispatcherType.ASYNC) {
+                AsyncContext ac = req.startAsync();
+                try {
+                    ac.dispatch(targetUri);
+                } catch (IllegalArgumentException iae) {
+                    // targetUri is invalid?
+                    if (!targetUri.startsWith("/")) {
+                        // That'll do it.
+                        ac.complete();
+                        writeResponse(resp, "ASYNC-IAE");
+                    } else {
+                        // Not expected. Rethrow.
+                        throw iae;
+                    }
+                } catch (UnsupportedOperationException uoe) {
+                    // While a custom context implementation could cause this,
+                    // if this occurs during this unit test the cause will be 
an
+                    // invalid (unmapped) target path which returned a null
+                    // dispatcher
+                    ac.complete();
+                    writeResponse(resp, "ASYNC-RD-NULL");
+                }
+            } else {
+                // Unexpected dispatch type for this test
+                throw new ServletException("Unknown dispatch type: " + 
dispatcherType);
+            }
+        }
+
+
+        private void writeResponse(HttpServletResponse resp, String message) 
throws IOException {
+            resp.setContentType("text/plain");
+            resp.setCharacterEncoding("UTF-8");
+            PrintWriter pw = resp.getWriter();
+            pw.print(message);
+        }
+    }
+
+
+    private class Target extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+
+            Assert.assertEquals(expectedRequestURI, req.getRequestURI());
+            Assert.assertEquals(expectedContextPath, req.getContextPath());
+            Assert.assertEquals(expectedServletPath, req.getServletPath());
+            Assert.assertEquals(expectedPathInfo, req.getPathInfo());
+            Assert.assertEquals(expectedQueryString, req.getQueryString());
+            HttpServletMapping mapping = req.getHttpServletMapping();
+            Assert.assertEquals(expectedMappingMatch, 
mapping.getMappingMatch());
+            Assert.assertEquals(expectedMappingPattern, mapping.getPattern());
+            Assert.assertEquals(expectedMappingMatchValue, 
mapping.getMatchValue());
+            Assert.assertEquals(expectedMappingServletName, 
mapping.getServletName());
+
+            for (DispatcherType type : DispatcherType.values()) {
+                if (type == dispatcherType) {
+                    String name = 
dispatcherType.name().toLowerCase(Locale.ENGLISH);
+                    Assert.assertEquals(expectedDispatcherRequestURI,
+                            req.getAttribute("javax.servlet." + name + 
".request_uri"));
+                    Assert.assertEquals(expectedDispatcherContextPath,
+                            req.getAttribute("javax.servlet." + name + 
".context_path"));
+                    Assert.assertEquals(expectedDispatcherServletPath,
+                            req.getAttribute("javax.servlet." + name + 
".servlet_path"));
+                    Assert.assertEquals(expectedDispatcherPathInfo,
+                            req.getAttribute("javax.servlet." + name + 
".path_info"));
+                    Assert.assertEquals(expectedDispatcherQueryString,
+                            req.getAttribute("javax.servlet." + name + 
".query_string"));
+                    HttpServletMapping dispatcherMapping =
+                            (HttpServletMapping) req.getAttribute(
+                                    "javax.servlet." + name + ".mapping");
+                    Assert.assertNotNull(dispatcherMapping);
+                    Assert.assertEquals(expectedDispatcherMappingMatch,
+                            dispatcherMapping.getMappingMatch());
+                    Assert.assertEquals(expectedDispatcherMappingPattern,
+                            dispatcherMapping.getPattern());
+                    Assert.assertEquals(expectedDispatcherMappingMatchValue,
+                            dispatcherMapping.getMatchValue());
+                    Assert.assertEquals(expectedDispatcherMappingServletName,
+                            dispatcherMapping.getServletName());
+                } else if (type == DispatcherType.ERROR || type == 
DispatcherType.REQUEST) {
+                    // Skip - not tested
+                } else {
+                    assertAllNull(req, 
type.name().toLowerCase(Locale.ENGLISH));
+                }
+            }
+
+            resp.setContentType("text/plain");
+            resp.setCharacterEncoding("UTF-8");
+            PrintWriter pw = resp.getWriter();
+            pw.print("OK");
+        }
+    }
+
+
+    private void assertAllNull(HttpServletRequest req, String type) {
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".request_uri"));
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".context_path"));
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".servlet_path"));
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".path_info"));
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".query_string"));
+        Assert.assertNull(req.getAttribute("javax.servlet." + type + 
".mapping"));
+    }
+}

Propchange: 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcherB.java
------------------------------------------------------------------------------
    svn:executable = *

Added: 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java?rev=1839604&view=auto
==============================================================================
--- 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java
 (added)
+++ 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java
 Wed Aug 29 19:29:46 2018
@@ -0,0 +1,66 @@
+/*
+ * 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.util.Arrays;
+import java.util.Collection;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+import org.apache.catalina.startup.TomcatBaseTest;
+
+@RunWith(value = Parameterized.class)
+public class TestApplicationContextStripPathParams extends TomcatBaseTest {
+
+    private final String input;
+    private final String expectedOutput;
+
+    public TestApplicationContextStripPathParams(String input, String 
expectedOutput) {
+        this.input = input;
+        this.expectedOutput = expectedOutput;
+    }
+
+    @Parameters(name = "{index}: input[{0}]")
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][]{
+            { "/foo", "/foo"},
+            { "/foo/", "/foo/"},
+            { "/foo/bar", "/foo/bar"},
+            { "/foo;", "/foo"},
+            { "/foo;/", "/foo/"},
+            { "/foo;/bar", "/foo/bar"},
+            { "/foo;a=1", "/foo"},
+            { "/foo;a=1/", "/foo/"},
+            { "/foo;a=1/bar", "/foo/bar"},
+            // Arguably not valid but does the right thing anyway
+            { ";/foo", "/foo"},
+            { ";a=1/foo", "/foo"},
+            { ";/foo/bar", "/foo/bar"},
+            { ";/foo;a=1/bar", "/foo/bar"},
+        });
+    }
+
+    @Test
+    public void testStringPathParams() {
+        String output = ApplicationContext.stripPathParams(input);
+        Assert.assertEquals(expectedOutput, output);
+    }
+}
\ No newline at end of file

Propchange: 
tomcat/trunk/test/org/apache/catalina/core/TestApplicationContextStripPathParams.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1839604&r1=1839603&r2=1839604&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Aug 29 19:29:46 2018
@@ -45,6 +45,14 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 9.0.12 (markt)" rtext="in development">
+  <subsection name="Catalina">
+    <changelog>
+      <fix>
+        Improve the handling of path parameters when working with
+        RequestDispatcher objects. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Coyote">
     <changelog>
       <fix>



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to