This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5077-dmi-aware
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1b0c743e85aefd81485185f31441c6f7f6898cba
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Sat Jul 11 20:31:30 2020 +0200
WW-5077 Uses a different pattern if DMI is enabled
---
.../security/DefaultAcceptedPatternsChecker.java | 23 +++-
.../security/DefaultExcludedPatternsChecker.java | 2 +-
.../DefaultAcceptedPatternsCheckerTest.java | 136 +++++++++++++--------
3 files changed, 104 insertions(+), 57 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java
index 38f2b7e..2745955 100644
---
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java
+++
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java
@@ -21,8 +21,10 @@ package com.opensymphony.xwork2.security;
import com.opensymphony.xwork2.XWorkConstants;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.util.TextParseUtil;
+import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
+import org.apache.struts2.StrutsConstants;
import java.util.Arrays;
import java.util.Collections;
@@ -35,7 +37,11 @@ public class DefaultAcceptedPatternsChecker implements
AcceptedPatternsChecker {
private static final Logger LOG =
LogManager.getLogger(DefaultAcceptedPatternsChecker.class);
public static final String[] ACCEPTED_PATTERNS = {
-
"\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
+
"\\w+((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*"
+ };
+
+ public static final String[] DMI_AWARE_ACCEPTED_PATTERNS = {
+
"\\w+([:]?\\w+)*((\\.\\w+)|(\\[\\d+])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*([!]?\\w+)*"
};
private Set<Pattern> acceptedPatterns;
@@ -44,10 +50,21 @@ public class DefaultAcceptedPatternsChecker implements
AcceptedPatternsChecker {
setAcceptedPatterns(ACCEPTED_PATTERNS);
}
+ public DefaultAcceptedPatternsChecker(
+ @Inject(value =
StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, required = false)
String dmiValue
+ ) {
+ if (BooleanUtils.toBoolean(dmiValue)) {
+ LOG.debug("DMI is enabled, adding DMI related accepted patterns");
+ setAcceptedPatterns(DMI_AWARE_ACCEPTED_PATTERNS);
+ } else {
+ setAcceptedPatterns(ACCEPTED_PATTERNS);
+ }
+ }
+
@Inject(value = XWorkConstants.OVERRIDE_ACCEPTED_PATTERNS, required =
false)
protected void setOverrideAcceptedPatterns(String acceptablePatterns) {
LOG.warn("Overriding accepted patterns [{}] with [{}], be aware that
this affects all instances and safety of your application!",
- acceptedPatterns, acceptablePatterns);
+ acceptedPatterns, acceptablePatterns);
acceptedPatterns = new HashSet<>();
try {
for (String pattern :
TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) {
@@ -88,7 +105,7 @@ public class DefaultAcceptedPatternsChecker implements
AcceptedPatternsChecker {
LOG.debug("Sets accepted patterns to [{}], note this impacts the
safety of your application!", patterns);
} else {
LOG.warn("Replacing accepted patterns [{}] with [{}], be aware
that this affects all instances and safety of your application!",
- acceptedPatterns, patterns);
+ acceptedPatterns, patterns);
}
acceptedPatterns = new HashSet<>(patterns.size());
try {
diff --git
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
index 761e794..e1c2aa9 100644
---
a/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
+++
b/core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java
@@ -81,7 +81,7 @@ public class DefaultExcludedPatternsChecker implements
ExcludedPatternsChecker {
@Inject(StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION)
protected void setDynamicMethodInvocation(String dmiValue) {
- if (BooleanUtils.toBoolean(dmiValue) == false) {
+ if (!BooleanUtils.toBoolean(dmiValue)) {
LOG.debug("DMI is disabled, adding DMI related excluded patterns");
setAdditionalExcludePatterns("^(action|method):.*");
}
diff --git
a/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java
b/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java
index 5e5356a..1dc8d8a 100644
---
a/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsCheckerTest.java
@@ -27,41 +27,41 @@ import java.util.regex.Pattern;
public class DefaultAcceptedPatternsCheckerTest extends XWorkTestCase {
- public void testHardcodedAcceptedPatterns() throws Exception {
- // given
- List<String> params = new ArrayList<String>() {
- {
- add("%{#application['test']}");
- add("%{#application.test}");
- add("%{#Application['test']}");
- add("%{#Application.test}");
- add("%{#session['test']}");
- add("%{#session.test}");
- add("%{#Session['test']}");
- add("%{#Session.test}");
- add("%{#struts['test']}");
- add("%{#struts.test}");
- add("%{#Struts['test']}");
- add("%{#Struts.test}");
- add("%{#request['test']}");
- add("%{#request.test}");
- add("%{#Request['test']}");
- add("%{#Request.test}");
- add("%{#servletRequest['test']}");
- add("%{#servletRequest.test}");
- add("%{#ServletRequest['test']}");
- add("%{#ServletRequest.test}");
- add("%{#servletResponse['test']}");
- add("%{#servletResponse.test}");
- add("%{#ServletResponse['test']}");
- add("%{#ServletResponse.test}");
- add("%{#parameters['test']}");
- add("%{#parameters.test}");
- add("%{#Parameters['test']}");
- add("%{#Parameters.test}");
- }
- };
+ private final List<String> params = new ArrayList<String>() {
+ {
+ add("%{#application['test']}");
+ add("%{#application.test}");
+ add("%{#Application['test']}");
+ add("%{#Application.test}");
+ add("%{#session['test']}");
+ add("%{#session.test}");
+ add("%{#Session['test']}");
+ add("%{#Session.test}");
+ add("%{#struts['test']}");
+ add("%{#struts.test}");
+ add("%{#Struts['test']}");
+ add("%{#Struts.test}");
+ add("%{#request['test']}");
+ add("%{#request.test}");
+ add("%{#Request['test']}");
+ add("%{#Request.test}");
+ add("%{#servletRequest['test']}");
+ add("%{#servletRequest.test}");
+ add("%{#ServletRequest['test']}");
+ add("%{#ServletRequest.test}");
+ add("%{#servletResponse['test']}");
+ add("%{#servletResponse.test}");
+ add("%{#ServletResponse['test']}");
+ add("%{#ServletResponse.test}");
+ add("%{#parameters['test']}");
+ add("%{#parameters.test}");
+ add("%{#Parameters['test']}");
+ add("%{#Parameters.test}");
+ }
+ };
+ public void testHardcodedAcceptedPatterns() {
+ // given
AcceptedPatternsChecker checker = new DefaultAcceptedPatternsChecker();
for (String param : params) {
@@ -73,7 +73,20 @@ public class DefaultAcceptedPatternsCheckerTest extends
XWorkTestCase {
}
}
- public void testUnderscoreInParamName() throws Exception {
+ public void testHardcodedAcceptedPatternsWithDmiEnabled() {
+ // given
+ AcceptedPatternsChecker checker = new
DefaultAcceptedPatternsChecker(Boolean.TRUE.toString());
+
+ for (String param : params) {
+ // when
+ AcceptedPatternsChecker.IsAccepted actual =
checker.isAccepted(param);
+
+ // then
+ assertFalse("Access to " + param + " is possible!",
actual.isAccepted());
+ }
+ }
+
+ public void testUnderscoreInParamName() {
// given
AcceptedPatternsChecker checker = new DefaultAcceptedPatternsChecker();
@@ -84,22 +97,33 @@ public class DefaultAcceptedPatternsCheckerTest extends
XWorkTestCase {
assertTrue("Param with underscore wasn't accepted!",
actual.isAccepted());
}
- public void testAcceptedPatternsImmutable() throws Exception {
+ public void testUnderscoreInParamNameWithDmiEnabled() {
+ // given
+ AcceptedPatternsChecker checker = new
DefaultAcceptedPatternsChecker(Boolean.TRUE.toString());
+
+ // when
+ AcceptedPatternsChecker.IsAccepted actual =
checker.isAccepted("mapParam['param_1']");
+
+ // then
+ assertTrue("Param with underscore wasn't accepted!",
actual.isAccepted());
+ }
+
+ public void testAcceptedPatternsImmutable() {
AcceptedPatternsChecker checker = new DefaultAcceptedPatternsChecker();
Set<Pattern> acceptedPatternSet = checker.getAcceptedPatterns();
assertNotNull("default accepted patterns null?", acceptedPatternSet);
assertFalse("default accepted patterns empty?",
acceptedPatternSet.isEmpty());
try {
- acceptedPatternSet.add(Pattern.compile("SomeRegexPattern") );
- fail ("accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ acceptedPatternSet.add(Pattern.compile("SomeRegexPattern"));
+ fail("accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
try {
acceptedPatternSet.clear();
- fail ("accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ fail("accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
@@ -108,15 +132,15 @@ public class DefaultAcceptedPatternsCheckerTest extends
XWorkTestCase {
assertNotNull("replaced default accepted patterns null?",
acceptedPatternSet);
assertFalse("replaced default accepted patterns empty?",
acceptedPatternSet.isEmpty());
try {
- acceptedPatternSet.add(Pattern.compile("SomeRegexPattern") );
- fail ("replaced accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ acceptedPatternSet.add(Pattern.compile("SomeRegexPattern"));
+ fail("replaced accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
try {
acceptedPatternSet.clear();
- fail ("accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ fail("accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
@@ -125,23 +149,29 @@ public class DefaultAcceptedPatternsCheckerTest extends
XWorkTestCase {
acceptedPatternSet = checker.getAcceptedPatterns();
assertNotNull("replaced default accepted patterns null?",
acceptedPatternSet);
assertFalse("replaced default accepted patterns empty?",
acceptedPatternSet.isEmpty());
- assertTrue("replaced default accepted patterns not size " +
testPatternArray.length + "?",
- acceptedPatternSet.size() == testPatternArray.length);
+ assertEquals("replaced default accepted patterns not size " +
testPatternArray.length + "?", acceptedPatternSet.size(),
testPatternArray.length);
for (String testPatternArray1 : testPatternArray) {
assertTrue(testPatternArray1 + " not accepted?",
checker.isAccepted(testPatternArray1).isAccepted());
}
try {
- acceptedPatternSet.add(Pattern.compile("SomeRegexPattern") );
- fail ("replaced accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ acceptedPatternSet.add(Pattern.compile("SomeRegexPattern"));
+ fail("replaced accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
try {
acceptedPatternSet.clear();
- fail ("accepted patterns modifiable?");
- } catch(UnsupportedOperationException uoe) {
+ fail("accepted patterns modifiable?");
+ } catch (UnsupportedOperationException uoe) {
// Expected result
}
+ }
+
+ public void testDmiIsEnabled() {
+ DefaultAcceptedPatternsChecker checker = new
DefaultAcceptedPatternsChecker(Boolean.TRUE.toString());
+
+ AcceptedPatternsChecker.IsAccepted accepted =
checker.isAccepted("action:myAction!save");
+ assertTrue("dmi isn't accepted", accepted.isAccepted());
}
}
\ No newline at end of file
