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 2014/05/04 11:18:07 UTC
[1/2] git commit: Removes hardcoded excluded params
Repository: struts
Updated Branches:
refs/heads/feature/exclude-object-class f84efa5f4 -> b3ca9ea5e
Removes hardcoded excluded params
Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/cb590742
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/cb590742
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/cb590742
Branch: refs/heads/feature/exclude-object-class
Commit: cb590742892c39dc6abb84f6a85d87235d555f32
Parents: f84efa5
Author: Lukasz Lenart <lu...@apache.org>
Authored: Sun May 4 10:48:09 2014 +0200
Committer: Lukasz Lenart <lu...@apache.org>
Committed: Sun May 4 10:48:09 2014 +0200
----------------------------------------------------------------------
.../interceptor/ParametersInterceptor.java | 16 +++-----------
.../interceptor/ParametersInterceptorTest.java | 16 ++++++++++----
.../src/test/resources/xwork-test-beans.xml | 22 ++------------------
3 files changed, 17 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/struts/blob/cb590742/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 cb38d57..e09ab54 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
@@ -141,21 +141,16 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
private static final Logger LOG = LoggerFactory.getLogger(ParametersInterceptor.class);
- public static final String ACCEPTED_PARAM_NAMES = "\\w+((\\.\\w+)|(\\[\\d+\\])|(\\(\\d+\\))|(\\['(\\w|[\\u4e00-\\u9fa5])+'\\])|(\\('(\\w|[\\u4e00-\\u9fa5])+'\\)))*";
-
protected static final int PARAM_NAME_MAX_LENGTH = 100;
private int paramNameMaxLength = PARAM_NAME_MAX_LENGTH;
+ private boolean devMode = false;
protected boolean ordered = false;
+
protected Set<Pattern> excludeParams = Collections.emptySet();
protected Set<Pattern> acceptParams = Collections.emptySet();
- private boolean devMode = false;
-
- // Allowed names of parameters
- private Pattern acceptedPattern = Pattern.compile(ACCEPTED_PARAM_NAMES);
-
private ValueStackFactory valueStackFactory;
@Inject
@@ -426,13 +421,8 @@ public class ParametersInterceptor extends MethodFilterInterceptor {
}
notifyDeveloper("Parameter [#0] didn't match acceptParams list of patterns!", paramName);
return false;
- } else {
- boolean matches = acceptedPattern.matcher(paramName).matches();
- if (!matches) {
- notifyDeveloper("Parameter [#0] didn't match acceptedPattern pattern!", paramName);
- }
- return matches;
}
+ return true;
}
protected boolean isExcluded(String paramName) {
http://git-wip-us.apache.org/repos/asf/struts/blob/cb590742/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 f0adf02..79f46e6 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
@@ -32,11 +32,13 @@ import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import com.opensymphony.xwork2.ognl.OgnlValueStack;
import com.opensymphony.xwork2.ognl.OgnlValueStackFactory;
+import com.opensymphony.xwork2.ognl.SecurityMemberAccess;
import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
import com.opensymphony.xwork2.util.CompoundRoot;
import com.opensymphony.xwork2.util.ValueStack;
import com.opensymphony.xwork2.util.ValueStackFactory;
import junit.framework.Assert;
+import ognl.OgnlContext;
import ognl.PropertyAccessor;
import java.io.File;
@@ -293,9 +295,8 @@ public class ParametersInterceptorTest extends XWorkTestCase {
//then
assertEquals("This is blah", ((SimpleAction) proxy.getAction()).getBlah());
- Object allowMethodAccess = stack.findValue("\u0023_memberAccess['allowStaticMethodAccess']");
- assertNotNull(allowMethodAccess);
- assertEquals(Boolean.FALSE, allowMethodAccess);
+ boolean allowMethodAccess = ((SecurityMemberAccess) ((OgnlContext) stack.getContext()).getMemberAccess()).getAllowStaticMethodAccess();
+ assertFalse(allowMethodAccess);
}
public void testParameters() throws Exception {
@@ -323,7 +324,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(0, existingMap.size());
+ assertEquals(4, existingMap.size());
}
public void testParametersWithChineseInTheName() throws Exception {
@@ -650,6 +651,13 @@ 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");
}
};
http://git-wip-us.apache.org/repos/asf/struts/blob/cb590742/xwork-core/src/test/resources/xwork-test-beans.xml
----------------------------------------------------------------------
diff --git a/xwork-core/src/test/resources/xwork-test-beans.xml b/xwork-core/src/test/resources/xwork-test-beans.xml
index 7268ef7..1606f1d 100644
--- a/xwork-core/src/test/resources/xwork-test-beans.xml
+++ b/xwork-core/src/test/resources/xwork-test-beans.xml
@@ -3,25 +3,7 @@
"http://struts.apache.org/dtds/xwork-2.0.dtd">
<xwork>
-<!--
- <bean class="com.opensymphony.xwork2.ObjectFactory" name="default" />
- <bean type="com.opensymphony.xwork2.ActionProxyFactory" name="default" class="com.opensymphony.xwork2.DefaultActionProxyFactory"/>
- <constant name="devMode" value="false" />
- <bean type="com.opensymphony.xwork2.util.ValueStackFactory"
- class="com.opensymphony.xwork2.ognl.OgnlValueStackFactory" />
- <bean type="com.opensymphony.xwork2.util.reflection.ReflectionProvider"
- class="com.opensymphony.xwork2.ognl.OgnlReflectionProvider" />
- <bean type="com.opensymphony.xwork2.util.reflection.ReflectionContextFactory"
- class="com.opensymphony.xwork2.ognl.OgnlReflectionContextFactory" />
- <bean class="com.opensymphony.xwork2.conversion.impl.XWorkConverter" />
- <bean type="com.opensymphony.xwork2.conversion.ObjectTypeDeterminer"
- class="com.opensymphony.xwork2.conversion.impl.DefaultObjectTypeDeterminer" />
--->
- <!-- static injections -->
- <!--
- <bean class="com.opensymphony.xwork2.ognl.OgnlValueStack" static="true"/>
- <bean class="com.opensymphony.xwork2.conversion.impl.XWorkConverter" static="true"/>
- <bean class="com.opensymphony.xwork2.util.reflection.ReflectionProviderFactory" static="true" />
- -->
+ <constant name="ognlExcludedClasses" value="java.lang.Object,java.lang.Runtime,ognl.OgnlContext,ognl.MemberAccess,ognl.ClassResolver,ognl.TypeConverter,com.opensymphony.xwork2.ognl.SecurityMemberAccess" />
+
</xwork>
\ No newline at end of file
[2/2] git commit: Adds special treatment of Object class and unit test
Posted by lu...@apache.org.
Adds special treatment of Object class and unit test
Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/b3ca9ea5
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/b3ca9ea5
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/b3ca9ea5
Branch: refs/heads/feature/exclude-object-class
Commit: b3ca9ea5e31fc9b6c0a5e644e833874bb7cc62fa
Parents: cb59074
Author: Lukasz Lenart <lu...@apache.org>
Authored: Sun May 4 11:18:00 2014 +0200
Committer: Lukasz Lenart <lu...@apache.org>
Committed: Sun May 4 11:18:00 2014 +0200
----------------------------------------------------------------------
.../xwork2/ognl/SecurityMemberAccess.java | 11 +-
.../xwork2/ognl/SecurityMemberAccessTest.java | 139 +++++++++++++++++++
2 files changed, 146 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/struts/blob/b3ca9ea5/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
index 9d84702..7fe77c3 100644
--- a/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/xwork-core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -21,6 +21,7 @@ import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Collections;
+import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
@@ -47,8 +48,7 @@ public class SecurityMemberAccess extends DefaultMemberAccess {
}
@Override
- public boolean isAccessible(Map context, Object target, Member member,
- String propertyName) {
+ public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
if (isClassExcluded(target.getClass(), member.getDeclaringClass())) {
return false;
@@ -79,8 +79,11 @@ public class SecurityMemberAccess extends DefaultMemberAccess {
}
protected boolean isClassExcluded(Class<?> targetClass, Class<?> declaringClass) {
- for (Class excludedClass : excludedClasses) {
- if (targetClass.isAssignableFrom(excludedClass) || declaringClass.isAssignableFrom(excludedClass)) {
+ if (targetClass == Object.class || declaringClass == Object.class) {
+ return true;
+ }
+ for (Class<?> excludedClass : excludedClasses) {
+ if (excludedClass.isAssignableFrom(targetClass) || declaringClass.isAssignableFrom(excludedClass)) {
return true;
}
}
http://git-wip-us.apache.org/repos/asf/struts/blob/b3ca9ea5/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
----------------------------------------------------------------------
diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
new file mode 100644
index 0000000..4ccc831
--- /dev/null
+++ b/xwork-core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -0,0 +1,139 @@
+package com.opensymphony.xwork2.ognl;
+
+import junit.framework.TestCase;
+
+import java.lang.reflect.Member;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+public class SecurityMemberAccessTest extends TestCase {
+
+ private Map context;
+ private FooBar target;
+
+ @Override
+ public void setUp() throws Exception {
+ context = new HashMap();
+ target = new FooBar();
+ }
+
+ public void testWithoutClassExclusion() throws Exception {
+ // given
+ SecurityMemberAccess sma = new SecurityMemberAccess(false);
+
+ String propertyName = "stringField";
+ Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
+
+ // when
+ boolean accessible = sma.isAccessible(context, target, member, propertyName);
+
+ // then
+ assertTrue(accessible);
+ }
+
+ public void testClassExclusion() throws Exception {
+ // given
+ SecurityMemberAccess sma = new SecurityMemberAccess(false);
+
+ String propertyName = "stringField";
+ Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1));
+
+ Set<Class<?>> excluded = new HashSet<Class<?>>();
+ excluded.add(FooBar.class);
+ sma.setExcludedClasses(excluded);
+
+ // when
+ boolean accessible = sma.isAccessible(context, target, member, propertyName);
+
+ // then
+ assertFalse(accessible);
+ }
+
+ public void testObjectClassExclusion() throws Exception {
+ // given
+ SecurityMemberAccess sma = new SecurityMemberAccess(false);
+
+ String propertyName = "toString";
+ Member member = FooBar.class.getMethod(propertyName);
+
+ // when
+ boolean accessible = sma.isAccessible(context, target, member, propertyName);
+
+ // then
+ assertFalse("toString() from Object is accessible!!!", accessible);
+ }
+
+ public void testObjectOverwrittenMethodsExclusion() throws Exception {
+ // given
+ SecurityMemberAccess sma = new SecurityMemberAccess(false);
+
+ String propertyName = "hashCode";
+ Member member = FooBar.class.getMethod(propertyName);
+
+ // when
+ boolean accessible = sma.isAccessible(context, target, member, propertyName);
+
+ // then
+ assertTrue("hashCode() from FooBar isn't accessible!!!", accessible);
+ }
+
+ public void testInterfaceInheritanceExclusion() throws Exception {
+ // given
+ SecurityMemberAccess sma = new SecurityMemberAccess(false);
+
+ String propertyName = "barLogic";
+ Member member = FooBar.class.getMethod("barLogic");
+
+ Set<Class<?>> excluded = new HashSet<Class<?>>();
+ excluded.add(BarInterface.class);
+ sma.setExcludedClasses(excluded);
+
+ // when
+ boolean accessible = sma.isAccessible(context, target, member, propertyName);
+
+ // then
+ assertFalse("barLogic() from BarInterface is accessible!!!", accessible);
+ }
+
+}
+
+class FooBar implements FooInterface {
+
+ private String stringField;
+
+ public String getStringField() {
+ return stringField;
+ }
+
+ public void setStringField(String stringField) {
+ this.stringField = stringField;
+ }
+
+ public String fooLogic() {
+ return "fooLogic";
+ }
+
+ public String barLogic() {
+ return "barLogic";
+ }
+
+ @Override
+ public int hashCode() {
+ return 1;
+ }
+
+}
+
+interface FooInterface extends BarInterface {
+
+ String fooLogic();
+
+}
+
+interface BarInterface {
+
+ String barLogic();
+
+}