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 2014/03/30 21:27:18 UTC
git commit: Improves pattern to avoid classloader pollution and adds
dedicated tests
Repository: struts
Updated Branches:
refs/heads/develop 9a94699da -> aaf5a3010
Improves pattern to avoid classloader pollution and adds dedicated tests
Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/aaf5a301
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/aaf5a301
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/aaf5a301
Branch: refs/heads/develop
Commit: aaf5a3010e3c11ae14e3d3c966a53ebab67146be
Parents: 9a94699
Author: Lukasz Lenart <lu...@apache.org>
Authored: Sun Mar 30 21:27:05 2014 +0200
Committer: Lukasz Lenart <lu...@apache.org>
Committed: Sun Mar 30 21:27:05 2014 +0200
----------------------------------------------------------------------
core/src/main/resources/struts-default.xml | 8 +-
.../interceptor/ParametersInterceptorTest.java | 86 +++++++++++++++++++-
2 files changed, 89 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/struts/blob/aaf5a301/core/src/main/resources/struts-default.xml
----------------------------------------------------------------------
diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml
index 5c446b1..87f1ff5 100644
--- a/core/src/main/resources/struts-default.xml
+++ b/core/src/main/resources/struts-default.xml
@@ -203,7 +203,7 @@
<interceptor-ref name="multiselect"/>
<interceptor-ref name="actionMappingParams"/>
<interceptor-ref name="params">
- <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
+ <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
<interceptor-ref name="conversionError"/>
<interceptor-ref name="deprecation"/>
@@ -260,7 +260,7 @@
<interceptor-ref name="datetime"/>
<interceptor-ref name="multiselect"/>
<interceptor-ref name="params">
- <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
+ <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
<interceptor-ref name="servletConfig"/>
<interceptor-ref name="prepare"/>
@@ -270,7 +270,7 @@
<interceptor-ref name="staticParams"/>
<interceptor-ref name="actionMappingParams"/>
<interceptor-ref name="params">
- <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
+ <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
<interceptor-ref name="conversionError"/>
<interceptor-ref name="validation">
@@ -308,7 +308,7 @@
<interceptor-ref name="staticParams"/>
<interceptor-ref name="actionMappingParams"/>
<interceptor-ref name="params">
- <param name="excludeParams">^class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
+ <param name="excludeParams">(.*\.|^)class\..*,^dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,^parameters\..*,^action:.*,^method:.*</param>
</interceptor-ref>
<interceptor-ref name="conversionError"/>
<interceptor-ref name="validation">
http://git-wip-us.apache.org/repos/asf/struts/blob/aaf5a301/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
index 50eeb4f..5a4485d 100644
--- a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
+++ b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
@@ -87,7 +87,7 @@ public class ParametersInterceptorTest extends XWorkTestCase {
assertEquals(expected, actual);
}
- public void testInsecureParamaters() throws Exception {
+ public void testInsecureParameters() throws Exception {
// given
loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml"));
final Map<String, Object> params = new HashMap<String, Object>() {
@@ -118,6 +118,90 @@ public class ParametersInterceptorTest extends XWorkTestCase {
assertNull(action.getName());
}
+ public void testClassPollutionBlockedByPattern() throws Exception {
+ // given
+ final String pollution1 = "class.classLoader.jarPath";
+ final String pollution2 = "model.class.classLoader.jarPath";
+
+ loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml"));
+ final Map<String, Object> params = new HashMap<String, Object>() {
+ {
+ put(pollution1, "bad");
+ put(pollution2, "very bad");
+ }
+ };
+
+ final Map<String, Boolean> excluded = new HashMap<String, Boolean>();
+ ParametersInterceptor pi = new ParametersInterceptor() {
+
+ @Override
+ protected boolean isExcluded(String paramName) {
+ boolean result = super.isExcluded(paramName);
+ excluded.put(paramName, result);
+ return result;
+ }
+
+ };
+
+ pi.setExcludeParams("(.*\\.|^)class\\..*");
+ container.inject(pi);
+ ValueStack vs = ActionContext.getContext().getValueStack();
+
+ // when
+ ValidateAction action = new ValidateAction();
+ pi.setParameters(action, vs, params);
+
+ // then
+ assertEquals(0, action.getActionMessages().size());
+ assertTrue(excluded.get(pollution1));
+ assertTrue(excluded.get(pollution2));
+ }
+
+ public void testClassPollutionBlockedByOgnl() throws Exception {
+ // given
+ final String pollution1 = "class.classLoader.jarPath";
+ final String pollution2 = "model.class.classLoader.jarPath";
+
+ loadConfigurationProviders(new XWorkConfigurationProvider(), new XmlConfigurationProvider("xwork-param-test.xml"));
+ final Map<String, Object> params = new HashMap<String, Object>() {
+ {
+ put(pollution1, "bad");
+ put(pollution2, "very bad");
+ }
+ };
+
+ final Map<String, Boolean> excluded = new HashMap<String, Boolean>();
+ ParametersInterceptor pi = new ParametersInterceptor() {
+
+ @Override
+ protected boolean isExcluded(String paramName) {
+ boolean result = super.isExcluded(paramName);
+ excluded.put(paramName, result);
+ return result;
+ }
+
+ };
+
+ container.inject(pi);
+ ValueStack vs = ActionContext.getContext().getValueStack();
+
+ // when
+ ValidateAction action = new ValidateAction();
+ pi.setParameters(action, vs, params);
+
+ // then
+ assertEquals(2, action.getActionMessages().size());
+
+ String msg1 = action.getActionMessage(0);
+ String msg2 = action.getActionMessage(1);
+
+ assertEquals("Error setting expression 'class.classLoader.jarPath' with value 'bad'", msg1);
+ assertEquals("Error setting expression 'model.class.classLoader.jarPath' with value 'very bad'", msg2);
+
+ assertFalse(excluded.get(pollution1));
+ assertFalse(excluded.get(pollution2));
+ }
+
public void testDoesNotAllowMethodInvocations() throws Exception {
Map<String, Object> params = new HashMap<String, Object>();
params.put("@java.lang.System@exit(1).dummy", "dumb value");