svn commit: r1488895 - in /struts/struts2/branches/STRUTS_2_3_14_2_X/core/src: main/java/org/apache/struts2/ main/java/org/apache/struts2/dispatcher/mapper/ test/java/org/apache/struts2/dispatcher/mapper/

[lukaszlenart]

Author: lukaszlenart
Date: Mon Jun  3 08:46:20 2013
New Revision: 1488895

URL: http://svn.apache.org/r1488895
Log:
WW-4090 Itroduces actions names' whitelisting

Modified:
    
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/StrutsConstants.java
    
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
    
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java

Modified: 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/StrutsConstants.java
URL: 
http://svn.apache.org/viewvc/struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/StrutsConstants.java?rev=1488895&r1=1488894&r2=1488895&view=diff
==============================================================================
--- 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/StrutsConstants.java
 (original)
+++ 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/StrutsConstants.java
 Mon Jun  3 08:46:20 2013
@@ -252,4 +252,7 @@ public final class StrutsConstants {
 
     public static final String STRUTS_EXPRESSION_PARSER = 
"struts.expression.parser";
 
+    /** actions names' whitelist **/
+    public static final String STRUTS_ALLOWED_ACTION_NAMES = 
"struts.allowed.action.names";
+
 }

Modified: 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
URL: 
http://svn.apache.org/viewvc/struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java?rev=1488895&r1=1488894&r2=1488895&view=diff
==============================================================================
--- 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
 (original)
+++ 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java
 Mon Jun  3 08:46:20 2013
@@ -35,12 +35,7 @@ import org.apache.struts2.dispatcher.Ser
 import org.apache.struts2.util.PrefixTrie;
 
 import javax.servlet.http.HttpServletRequest;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 /**
  * <!-- START SNIPPET: javadoc -->
@@ -171,6 +166,7 @@ public class DefaultActionMapper impleme
     protected boolean allowSlashesInActionNames = false;
     protected boolean alwaysSelectFullNamespace = false;
     protected PrefixTrie prefixTrie = null;
+    protected String allowedActionNames = "[a-z]*[A-Z]*[0-9]*[.\\-_!/]*";
 
     protected List<String> extensions = new ArrayList<String>() {{
         add("action");
@@ -260,6 +256,11 @@ public class DefaultActionMapper impleme
         this.alwaysSelectFullNamespace = "true".equals(val);
     }
 
+    @Inject(value = StrutsConstants.STRUTS_ALLOWED_ACTION_NAMES, required = 
false)
+    public void setAllowedActionNames(String allowedActionNames) {
+        this.allowedActionNames = allowedActionNames;
+    }
+
     @Inject
     public void setContainer(Container container) {
         this.container = container;
@@ -417,7 +418,25 @@ public class DefaultActionMapper impleme
         }
 
         mapping.setNamespace(namespace);
-        mapping.setName(name);
+        mapping.setName(cleanupActionName(name));
+    }
+
+    /**
+     * Cleans up action name from suspicious characters
+     *
+     * @param rawActionName action name extracted from URI
+     * @return safe action name
+     */
+    protected String cleanupActionName(final String rawActionName) {
+        if (rawActionName.matches(allowedActionNames)) {
+            return rawActionName;
+        } else {
+            String cleanActionName = rawActionName;
+            for(String chunk : rawActionName.split(allowedActionNames)) {
+                cleanActionName = cleanActionName.replace(chunk, "");
+            }
+            return cleanActionName;
+        }
     }
 
     /**

Modified: 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
URL: 
http://svn.apache.org/viewvc/struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java?rev=1488895&r1=1488894&r2=1488895&view=diff
==============================================================================
--- 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
 (original)
+++ 
struts/struts2/branches/STRUTS_2_3_14_2_X/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java
 Mon Jun  3 08:46:20 2013
@@ -747,4 +747,23 @@ public class DefaultActionMapperTest ext
 
     }
 
+    public void testAllowedActionNames() throws Exception {
+        DefaultActionMapper mapper = new DefaultActionMapper();
+
+        String actionName = "action";
+        assertEquals(actionName, mapper.cleanupActionName(actionName));
+
+        actionName = "${action}";
+        assertEquals("action", mapper.cleanupActionName(actionName));
+
+        actionName = "${${%{action}}}";
+        assertEquals("action", mapper.cleanupActionName(actionName));
+
+        actionName = "${#foo='action',#foo}";
+        assertEquals("fooactionfoo", mapper.cleanupActionName(actionName));
+
+        actionName = "test-action";
+        assertEquals("test-action", mapper.cleanupActionName(actionName));
+    }
+
 }