Repository: struts Updated Branches: refs/heads/master 249d2f8d8 -> 6fb870d38
WW-4575 Uses startWith instead of pattern matching to improve overall performance Conflicts: core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java Project: http://git-wip-us.apache.org/repos/asf/struts/repo Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/e9d92f50 Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/e9d92f50 Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/e9d92f50 Branch: refs/heads/master Commit: e9d92f5000a827ac83b63aada624946854a77fef Parents: 249d2f8 Author: Lukasz Lenart <lukaszlen...@apache.org> Authored: Tue Jan 26 09:33:48 2016 +0100 Committer: Lukasz Lenart <lukaszlen...@apache.org> Committed: Tue Jan 26 11:03:40 2016 +0100 ---------------------------------------------------------------------- .../com/opensymphony/xwork2/XWorkConstants.java | 1 + .../com/opensymphony/xwork2/ognl/OgnlUtil.java | 15 +++++++++++++-- .../xwork2/ognl/OgnlValueStack.java | 1 + .../xwork2/ognl/SecurityMemberAccess.java | 16 +++++++++++++++- core/src/main/resources/struts-default.xml | 20 +++++++------------- .../xwork2/ognl/SecurityMemberAccessTest.java | 20 +++++++++++++++++++- 6 files changed, 56 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java b/core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java index 433b005..bc532d0 100644 --- a/core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java +++ b/core/src/main/java/com/opensymphony/xwork2/XWorkConstants.java @@ -20,6 +20,7 @@ public final class XWorkConstants { public static final String OGNL_EXCLUDED_CLASSES = "ognlExcludedClasses"; public static final String OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS = "ognlExcludedPackageNamePatterns"; + public static final String OGNL_EXCLUDED_PACKAGE_NAMES = "ognlExcludedPackageNames"; public static final String ADDITIONAL_EXCLUDED_PATTERNS = "additionalExcludedPatterns"; public static final String ADDITIONAL_ACCEPTED_PATTERNS = "additionalAcceptedPatterns"; http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index 8143613..679c804 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -59,6 +59,7 @@ public class OgnlUtil { private Set<Class<?>> excludedClasses = new HashSet<>(); private Set<Pattern> excludedPackageNamePatterns = new HashSet<>(); + private Set<String> excludedPackageNames = new HashSet<>(); private Container container; private boolean allowStaticMethodAccess; @@ -100,13 +101,18 @@ public class OgnlUtil { } @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) - public void setExcludedPackageName(String commaDelimitedPackagePatterns) { + public void setExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { Set<String> packagePatterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackagePatterns); for (String pattern : packagePatterns) { - excludedPackageNamePatterns.add(Pattern.compile(pattern)); + excludedPackageNamePatterns.add(Pattern.compile(pattern)); } } + @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAMES, required = false) + public void setExcludedPackageNames(String commaDelimitedPackageNames) { + excludedPackageNames = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames); + } + public Set<Class<?>> getExcludedClasses() { return excludedClasses; } @@ -115,6 +121,10 @@ public class OgnlUtil { return excludedPackageNamePatterns; } + public Set<String> getExcludedPackageNames() { + return excludedPackageNames; + } + @Inject public void setContainer(Container container) { this.container = container; @@ -572,6 +582,7 @@ public class OgnlUtil { SecurityMemberAccess memberAccess = new SecurityMemberAccess(allowStaticMethodAccess); memberAccess.setExcludedClasses(excludedClasses); memberAccess.setExcludedPackageNamePatterns(excludedPackageNamePatterns); + memberAccess.setExcludedPackageNames(excludedPackageNames); return Ognl.createDefaultContext(root, resolver, defaultConverter, memberAccess); } http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java index 4394d03..7201ace 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java @@ -82,6 +82,7 @@ public class OgnlValueStack implements Serializable, ValueStack, ClearableValueS this.ognlUtil = ognlUtil; securityMemberAccess.setExcludedClasses(ognlUtil.getExcludedClasses()); securityMemberAccess.setExcludedPackageNamePatterns(ognlUtil.getExcludedPackageNamePatterns()); + securityMemberAccess.setExcludedPackageNames(ognlUtil.getExcludedPackageNames()); } protected void setRoot(XWorkConverter xworkConverter, CompoundRootAccessor accessor, CompoundRoot compoundRoot, http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java index 2afd3d6..ecda59a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -40,6 +40,7 @@ public class SecurityMemberAccess extends DefaultMemberAccess { private Set<Pattern> acceptProperties = Collections.emptySet(); private Set<Class<?>> excludedClasses = Collections.emptySet(); private Set<Pattern> excludedPackageNamePatterns = Collections.emptySet(); + private Set<String> excludedPackageNames = Collections.emptySet(); public SecurityMemberAccess(boolean method) { super(false); @@ -123,16 +124,25 @@ public class SecurityMemberAccess extends DefaultMemberAccess { final String targetPackageName = targetPackage == null ? "" : targetPackage.getName(); final String memberPackageName = memberPackage == null ? "" : memberPackage.getName(); + for (Pattern pattern : excludedPackageNamePatterns) { if (pattern.matcher(targetPackageName).matches() || pattern.matcher(memberPackageName).matches()) { return true; } } + + for (String packageName: excludedPackageNames) { + if (targetPackageName.startsWith(packageName) || targetPackageName.equals(packageName) + || memberPackageName.startsWith(packageName) || memberPackageName.equals(packageName)) { + return true; + } + } + return false; } protected boolean isClassExcluded(Class<?> clazz) { - if (clazz == Object.class) { + if (clazz == Object.class || (clazz == Class.class && !allowStaticMethodAccess)) { return true; } for (Class<?> excludedClass : excludedClasses) { @@ -191,4 +201,8 @@ public class SecurityMemberAccess extends DefaultMemberAccess { public void setExcludedPackageNamePatterns(Set<Pattern> excludedPackageNamePatterns) { this.excludedPackageNamePatterns = excludedPackageNamePatterns; } + + public void setExcludedPackageNames(Set<String> excludedPackageNames) { + this.excludedPackageNames = excludedPackageNames; + } } http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/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 1f5fa17..82bc63b 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -39,20 +39,14 @@ <struts> <constant name="struts.excludedClasses" - value=" - java.lang.Object, - java.lang.Runtime, - java.lang.System, - java.lang.Class, - java.lang.ClassLoader, - java.lang.Shutdown, - ognl.OgnlContext, - ognl.MemberAccess, - ognl.ClassResolver, - ognl.TypeConverter, - com.opensymphony.xwork2.ActionContext" /> + value="com.opensymphony.xwork2.ActionContext" /> + <!-- this must be valid regex, each '.' in package name must be escaped! --> - <constant name="struts.excludedPackageNamePatterns" value="^java\.lang\..*,^ognl.*,^(?!javax\.servlet\..+)(javax\..+)" /> + <!-- it's more flexible but slower than simple string comparison --> + <!-- constant name="struts.excludedPackageNamePatterns" value="^java\.lang\..*,^ognl.*,^(?!javax\.servlet\..+)(javax\..+)" / --> + + <!-- this is simpler version of the above used with string comparison --> + <constant name="struts.excludedPackageNames" value="java.lang,ognl,javax" /> <bean class="com.opensymphony.xwork2.ObjectFactory" name="struts"/> <bean type="com.opensymphony.xwork2.factory.ResultFactory" name="struts" class="org.apache.struts2.factory.StrutsResultFactory" /> http://git-wip-us.apache.org/repos/asf/struts/blob/e9d92f50/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java index 5db20fc..778f919 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -191,6 +191,24 @@ public class SecurityMemberAccessTest extends TestCase { assertFalse("stringField is accessible!", actual); } + public void testPackageNameExclusion() throws Exception { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + Set<String> excluded = new HashSet<String>(); + excluded.add(FooBar.class.getPackage().getName()); + sma.setExcludedPackageNames(excluded); + + String propertyName = "stringField"; + Member member = FooBar.class.getMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); + + // when + boolean actual = sma.isAccessible(context, target, member, propertyName); + + // then + assertFalse("stringField is accessible!", actual); + } + public void testDefaultPackageExclusion() throws Exception { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); @@ -274,7 +292,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAllowStaticAccessIfClassIsNotExcluded() throws Exception { // given - SecurityMemberAccess sma = new SecurityMemberAccess(false); + SecurityMemberAccess sma = new SecurityMemberAccess(true); sma.setExcludedClasses(new HashSet<Class<?>>(Arrays.<Class<?>>asList(ClassLoader.class))); // when