You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2026/09/29 21:30:32 UTC
(sling-org-apache-sling-engine) branch SLING-13371 created (now 02aefc7)
This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a change to branch SLING-13371
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
at 02aefc7 SLING-13371 Fix path resolution divergence for ';' path parameters
This branch includes the following new commits:
new 02aefc7 SLING-13371 Fix path resolution divergence for ';' path parameters
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(sling-org-apache-sling-engine) 01/01: SLING-13371 Fix path resolution divergence for ';' path parameters
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch SLING-13371
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-engine.git
commit 02aefc77ab4c1e70736d848a0ae6fa50da981c51
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Tue Sep 29 23:30:16 2026 +0200
SLING-13371 Fix path resolution divergence for ';' path parameters
---
.../sling/engine/impl/request/RequestData.java | 60 +++++-
.../engine/impl/request/InitResourceTest.java | 232 +++++++++++++++++++--
.../impl/request/StripPathParametersTest.java | 79 +++++++
3 files changed, 342 insertions(+), 29 deletions(-)
diff --git a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
index ab64350..8fae95b 100644
--- a/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
+++ b/src/main/java/org/apache/sling/engine/impl/request/RequestData.java
@@ -21,8 +21,8 @@ package org.apache.sling.engine.impl.request;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
-import java.net.MalformedURLException;
-import java.net.URL;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
@@ -218,13 +218,31 @@ public class RequestData {
StringBuffer requestURL = servletRequest.getRequestURL();
String path = request.getPathInfo();
- if (requestURL.indexOf(";") > -1 && !path.contains(";")) {
+ if (requestURL.indexOf(";") > -1 && path != null && !path.contains(";")) {
+ // The container stripped path parameters (';...') from the path info. Re-derive
+ // the path from the raw request URL to preserve them, but only use the re-derived
+ // path if it is equivalent to the container provided path info modulo the path
+ // parameters. Otherwise every upstream path based decision (container
+ // normalization, authentication requirements, filter patterns, access rules)
+ // would have been made on a different path than the one resolved here.
try {
- final URL rUrl = new URL(requestURL.toString());
+ // java.net.URI#getPath decodes percent escapes, so the derived path is in
+ // the same canonical (decoded) form as the container provided path info
+ final String rawPath = new URI(requestURL.toString()).getPath();
final String prefix = request.getContextPath().concat(request.getServletPath());
- path = rUrl.getPath().substring(prefix.length());
- } catch (final MalformedURLException e) {
- // ignore
+ if (rawPath != null && rawPath.startsWith(prefix)) {
+ final String candidate = rawPath.substring(prefix.length());
+ if (path.equals(stripPathParameters(candidate))) {
+ path = candidate;
+ } else {
+ log.debug(
+ "initResource: ignoring path with parameters {} not equivalent to container provided path {}",
+ candidate,
+ path);
+ }
+ }
+ } catch (final URISyntaxException e) {
+ // ignore and use the container provided path info
}
}
@@ -240,6 +258,34 @@ public class RequestData {
return resource;
}
+ /**
+ * Removes URL path parameters ({@code ;name=value}, scoped to a path segment as per
+ * RFC 3986) from the given path. Used to verify that a path re-derived from the raw
+ * request URL only differs from the container provided path info by the path
+ * parameters it preserves.
+ *
+ * @param path the path to strip path parameters from
+ * @return the path without path parameters
+ */
+ static String stripPathParameters(final String path) {
+ if (path.indexOf(';') < 0) {
+ return path;
+ }
+ final StringBuilder builder = new StringBuilder(path.length());
+ for (int i = 0; i < path.length(); i++) {
+ final char c = path.charAt(i);
+ if (c == ';') {
+ // skip the path parameter(s) up to the end of the current segment
+ while (i + 1 < path.length() && path.charAt(i + 1) != '/') {
+ i++;
+ }
+ } else {
+ builder.append(c);
+ }
+ }
+ return builder.toString();
+ }
+
public void initServlet(final Resource resource, final ServletResolver sr) {
// the resource and the request path info, will never be null
RequestPathInfo requestPathInfo = new SlingRequestPathInfo(resource);
diff --git a/src/test/java/org/apache/sling/engine/impl/request/InitResourceTest.java b/src/test/java/org/apache/sling/engine/impl/request/InitResourceTest.java
index 795fa5b..86f0acb 100644
--- a/src/test/java/org/apache/sling/engine/impl/request/InitResourceTest.java
+++ b/src/test/java/org/apache/sling/engine/impl/request/InitResourceTest.java
@@ -36,6 +36,31 @@ import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+/**
+ * Verifies {@link RequestData#initResource(ResourceResolver)}, in particular the fallback
+ * that re-derives the resolution path from the raw request URL when the container strips
+ * URL path parameters ({@code ;name=value}) from {@link HttpServletRequest#getPathInfo()}.
+ *
+ * <p>Each row below documents, in this order:
+ * <ul>
+ * <li><b>rawRequestURL</b> - the untouched URL as it appears on the wire (what
+ * {@code getRequestURL()} returns) - i.e. the attacker/client-controlled input;</li>
+ * <li><b>containerPathInfo</b> - what the servlet container hands back from
+ * {@code getPathInfo()} after its own decoding/normalization (path parameters already
+ * stripped);</li>
+ * <li><b>expectedResolvedPath</b> - the path {@code initResource()} must hand to the
+ * {@code ResourceResolver}, i.e. what the raw URL is effectively "decoded to" once path
+ * parameters are safely re-attached (or not, if re-attaching them would be unsafe).</li>
+ * </ul>
+ *
+ * <p>Note: {@code servletPath} is the empty string in every row below except the
+ * null-path-info row - not because it was not worth testing, but because
+ * {@code SlingJakartaHttpServletRequestImpl.getServletPath()} always returns {@code ""} (Sling
+ * registers with the HTTP Whiteboard at servlet path {@code "/*"}), so the container's servlet
+ * path is unreachable at this call site regardless of what the underlying
+ * {@code HttpServletRequest} mock reports; only {@code contextPath} contributes to the prefix
+ * that gets stripped in practice.
+ */
@RunWith(Parameterized.class)
public class InitResourceTest {
@@ -45,23 +70,78 @@ public class InitResourceTest {
private HttpServletResponse resp;
private ResourceResolver resourceResolver;
- private final String requestURL;
- private final String pathInfo;
- private final String expectedResolvePath;
- private final String servletPath;
+ private final String description;
+ private final String rawRequestURL;
+ private final String containerPathInfo;
+ private final String expectedResolvedPath;
private final String contextPath;
+ private final String servletPath;
- @Parameters(name = "URL={0} path={1}")
+ @Parameters(name = "{index}: {0}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] {
- {"http://localhost/one;v=1.1", "/one;v=1.1", "/one;v=1.1", "", ""},
- {"http://localhost/two;v=1.1", "/two", "/two;v=1.1", "", ""},
- {"http://localhost/three", "/three", "/three", "", ""},
- {"http://localhost/four%3Bv=1.1", "/four", "/four", "", ""},
- {"http://localhost/five%3Bv=1.1", "/five;v=1.1", "/five;v=1.1", "", ""},
- {"http://localhost/six;v=1.1", "/six;v=1.1", "/six;v=1.1", "", ""},
- {"http://localhost/seven", "/seven;v=1.1", "/seven;v=1.1", "", ""},
{
+ "container preserves a single path parameter as-is: no re-derivation needed",
+ "http://localhost/one;v=1.1",
+ "/one;v=1.1",
+ "/one;v=1.1",
+ "",
+ ""
+ },
+ {
+ "container strips a single path parameter: re-derived and re-attached",
+ "http://localhost/two;v=1.1",
+ "/two",
+ "/two;v=1.1",
+ "",
+ ""
+ },
+ {
+ "no path parameter present at all: path passed through unchanged",
+ "http://localhost/three",
+ "/three",
+ "/three",
+ "",
+ ""
+ },
+ {
+ "raw URL has a percent-encoded semicolon (not a real path parameter): container "
+ + "already decoded and dropped it, no re-derivation is attempted",
+ "http://localhost/four%3Bv=1.1",
+ "/four",
+ "/four",
+ "",
+ ""
+ },
+ {
+ "raw URL has a percent-encoded semicolon that the container decoded and kept: "
+ + "path passed through unchanged (already contains ';')",
+ "http://localhost/five%3Bv=1.1",
+ "/five;v=1.1",
+ "/five;v=1.1",
+ "",
+ ""
+ },
+ {
+ "container already exposes the literal ';' in path info: path passed through unchanged",
+ "http://localhost/six;v=1.1",
+ "/six;v=1.1",
+ "/six;v=1.1",
+ "",
+ ""
+ },
+ {
+ "path parameter present only in container-provided path info (no ';' in raw URL): "
+ + "path passed through unchanged",
+ "http://localhost/seven",
+ "/seven;v=1.1",
+ "/seven;v=1.1",
+ "",
+ ""
+ },
+ {
+ "multiple path parameters across multiple segments, behind a context path: all "
+ + "re-attached at their original segment",
"http://localhost/context/path;v=1.1/more/foo;x=y/end",
"/path/more/foo/end",
"/path;v=1.1/more/foo;x=y/end",
@@ -69,20 +149,123 @@ public class InitResourceTest {
""
},
{
+ "multiple path parameters across multiple segments, no context path: all "
+ + "re-attached at their original segment",
"http://localhost:4502/content;foo=bar/we-retail;bar=baz/us/en.html",
"/content/we-retail/us/en.html",
"/content;foo=bar/we-retail;bar=baz/us/en.html",
"",
""
- }
+ },
+ {
+ "raw URL has a percent-encoded space: URI decoding brings the re-derived path "
+ + "back to the same canonical (decoded) form as the container path info, "
+ + "'%20' -> ' ', so it can be safely compared and re-attached",
+ "http://localhost/a%20b;v=1.1",
+ "/a b",
+ "/a b;v=1.1",
+ "",
+ ""
+ },
+ {
+ "re-derived path (params stripped) does not match container path info at all: "
+ + "the raw URL is rejected outright, container path info wins",
+ "http://localhost/other;x=1",
+ "/two",
+ "/two",
+ "",
+ ""
+ },
+ {
+ "raw URL contains a literal '..;x' traversal segment the container normalized "
+ + "away: rejected because it is not equivalent to the container path info "
+ + "modulo path parameters, so it never resurfaces as a literal segment",
+ "http://localhost/private/..;x/secret",
+ "/secret",
+ "/secret",
+ "",
+ ""
+ },
+ {
+ "raw URL contains a percent-encoded '..;x' traversal segment ('%2e%2e' -> '..'): "
+ + "same rejection as the literal '..;x' case above, decoding does not "
+ + "change the outcome",
+ "http://localhost/private/%2e%2e;x/secret",
+ "/secret",
+ "/secret",
+ "",
+ ""
+ },
+ {
+ "raw URL path does not even start with the context path prefix: fall back to "
+ + "container path info instead of an out-of-bounds substring (previously a 500)",
+ "http://localhost/ctx;x/one",
+ "/one",
+ "/one",
+ "/context",
+ ""
+ },
+ {
+ "raw URL shorter than the context+servlet path prefix: fall back to container "
+ + "path info instead of an out-of-bounds substring (previously a 500)",
+ "http://localhost/a;b",
+ "/x",
+ "/x",
+ "/context",
+ ""
+ },
+ {
+ "raw URL is not a syntactically valid URI (unencoded space): URISyntaxException "
+ + "is swallowed, falls back to container path info",
+ "http://localhost/a b;v=1.1",
+ "/a b",
+ "/a b",
+ "",
+ ""
+ },
+ {
+ "container's own getServletPath()/getPathInfo() both return null (buggy/unusual "
+ + "container): the Sling request wrapper then also exposes pathInfo=null; "
+ + "must not NPE, path stays null (this is the only row using "
+ + "servletPath=null instead of \"\")",
+ "http://localhost/context;x=1",
+ null,
+ null,
+ "/context",
+ null
+ },
+ {
+ "path parameter sits immediately after the context+servlet prefix, leaving an "
+ + "empty base path once the prefix is stripped: still re-attached correctly",
+ "http://localhost/context;x=1",
+ "",
+ ";x=1",
+ "/context",
+ ""
+ },
+ {
+ "raw URL differs from container path info only in case ('/One' vs '/one'), not "
+ + "just by path parameters: rejected as a near-miss, container path info wins",
+ "http://localhost/one;x=1",
+ "/One",
+ "/One",
+ "",
+ ""
+ },
});
}
public InitResourceTest(
- String requestURL, String pathInfo, String expectedResolvePath, String contextPath, String servletPath) {
- this.requestURL = requestURL;
- this.pathInfo = pathInfo;
- this.expectedResolvePath = expectedResolvePath;
+ String description,
+ String rawRequestURL,
+ String containerPathInfo,
+ String expectedResolvedPath,
+ String contextPath,
+ String servletPath) {
+ this.description = description;
+ this.rawRequestURL = rawRequestURL;
+ this.containerPathInfo = containerPathInfo;
+ this.expectedResolvedPath = expectedResolvedPath;
this.contextPath = contextPath;
this.servletPath = servletPath;
}
@@ -103,12 +286,12 @@ public class InitResourceTest {
context.checking(new Expectations() {
{
allowing(req).getRequestURL();
- will(returnValue(new StringBuffer(requestURL)));
+ will(returnValue(new StringBuffer(rawRequestURL)));
allowing(req).getRequestURI();
allowing(req).getPathInfo();
- will(returnValue(pathInfo));
+ will(returnValue(containerPathInfo));
allowing(req).getContextPath();
will(returnValue(contextPath));
@@ -127,9 +310,14 @@ public class InitResourceTest {
allowing(req).getAttribute(RequestProgressTracker.class.getName());
will(returnValue(null));
- // Verify that the ResourceResolver is called with the expected path
- allowing(resourceResolver)
- .resolve(with(any(HttpServletRequest.class)), with(equal(expectedResolvePath)));
+ // Verify that the ResourceResolver is called with the expected (re-derived or
+ // passed-through) path
+ if (expectedResolvedPath == null) {
+ allowing(resourceResolver).resolve(with(any(HttpServletRequest.class)), with(aNull(String.class)));
+ } else {
+ allowing(resourceResolver)
+ .resolve(with(any(HttpServletRequest.class)), with(equal(expectedResolvedPath)));
+ }
allowing(processor).getMaxCallCounter();
will(returnValue(2));
diff --git a/src/test/java/org/apache/sling/engine/impl/request/StripPathParametersTest.java b/src/test/java/org/apache/sling/engine/impl/request/StripPathParametersTest.java
new file mode 100644
index 0000000..747bff9
--- /dev/null
+++ b/src/test/java/org/apache/sling/engine/impl/request/StripPathParametersTest.java
@@ -0,0 +1,79 @@
+/*
+ * 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.sling.engine.impl.request;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * Direct unit tests for {@link RequestData#stripPathParameters(String)}, the helper used by
+ * {@link RequestData#initResource} to check whether a path re-derived from the raw request URL
+ * is equivalent - modulo URL path parameters ({@code ;name=value}) - to the container-provided
+ * path info.
+ */
+public class StripPathParametersTest {
+
+ @Test
+ public void noPathParameterIsUnchanged() {
+ assertEquals("/one/two", RequestData.stripPathParameters("/one/two"));
+ }
+
+ @Test
+ public void singlePathParameterIsRemoved() {
+ assertEquals("/one", RequestData.stripPathParameters("/one;v=1.1"));
+ }
+
+ @Test
+ public void pathParameterInMiddleSegmentIsRemoved() {
+ assertEquals("/one/two", RequestData.stripPathParameters("/one;v=1.1/two"));
+ }
+
+ @Test
+ public void multiplePathParametersInSameSegmentAreAllRemoved() {
+ assertEquals("/one/two", RequestData.stripPathParameters("/one;a=1;b=2/two"));
+ }
+
+ @Test
+ public void trailingPathParameterWithNoFollowingSegmentIsRemoved() {
+ assertEquals("/one", RequestData.stripPathParameters("/one;v=1.1"));
+ // no characters at all after the trailing ';'
+ assertEquals("/one", RequestData.stripPathParameters("/one;"));
+ }
+
+ @Test
+ public void emptyPathParameterValueIsRemoved() {
+ assertEquals("/one/two", RequestData.stripPathParameters("/one;=/two"));
+ }
+
+ @Test
+ public void leadingPathParameterOnRootIsRemoved() {
+ assertEquals("/one", RequestData.stripPathParameters(";x=1/one"));
+ }
+
+ @Test
+ public void pathParameterConsumingWholeInputYieldsEmptyString() {
+ assertEquals("", RequestData.stripPathParameters(";x=1"));
+ }
+
+ @Test
+ public void emptyStringIsUnchanged() {
+ assertEquals("", RequestData.stripPathParameters(""));
+ }
+}