This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new b19df5d863 Refactor creation of RequestDispatcher instances for
consistency
b19df5d863 is described below
commit b19df5d863b650ccd1381a56d878b8aa02d25d99
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Jan 22 16:43:15 2025 +0000
Refactor creation of RequestDispatcher instances for consistency
Paths should always be processed in this order before mapping
- remove query string
- remove path parameters
- decode
- normalize
---
.../apache/catalina/core/ApplicationContext.java | 39 ++++++++++------------
...TestApplicationContextGetRequestDispatcher.java | 14 ++++----
webapps/docs/changelog.xml | 5 +++
3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/java/org/apache/catalina/core/ApplicationContext.java
b/java/org/apache/catalina/core/ApplicationContext.java
index 522d56417a..5e21d53e5e 100644
--- a/java/org/apache/catalina/core/ApplicationContext.java
+++ b/java/org/apache/catalina/core/ApplicationContext.java
@@ -372,36 +372,31 @@ public class ApplicationContext implements ServletContext
{
queryString = null;
}
+ // From this point, the removal of path parameters, decoding and
normalization is only for mapping purposes.
// Remove path parameters
- String uriNoParams = stripPathParams(uri);
+ String uriToMap = stripPathParams(uri);
+
+ // Decode only if the uri derived from the provided path is expected
to be encoded
+ if (getContext().getDispatchersUseEncodedPaths()) {
+ uriToMap = UDecoder.URLDecode(uriToMap, StandardCharsets.UTF_8);
+ }
// Then normalize
- String normalizedUri = RequestUtil.normalize(uriNoParams);
- if (normalizedUri == null) {
+ uriToMap = RequestUtil.normalize(uriToMap);
+ if (uriToMap == null) {
+
getContext().getLogger().warn(sm.getString("applicationContext.illegalDispatchPath",
path),
+ new IllegalArgumentException());
return null;
}
- // Mapping is against the normalized uri
-
+ /*
+ * uri is passed to the constructor for ApplicationDispatcher and is
ultimately used as the value for
+ * getRequestURI() which returns encoded values. getContextPath()
returns a decoded value. uri may be encoded or
+ * not. Need to prepend the context path to uri and ensure the result
is correctly encoded.
+ */
if (getContext().getDispatchersUseEncodedPaths()) {
- // Decode
- String decodedUri = UDecoder.URLDecode(normalizedUri,
StandardCharsets.UTF_8);
-
- // Security check to catch attempts to encode /../ sequences
- normalizedUri = RequestUtil.normalize(decodedUri);
- if (!decodedUri.equals(normalizedUri)) {
-
getContext().getLogger().warn(sm.getString("applicationContext.illegalDispatchPath",
path),
- 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);
}
@@ -422,7 +417,7 @@ public class ApplicationContext implements ServletContext {
CharChunk uriCC = uriMB.getCharChunk();
try {
uriCC.append(context.getPath());
- uriCC.append(normalizedUri);
+ uriCC.append(uriToMap);
service.getMapper().map(context, uriMB, mappingData);
if (mappingData.wrapper == null) {
return null;
diff --git
a/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
b/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
index ff19ec187c..8845de0a0f 100644
---
a/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
+++
b/test/org/apache/catalina/core/TestApplicationContextGetRequestDispatcher.java
@@ -85,13 +85,6 @@ public class TestApplicationContextGetRequestDispatcher
extends TomcatBaseTest {
}
- @Test
- public void testGetRequestDispatcherEncodedTraversal() throws Exception {
- doTestGetRequestDispatcher(
- true, "/prefix/start", null, "%2E%2E/target", "/target",
DispatcherServlet.NULL);
- }
-
-
@Test
public void testGetRequestDispatcherTraversal01() throws Exception {
doTestGetRequestDispatcher(
@@ -134,6 +127,13 @@ public class TestApplicationContextGetRequestDispatcher
extends TomcatBaseTest {
}
+ @Test
+ public void testGetRequestDispatcherTraversal07() throws Exception {
+ doTestGetRequestDispatcher(
+ true, "/prefix/start", null, "../../target", "/target",
DispatcherServlet.NULL);
+ }
+
+
@Test
public void testGetRequestDispatcher01() throws Exception {
doTestGetRequestDispatcher(
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 171801fcd8..85986319d2 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -165,6 +165,11 @@
<code>DELETE</code> request because the target resource cannot be
deleted. Pull request <pr>802</pr> provided by Chenjp. (markt)
</fix>
+ <scode>
+ Refactor creation of <code>RequestDispatcher</code> instances so that
+ the processing of the provided path is consistent with normal request
+ processing. (markt)
+ </scode>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
