This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new b70ecc8e1 WW-5645 Canonicalise static content paths and remove
redundant URL decode (#1777)
b70ecc8e1 is described below
commit b70ecc8e15df9e5cb734be2d87cc3e24a99a78ef
Author: Arun <[email protected]>
AuthorDate: Tue Jul 14 15:07:21 2026 +0530
WW-5645 Canonicalise static content paths and remove redundant URL decode
(#1777)
* Add path segment validation utility to StaticContentLoader
* Remove redundant URL decode in buildPath and reject malformed path
segments early
* Use shared path segment validation in WebJar URL provider
* Document encoding contract on RequestUtils.getServletPath
* Add tests for path segment validation in static content loader
* Add encoded traversal test for WebJar static content serving
* Add encoded traversal tests for WebJar URL provider
* Fix missing closing brace in StaticContentLoader causing compile failure
* Use per-segment matching in containsMalformedPathSegment to avoid false
positives
* Remove redundant dot-segment check now handled by
containsMalformedPathSegment
* Fix indentation on validateStaticContentPath closing brace
* Remove unused encoding field and setter from DefaultStaticContentLoader
* Replace denylist with path canonicalisation in Validator
* Wire canonicalisePath into static content serving
* Wire canonicalisePath into WebJar URL provider
* Update tests for canonicalise approach and remove unused setEncoding call
* Remove setEncoding calls from tests to match updated
DefaultStaticContentLoader
* Remove setEncoding calls from tests to match updated
DefaultStaticContentLoader
* Remove redundant encoded-traversal tests per maintainer review —
end-to-end 404 already covered
---
.../main/java/org/apache/struts2/RequestUtils.java | 5 +++
.../dispatcher/DefaultStaticContentLoader.java | 38 +++++++---------------
.../struts2/dispatcher/StaticContentLoader.java | 31 +++++++++++++++++-
.../struts2/webjars/DefaultWebJarUrlProvider.java | 11 +++----
.../dispatcher/DefaultStaticContentLoaderTest.java | 19 ++++++++++-
.../DefaultStaticContentLoaderWebJarTest.java | 12 ++++++-
.../dispatcher/StaticContentLoaderTest.java | 2 --
7 files changed, 80 insertions(+), 38 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/RequestUtils.java
b/core/src/main/java/org/apache/struts2/RequestUtils.java
index 5840ff06b..651a0a27f 100644
--- a/core/src/main/java/org/apache/struts2/RequestUtils.java
+++ b/core/src/main/java/org/apache/struts2/RequestUtils.java
@@ -53,6 +53,11 @@ public class RequestUtils {
* Retrieves the current request servlet path.
* Deals with differences between servlet specs (2.2 vs 2.3+)
*
+ * <p>Note: the fallback branch extracts from the raw {@code requestURI},
which may
+ * retain percent-encoded characters. Callers must not apply additional
URL decoding
+ * to the returned value because the servlet container has already
performed decoding
+ * where applicable.</p>
+ *
* @param request the request
* @return the servlet path
*/
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
index 3b7a64fd2..f9f83a5fd 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
@@ -32,16 +32,14 @@ import org.apache.struts2.webjars.WebJarUrlProvider;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.io.UnsupportedEncodingException;
+import java.util.Optional;
import java.net.URL;
-import java.net.URLDecoder;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
-import java.util.Optional;
import java.util.StringTokenizer;
/**
@@ -105,11 +103,6 @@ public class DefaultStaticContentLoader implements
StaticContentLoader {
*/
protected final Calendar lastModifiedCal = Calendar.getInstance();
- /**
- * Store state of StrutsConstants.STRUTS_I18N_ENCODING setting.
- */
- protected String encoding;
-
protected boolean devMode;
protected WebJarUrlProvider webJarUrlProvider;
@@ -145,16 +138,6 @@ public class DefaultStaticContentLoader implements
StaticContentLoader {
this.serveStaticBrowserCache =
BooleanUtils.toBoolean(serveStaticBrowserCache);
}
- /**
- * Modify state of StrutsConstants.STRUTS_I18N_ENCODING setting.
- *
- * @param encoding New setting
- */
- @Inject(StrutsConstants.STRUTS_I18N_ENCODING)
- public void setEncoding(String encoding) {
- this.encoding = encoding;
- }
-
@Inject(StrutsConstants.STRUTS_DEVMODE)
public void setDevMode(String devMode) {
this.devMode = Boolean.parseBoolean(devMode);
@@ -222,6 +205,14 @@ public class DefaultStaticContentLoader implements
StaticContentLoader {
throws IOException {
String name = cleanupPath(path);
+ Optional<String> canonical = Validator.canonicalisePath(name);
+ if (canonical.isEmpty()) {
+ LOG.debug("Rejecting static resource request: path escapes
intended scope");
+ sendNotFound(response);
+ return;
+ }
+ name = "/" + canonical.get();
+
if (name.startsWith(WEBJARS_REQUEST_PREFIX)) {
if (!findWebJarResource(name, path, request, response)) {
sendNotFound(response);
@@ -353,17 +344,12 @@ public class DefaultStaticContentLoader implements
StaticContentLoader {
* @param name resource name
* @param packagePrefix The package prefix to use to locate the resource
* @return full path
- * @throws UnsupportedEncodingException If there is a encoding problem
*/
- protected String buildPath(String name, String packagePrefix) throws
UnsupportedEncodingException {
- String resourcePath;
+ protected String buildPath(String name, String packagePrefix) {
if (packagePrefix.endsWith("/") && name.startsWith("/")) {
- resourcePath = packagePrefix + name.substring(1);
- } else {
- resourcePath = packagePrefix + name;
+ return packagePrefix + name.substring(1);
}
-
- return URLDecoder.decode(resourcePath, encoding);
+ return packagePrefix + name;
}
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
b/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
index 149b18c81..31af4f4ad 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
@@ -27,7 +27,9 @@ import org.apache.struts2.config.StrutsBeanSelectionProvider;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
-
+import java.util.ArrayDeque;
+import java.util.Deque;
+import java.util.Optional;
/**
* Interface for loading static resources, based on a path. After implementing
your own static content loader
* you must tell the framework how to use it, eg.
@@ -99,5 +101,32 @@ public interface StaticContentLoader {
return uiStaticContentPath;
}
}
+ /**
+ * Normalises a resource path by resolving {@code .} and {@code ..}
+ * segments and converting backslash separators to forward slashes.
+ *
+ * <p>Returns {@link Optional#empty()} if the resolved path would
+ * escape above the root (i.e. more {@code ..} segments than
+ * preceding path components).</p>
+ *
+ * @param path the raw path to normalise (must not be null)
+ * @return the canonical path without leading slash, or empty if
+ * the path escapes above the root
+ */
+ public static Optional<String> canonicalisePath(String path) {
+ String normalised = path.replace('\\', '/');
+ Deque<String> segments = new ArrayDeque<>();
+ for (String segment : normalised.split("/", -1)) {
+ if ("..".equals(segment)) {
+ if (segments.isEmpty()) {
+ return Optional.empty();
+ }
+ segments.removeLast();
+ } else if (!".".equals(segment) && !segment.isEmpty()) {
+ segments.addLast(segment);
+ }
+ }
+ return Optional.of(String.join("/", segments));
+ }
}
}
diff --git
a/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java
b/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java
index 0b99f85b8..892ccd47f 100644
---
a/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java
+++
b/core/src/main/java/org/apache/struts2/webjars/DefaultWebJarUrlProvider.java
@@ -123,15 +123,12 @@ public class DefaultWebJarUrlProvider implements
WebJarUrlProvider {
return Optional.empty();
}
String normalized = StringUtils.stripStart(logicalPath, "/");
- if (normalized.contains("\\")) {
+ Optional<String> canonical =
StaticContentLoader.Validator.canonicalisePath(normalized);
+ if (canonical.isEmpty()) {
+ LOG.debug("Rejecting WebJar path that escapes above root: {}",
logicalPath);
return Optional.empty();
}
- for (String segment : normalized.split("/")) {
- if (segment.equals("..") || segment.equals(".")) {
- LOG.debug("Rejecting WebJar path with traversal segment: {}",
logicalPath);
- return Optional.empty();
- }
- }
+ normalized = canonical.get();
int slash = normalized.indexOf('/');
if (slash < 1 || slash == normalized.length() - 1) {
return Optional.empty();
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
index db5caba14..d12164538 100644
---
a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
+++
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
@@ -140,7 +140,24 @@ public class DefaultStaticContentLoaderTest extends
StrutsInternalTestCase {
expect(hostConfigMock.getInitParameter("loggerFactory")).andStubReturn(null);
defaultStaticContentLoader = new DefaultStaticContentLoader();
defaultStaticContentLoader.setHostConfig(hostConfigMock);
- defaultStaticContentLoader.setEncoding("UTF-8");
defaultStaticContentLoader.setStaticContentPath("/static");
}
+ public void testBuildPathDoesNotDecodePercentEncoding() {
+ String result = defaultStaticContentLoader.buildPath("/%2e%2e/secret",
"static/");
+ assertEquals("static/%2e%2e/secret", result);
+ }
+
+ public void testFindStaticResourceRejectsLiteralTraversal() throws
Exception {
+ responseMock.sendError(HttpServletResponse.SC_NOT_FOUND);
+ expectLastCall();
+ replay(responseMock);
+
defaultStaticContentLoader.findStaticResource("/static/../../../etc/passwd",
requestMock, responseMock);
+ }
+
+ public void testFindStaticResourceRejectsBackslashPath() throws Exception {
+ responseMock.sendError(HttpServletResponse.SC_NOT_FOUND);
+ expectLastCall();
+ replay(responseMock);
+
defaultStaticContentLoader.findStaticResource("/static/..\\..\\secret",
requestMock, responseMock);
+ }
}
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java
index 388aff42e..009721b08 100644
---
a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java
+++
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderWebJarTest.java
@@ -68,7 +68,6 @@ public class DefaultStaticContentLoaderWebJarTest {
webJarLoader.setServeStaticContent("true");
webJarLoader.setStaticContentPath("/static");
webJarLoader.setServeStaticBrowserCache("true");
- webJarLoader.setEncoding("UTF-8");
org.apache.struts2.webjars.DefaultWebJarUrlProvider provider =
new org.apache.struts2.webjars.DefaultWebJarUrlProvider();
provider.setEnabled(String.valueOf(enabled));
@@ -116,4 +115,15 @@ public class DefaultStaticContentLoaderWebJarTest {
verify(response).sendError(HttpServletResponse.SC_NOT_FOUND);
}
+
+ @Test
+ public void webJarEncodedTraversalReturns404() throws Exception {
+ DefaultStaticContentLoader webJarLoader = newLoader(true);
+ HttpServletRequest request = mock(HttpServletRequest.class);
+ HttpServletResponse response = mock(HttpServletResponse.class);
+
+
webJarLoader.findStaticResource("/static/webjars/jquery/%2e%2e/%2e%2e/etc/passwd",
request, response);
+
+ verify(response).sendError(HttpServletResponse.SC_NOT_FOUND);
+ }
}
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java
index fbf2a044a..a7c41c8a6 100644
---
a/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java
+++
b/core/src/test/java/org/apache/struts2/dispatcher/StaticContentLoaderTest.java
@@ -106,8 +106,6 @@ public class StaticContentLoaderTest extends TestCase {
hostConfigMock.expectAndReturn("getInitParameter",
C.args(C.eq("packages")), null);
hostConfigMock.expectAndReturn("getInitParameter",
C.args(C.eq("loggerFactory")), null);
- contentLoader.setEncoding("utf-8");
-
contentLoader.setHostConfig((HostConfig) hostConfigMock.proxy());
}
