struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject struts git commit: WW-4669 Returns default action/method instead of throwing exception
Date Fri, 29 Jul 2016 08:09:35 GMT
Repository: struts
Updated Branches:
  refs/heads/master d19b9eaa8 -> 5acc71f7c


WW-4669 Returns default action/method instead of throwing exception


Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/5acc71f7
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/5acc71f7
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/5acc71f7

Branch: refs/heads/master
Commit: 5acc71f7c30e16807912bf99b4c32cc4b25f586e
Parents: d19b9ea
Author: Lukasz Lenart <lukaszlenart@apache.org>
Authored: Fri Jul 29 10:09:24 2016 +0200
Committer: Lukasz Lenart <lukaszlenart@apache.org>
Committed: Fri Jul 29 10:09:24 2016 +0200

----------------------------------------------------------------------
 .../org/apache/struts2/StrutsConstants.java     |  7 ++++
 .../dispatcher/mapper/DefaultActionMapper.java  | 44 ++++++++++++++++++--
 .../mapper/DefaultActionMapperTest.java         | 44 ++++++++------------
 .../apache/struts2/rest/RestActionMapper.java   |  5 ++-
 4 files changed, 68 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/main/java/org/apache/struts2/StrutsConstants.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index 186e880..c41d542 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -268,6 +268,13 @@ public final class StrutsConstants {
 
     /** actions names' whitelist **/
     public static final String STRUTS_ALLOWED_ACTION_NAMES = "struts.allowed.action.names";
+    /** default action name to use when action didn't match the whitelist **/
+    public static final String STRUTS_DEFAULT_ACTION_NAME = "struts.default.action.name";
+
+    /** methods names' whitelist **/
+    public static final String STRUTS_ALLOWED_METHOD_NAMES = "struts.allowed.method.names";
+    /** default method name to use when method didn't match the whitelist **/
+    public static final String STRUTS_DEFAULT_METHOD_NAME = "struts.default.method.name";
 
     /** enables action: prefix **/
     public static final String STRUTS_MAPPER_ACTION_PREFIX_ENABLED = "struts.mapper.action.prefix.enabled";

http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
index d0e89be..f1fcfee 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
@@ -121,6 +121,11 @@ public class DefaultActionMapper implements ActionMapper {
     protected boolean alwaysSelectFullNamespace = false;
     protected PrefixTrie prefixTrie = null;
     protected Pattern allowedActionNames = Pattern.compile("[a-zA-Z0-9._!/\\-]*");
+    protected String defaultActionName = "index";
+
+    protected Pattern allowedMethodNames = Pattern.compile("[a-zA-Z_]*[0-9]*");
+    protected String defaultMethodName = "execute";
+
     private boolean allowActionPrefix = false;
     private boolean allowActionCrossNamespaceAccess = false;
 
@@ -137,7 +142,7 @@ public class DefaultActionMapper implements ActionMapper {
                 put(METHOD_PREFIX, new ParameterAction() {
                     public void execute(String key, ActionMapping mapping) {
                         if (allowDynamicMethodCalls) {
-                            mapping.setMethod(cleanupActionName(key.substring(METHOD_PREFIX.length())));
+                            mapping.setMethod(cleanupMethodName(key.substring(METHOD_PREFIX.length())));
                         }
                     }
                 });
@@ -149,7 +154,7 @@ public class DefaultActionMapper implements ActionMapper {
                             if (allowDynamicMethodCalls) {
                                 int bang = name.indexOf('!');
                                 if (bang != -1) {
-                                    String method = cleanupActionName(name.substring(bang
+ 1));
+                                    String method = cleanupMethodName(name.substring(bang
+ 1));
                                     mapping.setMethod(method);
                                     name = name.substring(0, bang);
                                 }
@@ -205,6 +210,21 @@ public class DefaultActionMapper implements ActionMapper {
         this.allowedActionNames = Pattern.compile(allowedActionNames);
     }
 
+    @Inject(value = StrutsConstants.STRUTS_DEFAULT_ACTION_NAME, required = false)
+    public void setDefaultActionName(String defaultActionName) {
+        this.defaultActionName = defaultActionName;
+    }
+
+    @Inject(value = StrutsConstants.STRUTS_ALLOWED_METHOD_NAMES, required = false)
+    public void setAllowedMethodNames(String allowedMethodNames) {
+        this.allowedMethodNames = Pattern.compile(allowedMethodNames);
+    }
+
+    @Inject(value = StrutsConstants.STRUTS_DEFAULT_METHOD_NAME, required = false)
+    public void setDefaultMethodName(String defaultMethodName) {
+        this.defaultMethodName = defaultMethodName;
+    }
+
     @Inject(value = StrutsConstants.STRUTS_MAPPER_ACTION_PREFIX_ENABLED)
     public void setAllowActionPrefix(String allowActionPrefix) {
         this.allowActionPrefix = BooleanUtils.toBoolean(allowActionPrefix);
@@ -377,7 +397,7 @@ public class DefaultActionMapper implements ActionMapper {
     }
 
     /**
-     * Cleans up action name from suspicious characters
+     * Checks action name against allowed pattern if not matched returns default action name
      *
      * @param rawActionName action name extracted from URI
      * @return safe action name
@@ -386,7 +406,23 @@ public class DefaultActionMapper implements ActionMapper {
         if (allowedActionNames.matcher(rawActionName).matches()) {
             return rawActionName;
         } else {
-            throw new StrutsException("Action [" + rawActionName + "] does not match allowed
action names pattern [" + allowedActionNames + "]!");
+            LOG.warn("{} did not match allowed action names {} - default action {} will be
used!", rawActionName, allowedActionNames, defaultActionName);
+            return defaultActionName;
+        }
+    }
+
+    /**
+     * Checks method name (when DMI is enabled) against allowed pattern if not matched returns
default action name
+     *
+     * @param rawMethodName method name extracted from URI
+     * @return safe method name
+     */
+    protected String cleanupMethodName(final String rawMethodName) {
+        if (allowedMethodNames.matcher(rawMethodName).matches()) {
+            return rawMethodName;
+        } else {
+            LOG.warn("{} did not match allowed method names {} - default method {} will be
used!", rawMethodName, allowedMethodNames, defaultMethodName);
+            return defaultMethodName;
         }
     }
 

http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
index b51f569..2008e38 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
@@ -845,37 +845,14 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase
{
         String actionName = "action";
         assertEquals(actionName, mapper.cleanupActionName(actionName));
 
-        Throwable expected = null;
-
         actionName = "${action}";
-        try {
-            mapper.cleanupActionName(actionName);
-            fail();
-        } catch (Throwable t) {
-            expected = t;
-        }
-        assertTrue(expected instanceof StrutsException);
-        assertEquals("Action [${action}] does not match allowed action names pattern [" +
mapper.allowedActionNames.pattern() + "]!", expected.getMessage());
+        assertEquals(mapper.defaultActionName, mapper.cleanupActionName(actionName));
 
         actionName = "${${%{action}}}";
-        try {
-            mapper.cleanupActionName(actionName);
-            fail();
-        } catch (Throwable t) {
-            expected = t;
-        }
-        assertTrue(expected instanceof StrutsException);
-        assertEquals("Action [${${%{action}}}] does not match allowed action names pattern
[" + mapper.allowedActionNames.pattern() + "]!", expected.getMessage());
+        assertEquals(mapper.defaultActionName, mapper.cleanupActionName(actionName));
 
         actionName = "${#foo='action',#foo}";
-        try {
-            mapper.cleanupActionName(actionName);
-            fail();
-        } catch (Throwable t) {
-            expected = t;
-        }
-        assertTrue(expected instanceof StrutsException);
-        assertEquals("Action [${#foo='action',#foo}] does not match allowed action names
pattern [" + mapper.allowedActionNames.pattern() + "]!", expected.getMessage());
+        assertEquals(mapper.defaultActionName, mapper.cleanupActionName(actionName));
 
         actionName = "test-action";
         assertEquals("test-action", mapper.cleanupActionName(actionName));
@@ -887,4 +864,19 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase {
         assertEquals("test!bar.action", mapper.cleanupActionName(actionName));
     }
 
+    public void testAllowedMethodNames() throws Exception {
+        DefaultActionMapper mapper = new DefaultActionMapper();
+
+        assertEquals("", mapper.cleanupMethodName(""));
+        assertEquals("test", mapper.cleanupMethodName("test"));
+        assertEquals("test_method", mapper.cleanupMethodName("test_method"));
+        assertEquals("_test", mapper.cleanupMethodName("_test"));
+        assertEquals("test1", mapper.cleanupMethodName("test1"));
+
+        assertEquals(mapper.defaultMethodName, mapper.cleanupMethodName("2test"));
+        assertEquals(mapper.defaultMethodName, mapper.cleanupMethodName("%{exp}"));
+        assertEquals(mapper.defaultMethodName, mapper.cleanupMethodName("${%{foo}}"));
+        assertEquals(mapper.defaultMethodName, mapper.cleanupMethodName("${#foo='method',#foo}"));
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/struts/blob/5acc71f7/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
----------------------------------------------------------------------
diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
index 678a6f3..a56c8e1 100644
--- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
+++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java
@@ -121,6 +121,7 @@ public class RestActionMapper extends DefaultActionMapper {
     private boolean allowDynamicMethodCalls = false;
     
     public RestActionMapper() {
+        this.defaultMethodName = indexMethodName;
     }
     
     public String getIdParameterName() {
@@ -299,7 +300,7 @@ public class RestActionMapper extends DefaultActionMapper {
                 fullName = fullName.substring(0, lastSlashPos);
             }
 
-            mapping.setName(fullName);
+            mapping.setName(cleanupActionName(fullName));
         }
         return mapping;
     }
@@ -320,7 +321,7 @@ public class RestActionMapper extends DefaultActionMapper {
 
             mapping.setName(actionName);
             if (allowDynamicMethodCalls) {
-                mapping.setMethod(cleanupActionName(actionMethod));
+                mapping.setMethod(cleanupMethodName(actionMethod));
             } else {
                 mapping.setMethod(null);
             }


Mime
View raw message