Introduces more restrictive SMI

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);

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;

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)

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();

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,18 +37,50 @@ public class AllowedMethodsTest extends TestCase {
         assertFalse(allowedMethods.isAllowed("someOtherMethod"));
     }
 
-    public void testWidlcardWithStarMethods() throws Exception {
+    public void testWildcardMethodsWithSMI() throws Exception {
+        // given
+        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(false, literals, ActionConfig.DEFAULT_METHOD_REGEX);
+
+        // then
+        assertEquals(1, allowedMethods.list().size());
+        assertTrue(allowedMethods.isAllowed("cancelAction"));
+        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(literals);
+        AllowedMethods allowedMethods = AllowedMethods.build(true, literals, ActionConfig.DEFAULT_METHOD_REGEX);
 
         // then
         assertEquals(1, allowedMethods.list().size());
-        assertTrue(allowedMethods.isAllowed("cancelAction"));
+        assertTrue(allowedMethods.isAllowed("cancel*"));
+        assertFalse(allowedMethods.isAllowed("cancelAction"));
         assertFalse(allowedMethods.isAllowed("startEvent"));
     }
 
@@ -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());