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 2016/05/04 09:20:51 UTC

[2/7] struts git commit: Introduces more restrictive SMI

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 <lu...@apache.org>
Authored: Wed May 4 09:15:06 2016 +0200
Committer: Lukasz Lenart <lu...@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());