struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject [40/50] git commit: Uses new service to check if param matches accepted patterns
Date Wed, 18 Jun 2014 06:49:57 GMT
Uses new service to check if param matches accepted patterns


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

Branch: refs/heads/develop
Commit: 8a93df10c4f5f3f22f1837c47b4ca9b4facc4f94
Parents: b140faa
Author: Lukasz Lenart <lukaszlenart@apache.org>
Authored: Wed May 21 09:03:51 2014 +0200
Committer: Lukasz Lenart <lukaszlenart@apache.org>
Committed: Wed May 21 09:03:51 2014 +0200

----------------------------------------------------------------------
 .../org/apache/struts2/StrutsConstants.java     |  4 +-
 .../config/DefaultBeanSelectionProvider.java    |  3 ++
 core/src/main/resources/struts-default.xml      |  1 +
 .../providers/XWorkConfigurationProvider.java   |  3 ++
 .../interceptor/ParametersInterceptor.java      | 56 +++++++++-----------
 .../interceptor/ParametersInterceptorTest.java  | 11 +---
 6 files changed, 37 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/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 d173add..8c0c5ce 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -285,10 +285,12 @@ public final class StrutsConstants {
     /** Comma delimited set of excluded classes which cannot be accessed via expressions
**/
     public static final String STRUTS_EXCLUDED_CLASSES = "struts.excludedClasses";
 
-    /** Dedicated service to check if passed string is excluded or not **/
+    /** Dedicated services to check if passed string is excluded/accepted **/
     public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER = "struts.excludedPatterns.checker";
+    public static final String STRUTS_ACCEPTED_PATTERNS_CHECKER = "struts.acceptedPatterns.checker";
 
     /** Constant is used to override framework's default excluded patterns **/
     public static final String STRUTS_OVERRIDE_EXCLUDED_PATTERNS = "struts.override.excludedPatterns";
+    public static final String STRUTS_OVERRIDE_ACCEPTED_PATTERNS = "struts.override.acceptedPatterns";
 
 }

http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java
b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java
index be4fa82..4334d3c 100644
--- a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java
+++ b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java
@@ -22,6 +22,7 @@
 package org.apache.struts2.config;
 
 import com.opensymphony.xwork2.ActionProxyFactory;
+import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
 import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
 import com.opensymphony.xwork2.FileManager;
 import com.opensymphony.xwork2.FileManagerFactory;
@@ -392,6 +393,7 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider
 
         /** Checker is used mostly in interceptors, so there be one instance of checker per
interceptor with Scope.DEFAULT **/
         alias(ExcludedPatternsChecker.class, StrutsConstants.STRUTS_EXCLUDED_PATTERNS_CHECKER,
builder, props, Scope.DEFAULT);
+        alias(AcceptedPatternsChecker.class, StrutsConstants.STRUTS_ACCEPTED_PATTERNS_CHECKER,
builder, props, Scope.DEFAULT);
 
         switchDevMode(props);
 
@@ -403,6 +405,7 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider
         convertIfExist(props, StrutsConstants.STRUTS_CONFIGURATION_XML_RELOAD, XWorkConstants.RELOAD_XML_CONFIGURATION);
         convertIfExist(props, StrutsConstants.STRUTS_EXCLUDED_CLASSES, XWorkConstants.OGNL_EXCLUDED_CLASSES);
         convertIfExist(props, StrutsConstants.STRUTS_OVERRIDE_EXCLUDED_PATTERNS, XWorkConstants.OVERRIDE_EXCLUDED_PATTERNS);
+        convertIfExist(props, StrutsConstants.STRUTS_OVERRIDE_ACCEPTED_PATTERNS, XWorkConstants.OVERRIDE_ACCEPTED_PATTERNS);
 
         LocalizedTextUtil.addDefaultResourceBundle("org/apache/struts2/struts-messages");
         loadCustomResourceBundles(props);

http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/core/src/main/resources/struts-default.xml
----------------------------------------------------------------------
diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml
index 2fc16c9..a1aa63f 100644
--- a/core/src/main/resources/struts-default.xml
+++ b/core/src/main/resources/struts-default.xml
@@ -145,6 +145,7 @@
     <bean type="ognl.PropertyAccessor" name="java.util.HashMap" class="com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor"
/>
 
     <bean type="com.opensymphony.xwork2.security.ExcludedPatternsChecker" name="struts"
class="com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker" />
+    <bean type="com.opensymphony.xwork2.security.AcceptedPatternsChecker" name="struts"
class="com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker" />
 
     <package name="struts-default" abstract="true">
         <result-types>

http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java
b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java
index 9f28334..19e8e76 100644
--- a/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java
+++ b/xwork-core/src/main/java/com/opensymphony/xwork2/config/providers/XWorkConfigurationProvider.java
@@ -2,6 +2,8 @@ package com.opensymphony.xwork2.config.providers;
 
 import com.opensymphony.xwork2.ActionProxyFactory;
 import com.opensymphony.xwork2.DefaultActionProxyFactory;
+import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
+import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker;
 import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
 import com.opensymphony.xwork2.DefaultLocaleProvider;
 import com.opensymphony.xwork2.DefaultTextProvider;
