This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/struts.git
commit c2b2511f851da37055df2b13fb7405afe5f316e2 Author: Lukasz Lenart <lukaszlen...@apache.org> AuthorDate: Wed Jun 20 11:47:53 2018 +0200 Adds proper handling of primitive types --- .../xwork2/ognl/SecurityMemberAccess.java | 11 ++++-- core/src/main/resources/struts-default.xml | 2 +- .../xwork2/ognl/SecurityMemberAccessTest.java | 46 ++++++++++++++-------- 3 files changed, 38 insertions(+), 21 deletions(-) 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 05db70f..5d87944 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -154,9 +154,9 @@ public class SecurityMemberAccess implements MemberAccess { if (targetPackage == null || memberPackage == null) { LOG.warn("The use of the default (unnamed) package is discouraged!"); } - - final String targetPackageName = targetPackage == null ? "" : targetPackage.getName(); - final String memberPackageName = memberPackage == null ? "" : memberPackage.getName(); + + String targetPackageName = targetPackage == null ? "" : targetPackage.getName(); + String memberPackageName = memberPackage == null ? "" : memberPackage.getName(); for (Pattern pattern : excludedPackageNamePatterns) { if (pattern.matcher(targetPackageName).matches() || pattern.matcher(memberPackageName).matches()) { @@ -164,7 +164,10 @@ public class SecurityMemberAccess implements MemberAccess { } } - for (String packageName : excludedPackageNames) { + targetPackageName = targetPackageName + "."; + memberPackageName = memberPackageName + "."; + + for (String packageName: excludedPackageNames) { if (targetPackageName.startsWith(packageName) || targetPackageName.equals(packageName) || memberPackageName.startsWith(packageName) || memberPackageName.equals(packageName)) { return true; diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 3896a51..417e6d8 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -62,7 +62,7 @@ com.opensymphony.xwork2.ognl., java.lang., ognl., - javax, + javax., freemarker.core., freemarker.template., freemarker.ext.rhino., 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 c78e9ab..ddb3f28 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -22,7 +22,6 @@ import com.opensymphony.xwork2.util.TextParseUtil; import junit.framework.TestCase; import java.lang.reflect.Member; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -62,7 +61,7 @@ public class SecurityMemberAccessTest extends TestCase { String propertyName = "stringField"; Member member = FooBar.class.getDeclaredMethod("get" + propertyName.substring(0, 1).toUpperCase() + propertyName.substring(1)); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(FooBar.class); sma.setExcludedClasses(excluded); @@ -108,7 +107,7 @@ public class SecurityMemberAccessTest extends TestCase { String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(BarInterface.class); sma.setExcludedClasses(excluded); @@ -126,7 +125,7 @@ public class SecurityMemberAccessTest extends TestCase { String propertyName = "fooLogic"; Member member = FooBar.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(BarInterface.class); sma.setExcludedClasses(excluded); @@ -158,7 +157,7 @@ public class SecurityMemberAccessTest extends TestCase { String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(FooBarInterface.class); sma.setExcludedClasses(excluded); @@ -173,7 +172,7 @@ public class SecurityMemberAccessTest extends TestCase { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Pattern> excluded = new HashSet<Pattern>(); + Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); sma.setExcludedPackageNamePatterns(excluded); @@ -191,7 +190,7 @@ public class SecurityMemberAccessTest extends TestCase { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<String> excluded = new HashSet<String>(); + Set<String> excluded = new HashSet<>(); excluded.add(FooBar.class.getPackage().getName()); sma.setExcludedPackageNames(excluded); @@ -205,11 +204,11 @@ public class SecurityMemberAccessTest extends TestCase { assertFalse("stringField is accessible!", actual); } - public void testDefaultPackageExclusion() throws Exception { + public void testDefaultPackageExclusion() { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Pattern> excluded = new HashSet<Pattern>(); + Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); sma.setExcludedPackageNamePatterns(excluded); @@ -220,11 +219,11 @@ public class SecurityMemberAccessTest extends TestCase { assertFalse("default package is excluded!", actual); } - public void testDefaultPackageExclusion2() throws Exception { + public void testDefaultPackageExclusion2() { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Pattern> excluded = new HashSet<Pattern>(); + Set<Pattern> excluded = new HashSet<>(); excluded.add(Pattern.compile("^$")); sma.setExcludedPackageNamePatterns(excluded); @@ -317,10 +316,10 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithNames() throws Exception { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.,ognl,javax")); + sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("ognl.,javax.")); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(Object.class); excluded.add(Runtime.class); excluded.add(System.class); @@ -369,7 +368,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessPrimitiveDoubleWithPackageRegExs() throws Exception { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Pattern> patterns = new HashSet<Pattern>(); + Set<Pattern> patterns = new HashSet<>(); patterns.add(Pattern.compile("^java\\.lang\\..*")); sma.setExcludedPackageNamePatterns(patterns); @@ -386,7 +385,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsAccessible() throws Exception { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(ognl.MemberAccess.class); sma.setExcludedClasses(excluded); @@ -404,7 +403,7 @@ public class SecurityMemberAccessTest extends TestCase { public void testAccessMemberAccessIsBlocked() throws Exception { // given SecurityMemberAccess sma = new SecurityMemberAccess(false); - Set<Class<?>> excluded = new HashSet<Class<?>>(); + Set<Class<?>> excluded = new HashSet<>(); excluded.add(SecurityMemberAccess.class); sma.setExcludedClasses(excluded); @@ -419,6 +418,21 @@ public class SecurityMemberAccessTest extends TestCase { assertFalse(accessible); } + public void testPackageNameExclusionAsCommaDelimited() { + // given + SecurityMemberAccess sma = new SecurityMemberAccess(false); + + + sma.setExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang.")); + + // when + boolean actual = sma.isPackageExcluded(String.class.getPackage(), null); + actual &= sma.isPackageExcluded(null, String.class.getPackage()); + + // then + assertTrue("package java.lang. is accessible!", actual); + } + } class FooBar implements FooBarInterface {