struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject [struts] branch WW-5021-static-content-path updated: WW-5021 Adds validation of the provided static content path
Date Wed, 06 Jan 2021 16:03:23 GMT
This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch WW-5021-static-content-path
in repository https://gitbox.apache.org/repos/asf/struts.git


The following commit(s) were added to refs/heads/WW-5021-static-content-path by this push:
     new f956910  WW-5021 Adds validation of the provided static content path
f956910 is described below

commit f956910d20bbc162b4ba9bbc8899f88df9f8917a
Author: Lukasz Lenart <lukaszlenart@apache.org>
AuthorDate: Wed Jan 6 17:03:17 2021 +0100

    WW-5021 Adds validation of the provided static content path
---
 .../java/org/apache/struts2/components/UIBean.java |  5 +-
 .../struts2/config/entities/ConstantConfig.java    |  5 +-
 .../dispatcher/DefaultStaticContentLoader.java     | 24 +++++-----
 .../struts2/dispatcher/StaticContentLoader.java    | 38 +++++++++++++++
 .../org/apache/struts2/components/UIBeanTest.java  | 32 +++++++++++++
 .../config/entities/ConstantConfigTest.java        | 32 +++++++++++++
 .../dispatcher/DefaultStaticContentLoaderTest.java | 54 ++++++++++++++++++++--
 7 files changed, 169 insertions(+), 21 deletions(-)

diff --git a/core/src/main/java/org/apache/struts2/components/UIBean.java b/core/src/main/java/org/apache/struts2/components/UIBean.java
index 1586a55..2bafabc 100644
--- a/core/src/main/java/org/apache/struts2/components/UIBean.java
+++ b/core/src/main/java/org/apache/struts2/components/UIBean.java
@@ -30,6 +30,7 @@ import org.apache.struts2.components.template.Template;
 import org.apache.struts2.components.template.TemplateEngine;
 import org.apache.struts2.components.template.TemplateEngineManager;
 import org.apache.struts2.components.template.TemplateRenderingContext;
+import org.apache.struts2.dispatcher.StaticContentLoader;
 import org.apache.struts2.util.TextProviderHelper;
 import org.apache.struts2.views.annotations.StrutsTagAttribute;
 import org.apache.struts2.views.util.ContextUtil;
@@ -526,8 +527,8 @@ public abstract class UIBean extends Component {
     }
 
     @Inject(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH)
