Commits

Anonymous committed 06fa769

WW-2761 Parameters interceptor does not properly exclude parameters with OGNL expressions as the name

git-svn-id: http://svn.opensymphony.com/svn/xwork/trunk@1854e221344d-f017-0410-9bd5-d282ab1896d7

  • Participants
  • Parent commits 7bbf8f3

Comments (0)

Files changed (6)

src/java/com/opensymphony/xwork2/interceptor/ParametersInterceptor.java

 import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.util.ClearableValueStack;
 import com.opensymphony.xwork2.util.LocalizedTextUtil;
+import com.opensymphony.xwork2.util.MemberAccessValueStack;
 import com.opensymphony.xwork2.util.TextParseUtil;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.ValueStackFactory;
             ReflectionContextState.setReportingConversionErrors(context, true);
         }
 
+        boolean memberAccessStack = newStack instanceof MemberAccessValueStack;
+        if (memberAccessStack) {
+            //block or allow access to properties
+            //see WW-2761 for more details
+            MemberAccessValueStack accessValueStack = (MemberAccessValueStack) newStack;
+            accessValueStack.setAcceptedProperties(acceptedParams);
+            accessValueStack.setExcludeProperties(excludeParams);
+        }
+
         for (Map.Entry<String, Object> entry : acceptableParameters.entrySet()) {
             String name = entry.getKey();
             Object value = entry.getValue();
         if (!this.acceptedParams.isEmpty()) {
             for (Pattern pattern : acceptedParams) {
                 Matcher matcher = pattern.matcher(paramName);
-                if (!matcher.matches()) {
-                    return false;
+                if (matcher.matches()) {
+                    return true;
                 }
             }
         }

src/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java

 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
 import com.opensymphony.xwork2.util.ClearableValueStack;
 import com.opensymphony.xwork2.util.CompoundRoot;
+import com.opensymphony.xwork2.util.MemberAccessValueStack;
 import com.opensymphony.xwork2.util.ValueStack;
 import com.opensymphony.xwork2.util.logging.Logger;
 import com.opensymphony.xwork2.util.logging.LoggerFactory;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
 
 /**
  * Ognl implementation of a value stack that allows for dynamic Ognl expressions to be evaluated against it. When
  * @author tm_jee
  * @version $Date$ $Id$
  */
-public class OgnlValueStack implements Serializable, ValueStack, ClearableValueStack {
+public class OgnlValueStack implements Serializable, ValueStack, ClearableValueStack, MemberAccessValueStack {
 
     private static final long serialVersionUID = 370737852934925530L;
 
     Map<Object, Object> overrides;
     transient OgnlUtil ognlUtil;
 
+    transient SecurityMemberAccess securityMemberAccess;
+
     protected OgnlValueStack(XWorkConverter xworkConverter, CompoundRootAccessor accessor, TextProvider prov, boolean allowStaticAccess) {
         setRoot(xworkConverter, accessor, new CompoundRoot(), allowStaticAccess);
         push(prov);
     protected void setRoot(XWorkConverter xworkConverter,
                            CompoundRootAccessor accessor, CompoundRoot compoundRoot, boolean allowStaticMethodAccess) {
         this.root = compoundRoot;
+        this.securityMemberAccess =  new SecurityMemberAccess(allowStaticMethodAccess);
         this.context = Ognl.createDefaultContext(this.root, accessor, new OgnlTypeConverterWrapper(xworkConverter),
-                new StaticMemberAccess(allowStaticMethodAccess));
+               securityMemberAccess);
         context.put(VALUE_STACK, this);
         Ognl.setClassResolver(context, accessor);
         ((OgnlContext) context).setTraceEvaluations(false);
        ((OgnlContext)context).getValues().clear();
     }
 
+    public void setAcceptedProperties(Set<Pattern> acceptedProperties) {
+        securityMemberAccess.setAcceptedProperties(acceptedProperties);
+    }
 
+    public void setExcludeProperties(Set<Pattern> excludeProperties) {
+       securityMemberAccess.setExcludeProperties(excludeProperties);
+    }
 }

src/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java

+/*
+ * Copyright (c) 2002-2006 by OpenSymphony
+ * All rights reserved.
+ */
+package com.opensymphony.xwork2.ognl;
+
+import ognl.DefaultMemberAccess;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Allows access decisions to be made on the basis of whether a member is static or not.
+ * Also blocks or allows access to properties.
+ */
+public class SecurityMemberAccess extends DefaultMemberAccess {
+
+    private boolean allowStaticMethodAccess;
+    Set<Pattern> excludeProperties = Collections.emptySet();
+    Set<Pattern> acceptedProperties = Collections.emptySet();
+
+    public SecurityMemberAccess(boolean method) {
+        super(false);
+        allowStaticMethodAccess = method;
+    }
+
+    public boolean getAllowStaticMethodAccess() {
+        return allowStaticMethodAccess;
+    }
+
+    public void setAllowStaticMethodAccess(boolean allowStaticMethodAccess) {
+        this.allowStaticMethodAccess = allowStaticMethodAccess;
+    }
+
+    @Override
+    public boolean isAccessible(Map context, Object target, Member member,
+                                String propertyName) {
+
+        boolean allow = true;
+        int modifiers = member.getModifiers();
+        if (Modifier.isStatic(modifiers)) {
+            if (member instanceof Method && !getAllowStaticMethodAccess()) {
+                allow = false;
+                if (target instanceof Class) {
+                    Class clazz = (Class) target;
+                    Method method = (Method) member;
+                    if (Enum.class.isAssignableFrom(clazz) && method.getName().equals("values"))
+                        allow = true;
+                }
+            }
+        }
+
+        //failed static test
+        if (!allow)
+            return false;
+
+        // Now check for standard scope rules
+        if (!super.isAccessible(context, target, member, propertyName))
+            return false;
+
+        return isAcceptableProperty(propertyName);
+    }
+
+    protected boolean isAcceptableProperty(String name) {
+        if (isAccepted(name) && !isExcluded(name)) {
+            return true;
+        }
+        return false;
+    }
+
+    protected boolean isAccepted(String paramName) {
+        if (!this.acceptedProperties.isEmpty()) {
+            for (Pattern pattern : acceptedProperties) {
+                Matcher matcher = pattern.matcher(paramName);
+                if (matcher.matches()) {
+                    return true;
+                }
+            }
+
+            //no match, but acceptedParams is not empty
+            return false;
+        }
+
+        //empty acceptedParams
+        return true;
+    }
+
+    protected boolean isExcluded(String paramName) {
+        if (!this.excludeProperties.isEmpty()) {
+            for (Pattern pattern : excludeProperties) {
+                Matcher matcher = pattern.matcher(paramName);
+                if (matcher.matches()) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    public void setExcludeProperties(Set<Pattern> excludeProperties) {
+        this.excludeProperties = excludeProperties;
+    }
+
+    public void setAcceptedProperties(Set<Pattern> acceptedProperties) {
+        this.acceptedProperties = acceptedProperties;
+    }
+
+    public void clearMemerAccessRestrictions() {
+        this.acceptedProperties.clear();
+        this.excludeProperties.clear();
+    }
+}

src/java/com/opensymphony/xwork2/ognl/StaticMemberAccess.java

-/*
- * Copyright (c) 2002-2006 by OpenSymphony
- * All rights reserved.
- */
-package com.opensymphony.xwork2.ognl;
-
-import ognl.DefaultMemberAccess;
-
-import java.lang.reflect.Member;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.util.Map;
-
-/**
- * Allows access decisions to be made on the basis of whether a member is static or not
- */
-public class StaticMemberAccess extends DefaultMemberAccess {
-
-    private boolean allowStaticMethodAccess;
-
-    public StaticMemberAccess(boolean method) {
-        super(false);
-        allowStaticMethodAccess = method;
-    }
-
-    public boolean getAllowStaticMethodAccess() {
-        return allowStaticMethodAccess;
-    }
-
-    public void setAllowStaticMethodAccess(boolean allowStaticMethodAccess) {
-        this.allowStaticMethodAccess = allowStaticMethodAccess;
-    }
-
-    @Override
-    public boolean isAccessible(Map context, Object target, Member member,
-                                String propertyName) {
-
-        boolean allow = true;
-        int modifiers = member.getModifiers();
-        if (Modifier.isStatic(modifiers)) {
-            if (member instanceof Method && !getAllowStaticMethodAccess()) {
-                allow = false;
-                if (target instanceof Class) {
-                    Class clazz = (Class) target;
-                    Method method = (Method) member;
-                    if (Enum.class.isAssignableFrom(clazz) && method.getName().equals("values"))
-                        allow = true;
-                }
-            }
-        }
-
-        // Now check for standard scope rules
-        if (allow) {
-            return super.isAccessible(context, target, member, propertyName);
-        }
-
-        return false;
-    }
-
-
-}

src/java/com/opensymphony/xwork2/util/MemberAccessValueStack.java

+package com.opensymphony.xwork2.util;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+/**
+ * ValueStacks implementing this interface provide a way to remove block or allow access
+ * to properties using regular expressions
+ */
+public interface MemberAccessValueStack {
+    void setExcludeProperties(Set<Pattern> excludeProperties);
+
+    void setAcceptedProperties(Set<Pattern> acceptedProperties);
+}

src/test/com/opensymphony/xwork2/interceptor/ParametersInterceptorTest.java

         assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah());
     }
 
+    public void testExcludedTrickyParameters() throws Exception {
+        Map<String, Object> params = new HashMap<String, Object>() {
+            {
+                put("blah", "This is blah");
+                put("name", "try_1");
+                put("(name)", "try_2");
+                put("['name']", "try_3");
+                put("['na' + 'me']", "try_4");
+                put("{name}[0]", "try_5");
+                put("(new string{'name'})[0]", "try_6");
+                put("#{key: 'name'}.key", "try_7");
+
+            }
+        };
+
+        HashMap<String, Object> extraContext = new HashMap<String, Object>();
+        extraContext.put(ActionContext.PARAMETERS, params);
+
+        ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
+
+        ActionConfig config = configuration.getRuntimeConfiguration().getActionConfig("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME);
+        ParametersInterceptor pi =(ParametersInterceptor) config.getInterceptors().get(0).getInterceptor();
+        pi.setExcludeParams("name");
+
+        proxy.execute();
+
+        SimpleAction action = (SimpleAction) proxy.getAction();
+        assertNull(action.getName());
+        assertEquals("This is blah", (action).getBlah());
+    }
+
+    public void testAcceptedTrickyParameters() throws Exception {
+        Map<String, Object> params = new HashMap<String, Object>() {
+            {
+                put("blah", "This is blah");
+                put("['baz']", "123");
+                put("name", "try_1");
+                put("(name)", "try_2");
+                put("['name']", "try_3");
+                put("['na' + 'me']", "try_4");
+                put("{name}[0]", "try_5");
+                put("(new string{'name'})[0]", "try_6");
+                put("#{key: 'name'}.key", "try_7");
+            }
+        };
+
+        HashMap<String, Object> extraContext = new HashMap<String, Object>();
+        extraContext.put(ActionContext.PARAMETERS, params);
+
+        ActionProxy proxy = actionProxyFactory.createActionProxy("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME, extraContext);
+
+        ActionConfig config = configuration.getRuntimeConfiguration().getActionConfig("", MockConfigurationProvider.PARAM_INTERCEPTOR_ACTION_NAME);
+        ParametersInterceptor pi =(ParametersInterceptor) config.getInterceptors().get(0).getInterceptor();
+        pi.setAcceptedParamNames("blah, baz");
+
+        proxy.execute();
+
+        SimpleAction action = (SimpleAction) proxy.getAction();
+        assertNull(action.getName());
+        assertEquals("This is blah", (action).getBlah());
+         assertEquals(123, action.getBaz());
+    }
+
+
     public void testParametersNotAccessPrivateVariables() throws Exception {
         Map<String, Object> params = new HashMap<String, Object>();
         params.put("protectedMap.foo", "This is blah");