@@ -173,6 +175,7 @@ public class XWorkConfigurationProvider implements ConfigurationProvider
{
                 .factory(StringConverter.class, Scope.SINGLETON)
 
                 .factory(ExcludedPatternsChecker.class, DefaultExcludedPatternsChecker.class,
Scope.DEFAULT)
+                .factory(AcceptedPatternsChecker.class, DefaultAcceptedPatternsChecker.class,
Scope.DEFAULT)
         ;
 
         props.setProperty(XWorkConstants.DEV_MODE, Boolean.FALSE.toString());

http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
b/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
index f1906b0..c1b2f3d 100644
--- a/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
+++ b/xwork-core/src/main/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java
@@ -17,6 +17,7 @@ package com.opensymphony.xwork2.interceptor;
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
+import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
 import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
 import com.opensymphony.xwork2.ValidationAware;
 import com.opensymphony.xwork2.XWorkConstants;
@@ -151,9 +152,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
 
     protected boolean ordered = false;
 
-    protected Set<Pattern> acceptParams = Collections.emptySet();
-
     private ValueStackFactory valueStackFactory;
+    private AcceptedPatternsChecker acceptedPatterns;
 
     @Inject
     public void setValueStackFactory(ValueStackFactory valueStackFactory) {
@@ -170,23 +170,9 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
         this.excludedPatterns = excludedPatterns;
     }
 
-    /**
-	 * Sets a comma-delimited list of regular expressions to match
-	 * parameters that are allowed in the parameter map (aka whitelist).
-	 * <p/>
-	 * Don't change the default unless you know what you are doing in terms
-	 * of security implications.
-	 *
-	 * @param commaDelim A comma-delimited list of regular expressions
-	 */
-	public void setAcceptParamNames(String commaDelim) {
-        Collection<String> acceptPatterns = ArrayUtils.asCollection(commaDelim);
-        if (acceptPatterns != null) {
-            acceptParams = new HashSet<Pattern>();
-            for (String pattern : acceptPatterns) {
-                acceptParams.add(Pattern.compile(pattern));
-            }
-        }
+    @Inject
+    public void setAcceptedPatterns(AcceptedPatternsChecker acceptedPatterns) {
+        this.acceptedPatterns = acceptedPatterns;
     }
 
     /**
@@ -312,7 +298,7 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
             //block or allow access to properties
             //see WW-2761 for more details
             MemberAccessValueStack accessValueStack = (MemberAccessValueStack) newStack;
-            accessValueStack.setAcceptProperties(acceptParams);
+            accessValueStack.setAcceptProperties(acceptedPatterns.getAcceptedPatterns());
             accessValueStack.setExcludeProperties(excludedPatterns.getExcludedPatterns());
         }
 
@@ -419,23 +405,18 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
 	}
 
     protected boolean isAccepted(String paramName) {
-        if (!this.acceptParams.isEmpty()) {
-            for (Pattern pattern : acceptParams) {
-                Matcher matcher = pattern.matcher(paramName);
-                if (matcher.matches()) {
-                    return true;
-                }
-            }
-            notifyDeveloper("Parameter [#0] didn't match acceptParams list of patterns!",
paramName);
-            return false;
+        AcceptedPatternsChecker.IsAccepted result = acceptedPatterns.isAccepted(paramName);
+        if (result.isAccepted()) {
+            return true;
         }
-        return true;
+        notifyDeveloper("Parameter [#0] didn't match accepted pattern [#1]!", paramName,
String.valueOf(result.getAcceptedPattern()));
+        return false;
     }
 
     protected boolean isExcluded(String paramName) {
         ExcludedPatternsChecker.IsExcluded result = excludedPatterns.isExcluded(paramName);
         if (result.isExcluded()) {
-            notifyDeveloper("Parameter [#0] is on the excludeParams list of patterns!", paramName);
+            notifyDeveloper("Parameter [#0] matches excluded pattern [#1]!", paramName, String.valueOf(result.getExcludedPattern()));
             return true;
         }
         return false;
@@ -471,6 +452,19 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
 
     /**
      * Sets a comma-delimited list of regular expressions to match
+     * parameters that are allowed in the parameter map (aka whitelist).
+     * <p/>
+     * Don't change the default unless you know what you are doing in terms
+     * of security implications.
+     *
+     * @param commaDelim A comma-delimited list of regular expressions
+     */
+    public void setAcceptParamNames(String commaDelim) {
+        acceptedPatterns.addAcceptedPatterns(commaDelim);
+    }
+
+    /**
+     * Sets a comma-delimited list of regular expressions to match
      * parameters that should be removed from the parameter map.
      *
      * @param commaDelim A comma-delimited list of regular expressions

http://git-wip-us.apache.org/repos/asf/struts/blob/8a93df10/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
index 156c012..ce86051 100644
--- a/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
+++ b/xwork-core/src/test/java/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java
@@ -373,7 +373,7 @@ public class ParametersInterceptorTest extends XWorkTestCase {
         ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME,
null, extraContext);
         proxy.execute();
         Map<String, String> existingMap = ((SimpleAction) proxy.getAction()).getTheProtectedMap();
-        assertEquals(4, existingMap.size());
+        assertEquals(0, existingMap.size());
     }
 
     public void testParametersWithChineseInTheName() throws Exception {
@@ -479,7 +479,7 @@ public class ParametersInterceptorTest extends XWorkTestCase {
         proxy.execute();
 
         SimpleAction action = (SimpleAction) proxy.getAction();
-        assertNull(action.getName());
+        assertEquals("try_1", action.getName());
         assertEquals("This is blah", (action).getBlah());
         assertEquals(123, action.getBaz());
     }
@@ -700,13 +700,6 @@ public class ParametersInterceptorTest extends XWorkTestCase {
         final Map<String, Object> expected = new HashMap<String, Object>() {
             {
                 put("ordinary.bean", "value");
-                put("#some.internal.object", "true");
-                put("(bla)#some.internal.object", "true");
-                put("#some.internal.object(bla)#some.internal.object", "true");
-                put("#_some.internal.object", "true");
-                put("\u0023_some.internal.object", "true");
-                put("\u0023_some.internal.object,[dfd],bla(\u0023_some.internal.object)",
"true");
-                put("\\u0023_some.internal.object", "true");
             }
         };
 


Mime
View raw message