Introduces more restrictive SMI
Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/9ac863b3 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/9ac863b3 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/9ac863b3 Branch: refs/heads/master Commit: 9ac863b339a3513dabd417f4be8a802418a997ba Parents: 0bde271 Author: Lukasz Lenart <lukaszlen...@apache.org> Authored: Wed May 4 09:15:06 2016 +0200 Committer: Lukasz Lenart <lukaszlen...@apache.org> Committed: Wed May 4 09:15:06 2016 +0200 ---------------------------------------------------------------------- .../xwork2/config/entities/ActionConfig.java | 16 +++++-- .../xwork2/config/entities/AllowedMethods.java | 37 +++++++++++----- .../xwork2/config/impl/ActionConfigMatcher.java | 1 + .../providers/XmlConfigurationProvider.java | 1 + .../config/entities/AllowedMethodsTest.java | 46 +++++++++++++++++--- 5 files changed, 80 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/9ac863b3/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java index e12a86b..3e921f4 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/ActionConfig.java @@ -44,6 +44,7 @@ public class ActionConfig extends Located implements Serializable { public static final String DEFAULT_METHOD = "execute"; public static final String WILDCARD = "*"; public static final String REGEX_WILDCARD = "regex:.*"; + public static final String DEFAULT_METHOD_REGEX = "([A-Za-z0-9_$]*)"; protected List<InterceptorMapping> interceptors; // a list of interceptorMapping Objects eg. List<InterceptorMapping> protected Map<String,String> params; @@ -53,6 +54,7 @@ public class ActionConfig extends Located implements Serializable { protected String methodName; protected String packageName; protected String name; + protected boolean strictMethodInvocation; protected AllowedMethods allowedMethods; protected ActionConfig(String packageName, String name, String className) { @@ -63,7 +65,6 @@ public class ActionConfig extends Located implements Serializable { results = new LinkedHashMap<>(); interceptors = new ArrayList<>(); exceptionMappings = new ArrayList<>(); - allowedMethods = AllowedMethods.build(new HashSet<>(Collections.singletonList(DEFAULT_METHOD))); } /** @@ -80,7 +81,7 @@ public class ActionConfig extends Located implements Serializable { this.interceptors = new ArrayList<>(orig.interceptors); this.results = new LinkedHashMap<>(orig.results); this.exceptionMappings = new ArrayList<>(orig.exceptionMappings); - this.allowedMethods = AllowedMethods.build(orig.allowedMethods.list()); + this.allowedMethods = orig.allowedMethods; this.location = orig.location; } @@ -132,6 +133,10 @@ public class ActionConfig extends Located implements Serializable { return method.equals(methodName != null ? methodName : DEFAULT_METHOD) || allowedMethods.isAllowed(method); } + public boolean isStrictMethodInvocation() { + return strictMethodInvocation; + } + @Override public boolean equals(Object o) { if (this == o) { return true; @@ -328,12 +333,17 @@ public class ActionConfig extends Located implements Serializable { return this; } + public Builder setStrictMethodInvocation(boolean strictMethodInvocation) { + target.strictMethodInvocation = strictMethodInvocation; + return this; + } + public ActionConfig build() { target.params = Collections.unmodifiableMap(target.params); target.results = Collections.unmodifiableMap(target.results); target.interceptors = Collections.unmodifiableList(target.interceptors); target.exceptionMappings = Collections.unmodifiableList(target.exceptionMappings); - target.allowedMethods = AllowedMethods.build(allowedMethods); + target.allowedMethods = AllowedMethods.build(target.strictMethodInvocation, allowedMethods, DEFAULT_METHOD_REGEX); ActionConfig result = target; target = new ActionConfig(target); http://git-wip-us.apache.org/repos/asf/struts/blob/9ac863b3/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java index 22fea12..d7741da 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/entities/AllowedMethods.java @@ -28,44 +28,51 @@ public class AllowedMethods { private static final Logger LOG = LogManager.getLogger(AllowedMethods.class); private Set<AllowedMethod> allowedMethods; + private final boolean strictMethodInvocation; + private String defaultRegex; - public static AllowedMethods build(Set<String> methods) { + public static AllowedMethods build(boolean strictMethodInvocation, Set<String> methods, String defaultRegex) { Set<AllowedMethod> allowedMethods = new HashSet<>(); for (String method : methods) { boolean isPattern = false; + StringBuilder methodPattern = new StringBuilder(); int len = method.length(); - StringBuilder ret = new StringBuilder(); char c; for (int x = 0; x < len; x++) { c = method.charAt(x); if (x < len - 2 && c == '{' && '}' == method.charAt(x + 2)) { - ret.append("(.*)"); + methodPattern.append(defaultRegex); isPattern = true; x += 2; } else { - ret.append(c); + methodPattern.append(c); } } - if (isPattern && !method.startsWith("regex:")) { - allowedMethods.add(new PatternAllowedMethod(ret.toString(), method)); + + if (isPattern && !method.startsWith("regex:") && !strictMethodInvocation) { + allowedMethods.add(new PatternAllowedMethod(methodPattern.toString(), method)); } else if (method.startsWith("regex:")) { String pattern = method.substring(method.indexOf(":") + 1); allowedMethods.add(new PatternAllowedMethod(pattern, method)); - } else if (method.contains("*") && !method.startsWith("regex:")) { - String pattern = method.replaceAll("\\*", "(.*)"); + } else if (method.contains("*") && !method.startsWith("regex:") && !strictMethodInvocation) { + String pattern = method.replace("*", defaultRegex); allowedMethods.add(new PatternAllowedMethod(pattern, method)); + } else if (!isPattern) { + allowedMethods.add(new LiteralAllowedMethod(method)); } else { - allowedMethods.add(new LiteralAllowedMethod(ret.toString())); + LOG.trace("Ignoring method name: [{}] when SMI is set to [{}]", method, strictMethodInvocation); } } LOG.debug("Defined allowed methods: {}", allowedMethods); - return new AllowedMethods(allowedMethods); + return new AllowedMethods(strictMethodInvocation, allowedMethods, defaultRegex); } - private AllowedMethods(Set<AllowedMethod> methods) { + private AllowedMethods(boolean strictMethodInvocation, Set<AllowedMethod> methods, String defaultRegex) { + this.strictMethodInvocation = strictMethodInvocation; + this.defaultRegex = defaultRegex; this.allowedMethods = Collections.unmodifiableSet(methods); } @@ -86,6 +93,14 @@ public class AllowedMethods { return result; } + public String getDefaultRegex() { + return defaultRegex; + } + + public boolean isStrictMethodInvocation() { + return strictMethodInvocation; + } + @Override public boolean equals(Object o) { if (this == o) return true; http://git-wip-us.apache.org/repos/asf/struts/blob/9ac863b3/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java index 2a2f0ed..07a8c46 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/impl/ActionConfigMatcher.java @@ -115,6 +115,7 @@ public class ActionConfigMatcher extends AbstractMatcher<ActionConfig> implement .methodName(methodName) .addParams(params) .addResultConfigs(results) + .setStrictMethodInvocation(orig.isStrictMethodInvocation()) .addAllowedMethod(orig.getAllowedMethods()) .addInterceptors(orig.getInterceptors()) .addExceptionMappings(exs) http://git-wip-us.apache.org/repos/asf/struts/blob/9ac863b3/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java index 6075be2..c87cbea 100644 --- a/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/config/providers/XmlConfigurationProvider.java @@ -464,6 +464,7 @@ public class XmlConfigurationProvider implements ConfigurationProvider { .addInterceptors(interceptorList) .addExceptionMappings(exceptionMappings) .addParams(XmlHelper.getParams(actionElement)) + .setStrictMethodInvocation(packageContext.isStrictMethodInvocation()) .addAllowedMethod(allowedMethods) .location(location) .build(); http://git-wip-us.apache.org/repos/asf/struts/blob/9ac863b3/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java b/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java index 607a9dc..adb8935 100644 --- a/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/config/entities/AllowedMethodsTest.java @@ -14,7 +14,7 @@ public class AllowedMethodsTest extends TestCase { literals.add(method); // when - AllowedMethods allowedMethods = AllowedMethods.build(literals); + AllowedMethods allowedMethods = AllowedMethods.build(false, literals, ActionConfig.DEFAULT_METHOD_REGEX); // then assertEquals(1, allowedMethods.list().size()); @@ -22,14 +22,14 @@ public class AllowedMethodsTest extends TestCase { assertFalse(allowedMethods.isAllowed("someOtherMethod")); } - public void testWidlcardMethods() throws Exception { + public void testWildcardMethodsWithNoSMI() throws Exception { // given String method = "my{1}"; Set<String> literals = new HashSet<>(); literals.add(method); // when - AllowedMethods allowedMethods = AllowedMethods.build(literals); + AllowedMethods allowedMethods = AllowedMethods.build(false, literals, ActionConfig.DEFAULT_METHOD_REGEX); // then assertEquals(1, allowedMethods.list().size()); @@ -37,14 +37,30 @@ public class AllowedMethodsTest extends TestCase { assertFalse(allowedMethods.isAllowed("someOtherMethod")); } - public void testWidlcardWithStarMethods() throws Exception { + public void testWildcardMethodsWithSMI() throws Exception { // given - String method = "cancel*"; + Set<String> literals = new HashSet<>(); + literals.add("my{1}"); + literals.add("myMethod"); + + // when + AllowedMethods allowedMethods = AllowedMethods.build(true, literals, ActionConfig.DEFAULT_METHOD_REGEX); + + // then + assertEquals(1, allowedMethods.list().size()); + assertFalse(allowedMethods.isAllowed("my{1}")); + assertTrue(allowedMethods.isAllowed("myMethod")); + assertFalse(allowedMethods.isAllowed("someOtherMethod")); + } + + public void testWildcardWithStarMethodsWithNoSMI() throws Exception { + // given + String method = "cancel*Action*"; Set<String> literals = new HashSet<>(); literals.add(method); // when - AllowedMethods allowedMethods = AllowedMethods.build(literals); + AllowedMethods allowedMethods = AllowedMethods.build(false, literals, ActionConfig.DEFAULT_METHOD_REGEX); // then assertEquals(1, allowedMethods.list().size()); @@ -52,6 +68,22 @@ public class AllowedMethodsTest extends TestCase { assertFalse(allowedMethods.isAllowed("startEvent")); } + public void testWildcardWithStarMethodsWithSMI() throws Exception { + // given + String method = "cancel*"; + Set<String> literals = new HashSet<>(); + literals.add(method); + + // when + AllowedMethods allowedMethods = AllowedMethods.build(true, literals, ActionConfig.DEFAULT_METHOD_REGEX); + + // then + assertEquals(1, allowedMethods.list().size()); + assertTrue(allowedMethods.isAllowed("cancel*")); + assertFalse(allowedMethods.isAllowed("cancelAction")); + assertFalse(allowedMethods.isAllowed("startEvent")); + } + public void testRegexMethods() throws Exception { // given String method = "regex:my([a-zA-Z].*)"; @@ -59,7 +91,7 @@ public class AllowedMethodsTest extends TestCase { literals.add(method); // when - AllowedMethods allowedMethods = AllowedMethods.build(literals); + AllowedMethods allowedMethods = AllowedMethods.build(true, literals, ActionConfig.DEFAULT_METHOD_REGEX); // then assertEquals(1, allowedMethods.list().size());