-    public void setUiStaticContentPath(String uiStaticContentPath) {
-        this.uiStaticContentPath = uiStaticContentPath;
+    public void setStaticContentPath(String uiStaticContentPath) {
+        this.uiStaticContentPath = StaticContentLoader.Validator.validateStaticContentPath(uiStaticContentPath);
     }
 
     @Inject
diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
index 2ea04fd..93ec170 100644
--- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
+++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java
@@ -29,6 +29,7 @@ import java.util.regex.Pattern;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.dispatcher.StaticContentLoader;
 
 public class ConstantConfig {
     private Boolean devMode;
@@ -274,7 +275,7 @@ public class ConstantConfig {
         map.put(StrutsConstants.STRUTS_LOCALIZED_TEXT_PROVIDER, beanConfToString(localizedTextProvider));
         map.put(StrutsConstants.STRUTS_DISALLOW_PROXY_MEMBER_ACCESS, Objects.toString(disallowProxyMemberAccess,
null));
         map.put(StrutsConstants.STRUTS_OGNL_AUTO_GROWTH_COLLECTION_LIMIT, Objects.toString(ognlAutoGrowthCollectionLimit,
null));
-        map.put(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH, Objects.toString(staticContentPath,
null));
+        map.put(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH, Objects.toString(staticContentPath,
StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH));
 
         return map;
     }
@@ -1352,6 +1353,6 @@ public class ConstantConfig {
     }
 
     public void setStaticContentPath(String staticContentPath) {
-        this.staticContentPath = staticContentPath;
+        this.staticContentPath = StaticContentLoader.Validator.validateStaticContentPath(staticContentPath);
     }
 }
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 c633c1d..0ec5286 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
@@ -48,7 +48,7 @@ import java.util.StringTokenizer;
  *
  * <p>
  * This class is used to serve common static content needed when using various parts of Struts,
such as JavaScript
- * files, CSS files, etc. It works by looking for requests to {@link #staticContentPath}/*
 and then mapping the value
+ * files, CSS files, etc. It works by looking for requests to {@link #uiStaticContentPath}/*
 and then mapping the value
  * after to common packages in Struts and, optionally, in your class path. By default, the
following packages are
  * automatically searched:
  * </p>
@@ -60,7 +60,7 @@ import java.util.StringTokenizer;
  * </ul>
  *
  * <p>
- * This means that you can simply request {@link #staticContentPath}/xhtml/styles.css and
the XHTML UI theme's default stylesheet
+ * This means that you can simply request {@link #uiStaticContentPath}/xhtml/styles.css and
the XHTML UI theme's default stylesheet
  * will be returned. Likewise, many of the AJAX UI components require various JavaScript
files, which are found in the
  * org.apache.struts2.static package. If you wish to add additional packages to be searched,
you can add a comma
  * separated (space, tab and new line will do as well) list in the filter init parameter
named "packages". <b>Be
@@ -86,9 +86,9 @@ public class DefaultStaticContentLoader implements StaticContentLoader {
     protected boolean serveStatic;
 
     /**
-     * Store state of StrutsConstants.STRUTS_STATIC_CONTENT_PATH setting.
+     * Store state of {@link StrutsConstants#STRUTS_UI_STATIC_CONTENT_PATH} setting.
      */
-    protected String staticContentPath;
+    protected String uiStaticContentPath;
 
     /**
      * Store state of StrutsConstants.STRUTS_SERVE_STATIC_BROWSER_CACHE setting.
@@ -118,8 +118,8 @@ public class DefaultStaticContentLoader implements StaticContentLoader
{
     }
 
     @Inject(StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH)
-    public void setStaticContentPath(String staticContentPath) {
-        this.staticContentPath = staticContentPath;
+    public void setStaticContentPath(String uiStaticContentPath) {
+        this.uiStaticContentPath = StaticContentLoader.Validator.validateStaticContentPath(uiStaticContentPath);
     }
 
     /**
@@ -238,7 +238,7 @@ public class DefaultStaticContentLoader implements StaticContentLoader
{
             LOG.warn("Unable to send error response, code: {};", HttpServletResponse.SC_NOT_FOUND,
e1);
         } catch (IllegalStateException ise) {
             // Log illegalstate instead of passing unrecoverable exception to calling thread
-            LOG.warn("Unable to send error response, code: {}; isCommited: {};", HttpServletResponse.SC_NOT_FOUND,
response.isCommitted(), ise);
+            LOG.warn("Unable to send error response, code: {}; isCommitted: {};", HttpServletResponse.SC_NOT_FOUND,
response.isCommitted(), ise);
         }
     }
 
@@ -298,7 +298,7 @@ public class DefaultStaticContentLoader implements StaticContentLoader
{
      * Look for a static resource in the classpath.
      *
      * @param path The resource path
-     * @return The inputstream of the resource
+     * @return The URL of the resource
      * @throws IOException If there is a problem locating the resource
      */
     protected URL findResource(String path) throws IOException {
@@ -368,16 +368,16 @@ public class DefaultStaticContentLoader implements StaticContentLoader
{
     }
 
     public boolean canHandle(String resourcePath) {
-        return serveStatic && resourcePath.startsWith(staticContentPath + "/");
+        return serveStatic && resourcePath.startsWith(uiStaticContentPath + "/");
     }
 
     /**
      * @param path requested path
-     * @return path without leading {@link #staticContentPath}
+     * @return path without leading {@link #uiStaticContentPath}
      */
     protected String cleanupPath(String path) {
-        if (path.startsWith(staticContentPath)) {
-            return path.substring(staticContentPath.length());
+        if (path.startsWith(uiStaticContentPath)) {
+            return path.substring(uiStaticContentPath.length());
         } else {
             return path;
         }
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 608fdb7..da2fd7a 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/StaticContentLoader.java
@@ -18,6 +18,10 @@
  */
 package org.apache.struts2.dispatcher;
 
+import org.apache.commons.lang3.StringUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.config.StrutsBeanSelectionProvider;
 
 import javax.servlet.http.HttpServletRequest;
@@ -36,6 +40,12 @@ import java.io.IOException;
 public interface StaticContentLoader {
 
     /**
+     * Default path at which static content is served, can be changed
+     * by using {@link org.apache.struts2.StrutsConstants#STRUTS_UI_STATIC_CONTENT_PATH}
+     */
+    String DEFAULT_STATIC_CONTENT_PATH = "/static";
+
+    /**
      * @param path Requested resource path
      * @return true if this loader is able to load this type of resource, false otherwise
      */
@@ -57,4 +67,32 @@ public interface StaticContentLoader {
      */
     void findStaticResource(String path, HttpServletRequest request, HttpServletResponse
response) throws IOException;
 
+    class Validator {
+
+        private static final Logger LOG = LogManager.getLogger(DefaultStaticContentLoader.class);
+
+        public static String validateStaticContentPath(String uiStaticContentPath) {
+            if (StringUtils.isBlank(uiStaticContentPath)) {
+                LOG.warn("\"{}\" has been set to \"{}\", falling back into default value
\"{}\"",
+                    StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH,
+                    uiStaticContentPath,
+                    StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH);
+                return StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH;
+            } else if ("/".equals(uiStaticContentPath)) {
+                LOG.warn("\"{}\" cannot be set to \"{}\", falling back into default value
\"{}\"",
+                    StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH,
+                    uiStaticContentPath,
+                    StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH);
+                return StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH;
+            } else if(!uiStaticContentPath.startsWith("/")) {
+                LOG.warn("\"{}\" must start with \"/\", but has been set to \"{}\", prepending
the missing \"/\"!",
+                    StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH,
+                    uiStaticContentPath);
+                return "/" + uiStaticContentPath;
+            } else {
+                LOG.debug("\"{}\" has been set to \"{}\"", StrutsConstants.STRUTS_UI_STATIC_CONTENT_PATH,
uiStaticContentPath);
+                return uiStaticContentPath;
+            }
+        }
+    }
 }
diff --git a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
index 387d6b5..4eac690 100644
--- a/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
+++ b/core/src/test/java/org/apache/struts2/components/UIBeanTest.java
@@ -25,6 +25,8 @@ import org.apache.struts2.StrutsInternalTestCase;
 import org.apache.struts2.components.template.Template;
 import org.apache.struts2.components.template.TemplateEngine;
 import org.apache.struts2.components.template.TemplateEngineManager;
+import org.apache.struts2.dispatcher.DefaultStaticContentLoader;
+import org.apache.struts2.dispatcher.StaticContentLoader;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
 
@@ -343,4 +345,34 @@ public class UIBeanTest extends StrutsInternalTestCase {
 
         assertEquals(nonceVal, dblSelect.getParameters().get("nonce"));
     }
+
+    public void testSetNullUiStaticContentPath() {
+        // given
+        ValueStack stack = ActionContext.getContext().getValueStack();
+        MockHttpServletRequest req = new MockHttpServletRequest();
+        MockHttpServletResponse res = new MockHttpServletResponse();
+
+        TextField field = new TextField(stack, req, res);
+
+        // when
+        field.setStaticContentPath(null);
+        // then
+        assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, field.uiStaticContentPath);
+
+        // when
+        field.setStaticContentPath(" ");
+        // then
+        assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, field.uiStaticContentPath);
+
+        // when
+        field.setStaticContentPath("content");
+        // then
+        assertEquals("/content", field.uiStaticContentPath);
+
+        // when
+        field.setStaticContentPath("/content");
+        // then
+        assertEquals("/content", field.uiStaticContentPath);
+    }
+
 }
diff --git a/core/src/test/java/org/apache/struts2/config/entities/ConstantConfigTest.java
b/core/src/test/java/org/apache/struts2/config/entities/ConstantConfigTest.java
index 01503dd..a9efe59 100644
--- a/core/src/test/java/org/apache/struts2/config/entities/ConstantConfigTest.java
+++ b/core/src/test/java/org/apache/struts2/config/entities/ConstantConfigTest.java
@@ -18,11 +18,17 @@
  */
 package org.apache.struts2.config.entities;
 
+import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.TestBean;
 import com.opensymphony.xwork2.inject.Container;
+import com.opensymphony.xwork2.util.ValueStack;
 import org.apache.struts2.StrutsConstants;
+import org.apache.struts2.components.TextField;
+import org.apache.struts2.dispatcher.StaticContentLoader;
 import org.junit.Assert;
 import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -109,4 +115,30 @@ public class ConstantConfigTest {
             map.get(StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES));
     }
 
+    @Test
+    public void testSettingStaticContentPath() {
+        // given
+        ConstantConfig config = new ConstantConfig();
+
+        // when
+        config.setStaticContentPath(null);
+        // then
+        Assert.assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, config.getStaticContentPath());
+
+        // when
+        config.setStaticContentPath(" ");
+        // then
+        Assert.assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, config.getStaticContentPath());
+
+        // when
+        config.setStaticContentPath("content");
+        // then
+        Assert.assertEquals("/content", config.getStaticContentPath());
+
+        // when
+        config.setStaticContentPath("/content");
+        // then
+        Assert.assertEquals("/content", config.getStaticContentPath());
+    }
+
 }
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 d0e93da..d712088 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
@@ -30,13 +30,12 @@ import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.replay;
 
 public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase {
+
     private HttpServletRequest requestMock;
     private HttpServletResponse responseMock;
-    private HostConfig hostConfigMock;
     private DefaultStaticContentLoader defaultStaticContentLoader;
 
-    public void testParsePackages() throws Exception {
-
+    public void testParsePackages() {
         DefaultStaticContentLoader filterDispatcher = new DefaultStaticContentLoader();
         List<String> result1 = filterDispatcher.parse("foo.bar.package1 foo.bar.package2
foo.bar.package3");
         List<String> result2 = filterDispatcher.parse("foo.bar.package1\tfoo.bar.package2\tfoo.bar.package3");
@@ -102,10 +101,55 @@ public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase
{
         }
     }
 
-    protected void setUp() {
+    public void testSetNullUiStaticContentPath() {
+        // given
+        DefaultStaticContentLoader loader = new DefaultStaticContentLoader();
+
+        // when
+        loader.setStaticContentPath(null);
+
+        // then
+        assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, loader.uiStaticContentPath);
+    }
+
+    public void testSetEmptyUiStaticContentPath() {
+        // given
+        DefaultStaticContentLoader loader = new DefaultStaticContentLoader();
+
+        // when
+        loader.setStaticContentPath(" ");
+
+        // then
+        assertEquals(StaticContentLoader.DEFAULT_STATIC_CONTENT_PATH, loader.uiStaticContentPath);
+    }
+
+    public void testSetUiStaticContentPathWithoutLeadingSlash() {
+        // given
+        DefaultStaticContentLoader loader = new DefaultStaticContentLoader();
+
+        // when
+        loader.setStaticContentPath("content");
+
+        // then
+        assertEquals("/content", loader.uiStaticContentPath);
+    }
+
+    public void testSetUiStaticContentPath() {
+        // given
+        DefaultStaticContentLoader loader = new DefaultStaticContentLoader();
+
+        // when
+        loader.setStaticContentPath("/content");
+
+        // then
+        assertEquals("/content", loader.uiStaticContentPath);
+    }
+
+    protected void setUp() throws Exception {
+        super.setUp();
         requestMock = createMock(HttpServletRequest.class);
         responseMock = createMock(HttpServletResponse.class);
-        hostConfigMock = createMock(HostConfig.class);
+        HostConfig hostConfigMock = createMock(HostConfig.class);
         expect(hostConfigMock.getInitParameter("packages")).andStubReturn(null);
         expect(hostConfigMock.getInitParameter("loggerFactory")).andStubReturn(null);
         defaultStaticContentLoader = new DefaultStaticContentLoader();


Mime
View raw message