You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2021/01/06 16:03:23 UTC
[struts] branch WW-5021-static-content-path updated: WW-5021 Adds
validation of the provided static content path
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 <lu...@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();