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 <lukaszlen...@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();