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

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
      */

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;
     }
 
 

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));
+        }
     }
 }

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();

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);
+    }
 }

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);
+    }
 }

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());
     }