Author: markt
Date: Wed Jul 27 16:24:26 2016
New Revision: 1754306
URL: http://svn.apache.org/viewvc?rev=1754306&view=rev
Log:
Decode the path provided to the request dispatcher by default. The reasoning is:
- the servlet spec is clear (see 9.1.1) that path can include a query string;
- the query string is delimited by '?';
- '?' is a valid character in the path (if it is encoded);
- users may want to use '?'in the path;
- therefore users need to be able to provide an encoded path;
- therefore the RequestDispatcher needs to decode the path.
This change should be transparent unless an application is passing an unencoded
% to the request dispatcher which should be fairly rare.
The behaviour is configurable via a Context attribute so the previous behaviour
can easily be restored.
Added:
tomcat/tc6.0.x/trunk/test/org/apache/catalina/core/TestApplicationContext.java
- copied, changed from r1754287,
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/Context.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationContext.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardContext.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
tomcat/tc6.0.x/trunk/webapps/docs/config/context.xml
Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/Context.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/Context.java?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/Context.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/Context.java Wed Jul 27
16:24:26 2016
@@ -1232,4 +1232,26 @@ public interface Context extends Contain
* Context.
*/
public boolean getMapperDirectoryRedirectEnabled();
+
+ /**
+ * Are paths used in calls to obtain a request dispatcher expected to be
+ * encoded? This affects both how Tomcat handles calls to obtain a request
+ * dispatcher as well as how Tomcat generates paths used to obtain request
+ * dispatchers internally.
+ *
+ * @param dispatchersUseEncodedPaths {@code true} to use encoded paths,
+ * otherwise {@code false}
+ */
+ public void setDispatchersUseEncodedPaths(boolean
dispatchersUseEncodedPaths);
+
+ /**
+ * Are paths used in calls to obtain a request dispatcher expected to be
+ * encoded? This applys to both how Tomcat handles calls to obtain a
request
+ * dispatcher as well as how Tomcat generates paths used to obtain request
+ * dispatchers internally.
+ *
+ * @return {@code true} if encoded paths will be used, otherwise
+ * {@code false}
+ */
+ public boolean getDispatchersUseEncodedPaths();
}
Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java Wed
Jul 27 16:24:26 2016
@@ -72,6 +72,7 @@ import org.apache.catalina.util.Enumerat
import org.apache.catalina.util.ParameterMap;
import org.apache.catalina.util.StringManager;
import org.apache.catalina.util.StringParser;
+import org.apache.catalina.util.URLEncoder;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
@@ -1344,14 +1345,22 @@ public class Request
int pos = requestPath.lastIndexOf('/');
String relative = null;
- if (pos >= 0) {
- relative = requestPath.substring(0, pos + 1) + path;
+ if (context.getDispatchersUseEncodedPaths()) {
+ if (pos >= 0) {
+ relative = URLEncoder.DEFAULT.encode(
+ requestPath.substring(0, pos + 1), "UTF-8") + path;
+ } else {
+ relative = URLEncoder.DEFAULT.encode(requestPath, "UTF-8") +
path;
+ }
} else {
- relative = requestPath + path;
+ if (pos >= 0) {
+ relative = requestPath.substring(0, pos + 1) + path;
+ } else {
+ relative = requestPath + path;
+ }
}
return (context.getServletContext().getRequestDispatcher(relative));
-
}
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationContext.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationContext.java?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationContext.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationContext.java
Wed Jul 27 16:24:26 2016
@@ -21,8 +21,10 @@ package org.apache.catalina.core;
import java.io.File;
import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
import java.net.URL;
+import java.net.URLDecoder;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
@@ -50,6 +52,7 @@ import org.apache.catalina.util.RequestU
import org.apache.catalina.util.ResourceSet;
import org.apache.catalina.util.ServerInfo;
import org.apache.catalina.util.StringManager;
+import org.apache.catalina.util.URLEncoder;
import org.apache.naming.resources.DirContextURLStreamHandler;
import org.apache.naming.resources.Resource;
import org.apache.tomcat.util.buf.CharChunk;
@@ -406,17 +409,38 @@ public class ApplicationContext implemen
// Get query string
String queryString = null;
- int pos = path.indexOf('?');
+ String normalizedPath = path;
+ int pos = normalizedPath.indexOf('?');
if (pos >= 0) {
- queryString = path.substring(pos + 1);
- path = path.substring(0, pos);
+ queryString = normalizedPath.substring(pos + 1);
+ normalizedPath = normalizedPath.substring(0, pos);
}
- path = RequestUtil.normalize(path);
- if (path == null)
+ normalizedPath = RequestUtil.normalize(normalizedPath);
+ if (normalizedPath == null)
return (null);
- pos = path.length();
+ if (getContext().getDispatchersUseEncodedPaths()) {
+ // Decode
+ String decodedPath;
+ try {
+ decodedPath = URLDecoder.decode(normalizedPath, "UTF-8");
+ } catch (UnsupportedEncodingException e) {
+ // Impossible
+ return null;
+ }
+
+ // Security check to catch attempts to encode /../ sequences
+ normalizedPath = RequestUtil.normalize(decodedPath);
+ if (!decodedPath.equals(normalizedPath)) {
+ getContext().getLogger().warn(
+ sm.getString("applicationContext.illegalDispatchPath",
path),
+ new IllegalArgumentException());
+ return null;
+ }
+ }
+
+ pos = normalizedPath.length();
// Use the thread local URI and mapping data
DispatchData dd = dispatchData.get();
@@ -439,11 +463,11 @@ public class ApplicationContext implemen
* Ignore any trailing path params (separated by ';') for mapping
* purposes
*/
- int semicolon = path.indexOf(';');
+ int semicolon = normalizedPath.indexOf(';');
if (pos >= 0 && semicolon > pos) {
semicolon = -1;
}
- uriCC.append(path, 0, semicolon > 0 ? semicolon : pos);
+ uriCC.append(normalizedPath, 0, semicolon > 0 ? semicolon : pos);
context.getMapper().map(uriMB, mappingData);
if (mappingData.wrapper == null) {
return (null);
@@ -454,7 +478,7 @@ public class ApplicationContext implemen
* RequestDispatcher's requestURI
*/
if (semicolon > 0) {
- uriCC.append(path, semicolon, pos - semicolon);
+ uriCC.append(normalizedPath, semicolon, pos - semicolon);
}
} catch (Exception e) {
// Should never happen
@@ -468,11 +492,11 @@ public class ApplicationContext implemen
mappingData.recycle();
+ String encodedUri = URLEncoder.DEFAULT.encode(uriCC.toString(),
"UTF-8");
+
// Construct a RequestDispatcher to process this request
- return new ApplicationDispatcher
- (wrapper, uriCC.toString(), wrapperPath, pathInfo,
+ return new ApplicationDispatcher(wrapper, encodedUri, wrapperPath,
pathInfo,
queryString, null);
-
}
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/LocalStrings.properties
Wed Jul 27 16:24:26 2016
@@ -14,6 +14,7 @@
# limitations under the License.
applicationContext.attributeEvent=Exception thrown by attributes event listener
+applicationContext.illegalDispatchPath=An application attempted to obtain a
request dispatcher with an illegal path [{0}] that was rejected because it
contained an encoded directory traversal attempt
applicationContext.mapping.error=Error during mapping
applicationContext.requestDispatcher.iae=Path {0} does not start with a "/"
character
applicationContext.resourcePaths.iae=Path {0} does not start with a "/"
character
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardContext.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardContext.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/StandardContext.java Wed
Jul 27 16:24:26 2016
@@ -794,8 +794,26 @@ public class StandardContext
boolean mapperDirectoryRedirectEnabled = false;
+ private boolean dispatchersUseEncodedPaths = true;
+
+
// ----------------------------------------------------- Context Properties
+ public void setDispatchersUseEncodedPaths(boolean
dispatchersUseEncodedPaths) {
+ this.dispatchersUseEncodedPaths = dispatchersUseEncodedPaths;
+ }
+
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * The default value for this implementation is {@code true}.
+ */
+ public boolean getDispatchersUseEncodedPaths() {
+ return dispatchersUseEncodedPaths;
+ }
+
+
public void setMapperContextRootRedirectEnabled(boolean
mapperContextRootRedirectEnabled) {
this.mapperContextRootRedirectEnabled =
mapperContextRootRedirectEnabled;
}
Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/util/URLEncoder.java Wed Jul
27 16:24:26 2016
@@ -38,6 +38,16 @@ public class URLEncoder {
{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
'A', 'B', 'C', 'D', 'E', 'F'};
+ public static final URLEncoder DEFAULT = new URLEncoder();
+ static {
+ DEFAULT.addSafeCharacter('~');
+ DEFAULT.addSafeCharacter('-');
+ DEFAULT.addSafeCharacter('_');
+ DEFAULT.addSafeCharacter('.');
+ DEFAULT.addSafeCharacter('*');
+ DEFAULT.addSafeCharacter('/');
+ }
+
//Array containing the safe characters set.
protected BitSet safeCharacters = new BitSet(256);
Copied:
tomcat/tc6.0.x/trunk/test/org/apache/catalina/core/TestApplicationContext.java
(from r1754287,
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java)
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/test/org/apache/catalina/core/TestApplicationContext.java?p2=tomcat/tc6.0.x/trunk/test/org/apache/catalina/core/TestApplicationContext.java&p1=tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java&r1=1754287&r2=1754306&rev=1754306&view=diff
==============================================================================
---
tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
(original)
+++
tomcat/tc6.0.x/trunk/test/org/apache/catalina/core/TestApplicationContext.java
Wed Jul 27 16:24:26 2016
@@ -17,10 +17,7 @@
package org.apache.catalina.core;
import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collection;
-import javax.servlet.AsyncContext;
import javax.servlet.RequestDispatcher;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
@@ -29,33 +26,13 @@ import javax.servlet.http.HttpServletRes
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.catalina.util.URLEncoder;
import org.apache.tomcat.util.buf.ByteChunk;
-@RunWith(value = Parameterized.class)
-public class TestApplicationContextGetRequestDispatcher extends TomcatBaseTest
{
-
- private final boolean useAsync;
-
- public TestApplicationContextGetRequestDispatcher(boolean useAsync) {
- this.useAsync = useAsync;
- }
-
- @Parameters(name = "{index}: useAsync[{0}]")
- public static Collection<Object[]> data() {
- return Arrays.asList(new Object[][]{
- {Boolean.TRUE},
- {Boolean.FALSE}
- });
- }
+public class TestApplicationContext extends TomcatBaseTest {
@Test
public void testGetRequestDispatcherNullPath01() throws Exception {
@@ -371,13 +348,7 @@ public class TestApplicationContextGetRe
// Note: This will decode the provided path
ctx.addServletMapping(targetPath, "target");
- if (useAsync) {
- Wrapper w = Tomcat.addServlet(
- ctx, "rd", new AsyncDispatcherServlet(dispatchPath,
useEncodedDispatchPaths));
- w.setAsyncSupported(true);
- } else {
- Tomcat.addServlet(ctx, "rd", new DispatcherServlet(dispatchPath));
- }
+ Tomcat.addServlet(ctx, "rd", new DispatcherServlet(dispatchPath));
// Note: This will decode the provided path
ctx.addServletMapping(startPath, "rd");
@@ -459,46 +430,4 @@ public class TestApplicationContextGetRe
}
}
}
-
-
- private static class AsyncDispatcherServlet extends HttpServlet {
-
- private static final long serialVersionUID = 1L;
- private static final String NULL = "RD-NULL";
-
- private final String dispatchPath;
- private final boolean encodePath;
-
- public AsyncDispatcherServlet(String dispatchPath, boolean encodePath)
{
- this.dispatchPath = dispatchPath;
- this.encodePath = encodePath;
- }
-
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse resp)
- throws ServletException, IOException {
-
- AsyncContext ac = req.startAsync();
- // Quick and dirty. Sufficient for this test but ignores lots of
- // edge cases.
- String target = null;
- if (dispatchPath != null) {
- target = req.getServletPath();
- int lastSlash = target.lastIndexOf('/');
- target = target.substring(0, lastSlash + 1);
- if (encodePath) {
- target = URLEncoder.DEFAULT.encode(target, "UTF-8");
- }
- target += dispatchPath;
- }
- try {
- ac.dispatch(target);
- } catch (UnsupportedOperationException uoe) {
- ac.complete();
- resp.setContentType("text/plain");
- resp.setCharacterEncoding("UTF-8");
- resp.getWriter().print(NULL);
- }
- }
- }
}
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Jul 27 16:24:26 2016
@@ -103,6 +103,12 @@
attempts during the lock out period will no longer reset the lock out
timer to zero. (markt)
</fix>
+ <fix>
+ By default, treat paths used to obtain a request dispatcher as encoded.
+ This behaviour can be changed per web application via the
+ <code>dispatchersUseEncodedPaths</code> attribute of the Context.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/context.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/context.xml?rev=1754306&r1=1754305&r2=1754306&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/context.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/context.xml Wed Jul 27 16:24:26
2016
@@ -251,6 +251,14 @@
used.</p>
</attribute>
+ <attribute name="dispatchersUseEncodedPaths" required="false">
+ <p>Controls whether paths used in calls to obtain a request dispatcher
+ ares expected to be encoded. This affects both how Tomcat handles calls
+ to obtain a request dispatcher as well as how Tomcat generates paths
+ used to obtain request dispatchers internally. If not specified, the
+ default value of <code>true</code> is used.</p>
+ </attribute>
+
<attribute name="docBase" required="true">
<p>The <em>Document Base</em> (also known as the <em>Context
Root</em>) directory for this web application, or the pathname
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
