This is an automated email from the ASF dual-hosted git repository. kusal pushed a commit to branch WW-5341-classloaders in repository https://gitbox.apache.org/repos/asf/struts.git
commit d8e688ba6b581c930eba7fda16dfe2ac546786d4 Author: Kusal Kithul-Godage <g...@kusal.io> AuthorDate: Mon Aug 28 22:23:49 2023 +1000 WW-5341 Further refactor of OgnlUtil and SecurityMemberAccess to store excluded classes as Strings --- .../com/opensymphony/xwork2/ognl/OgnlUtil.java | 127 +++++++++------------ .../xwork2/ognl/SecurityMemberAccess.java | 24 ++-- .../xwork2/ognl/OgnlUtilStrutsTest.java | 4 +- .../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 85 +++++--------- .../xwork2/ognl/SecurityMemberAccessTest.java | 100 ++++++++-------- 5 files changed, 145 insertions(+), 195 deletions(-) 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 005d17eba..c8639a1e6 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -43,7 +43,6 @@ import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -53,6 +52,8 @@ import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import static com.opensymphony.xwork2.util.TextParseUtil.commaDelimitedStringToSet; +import static java.util.Collections.emptySet; +import static java.util.Collections.unmodifiableSet; import static java.util.stream.Collectors.toSet; import static org.apache.commons.lang3.StringUtils.strip; @@ -78,15 +79,15 @@ public class OgnlUtil { private boolean enableExpressionCache = true; private boolean enableEvalExpression; - private Set<Class<?>> excludedClasses; + private Set<String> excludedClasses; private Set<Pattern> excludedPackageNamePatterns; private Set<String> excludedPackageNames; - private Set<Class<?>> excludedPackageExemptClasses; + private Set<String> excludedPackageExemptClasses; - private Set<Class<?>> devModeExcludedClasses; + private Set<String> devModeExcludedClasses; private Set<Pattern> devModeExcludedPackageNamePatterns; private Set<String> devModeExcludedPackageNames; - private Set<Class<?>> devModeExcludedPackageExemptClasses; + private Set<String> devModeExcludedPackageExemptClasses; private Container container; private boolean allowStaticFieldAccess = true; @@ -126,18 +127,18 @@ public class OgnlUtil { if (ognlBeanInfoCacheFactory == null) { throw new IllegalArgumentException("BeanInfoCacheFactory parameter cannot be null"); } - excludedClasses = Collections.unmodifiableSet(new HashSet<>()); - excludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); - excludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); - excludedPackageExemptClasses = Collections.unmodifiableSet(new HashSet<>()); + excludedClasses = emptySet(); + excludedPackageNamePatterns = emptySet(); + excludedPackageNames = emptySet(); + excludedPackageExemptClasses = emptySet(); - devModeExcludedClasses = Collections.unmodifiableSet(new HashSet<>()); - devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(new HashSet<>()); - devModeExcludedPackageNames = Collections.unmodifiableSet(new HashSet<>()); - devModeExcludedPackageExemptClasses = Collections.unmodifiableSet(new HashSet<>()); + devModeExcludedClasses = emptySet(); + devModeExcludedPackageNamePatterns = emptySet(); + devModeExcludedPackageNames = emptySet(); + devModeExcludedPackageExemptClasses = emptySet(); - this.expressionCache = ognlExpressionCacheFactory.buildOgnlCache(); - this.beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(); + expressionCache = ognlExpressionCacheFactory.buildOgnlCache(); + beanInfoCache = ognlBeanInfoCacheFactory.buildOgnlCache(); } @Inject @@ -176,101 +177,87 @@ public class OgnlUtil { @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES, required = false) protected void setExcludedClasses(String commaDelimitedClasses) { - Set<Class<?>> excludedClasses = new HashSet<>(); - excludedClasses.addAll(this.excludedClasses); - excludedClasses.addAll(parseClasses(commaDelimitedClasses)); - this.excludedClasses = Collections.unmodifiableSet(excludedClasses); + excludedClasses = toNewClassesSet(excludedClasses, commaDelimitedClasses); } @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_CLASSES, required = false) protected void setDevModeExcludedClasses(String commaDelimitedClasses) { - Set<Class<?>> excludedClasses = new HashSet<>(); - excludedClasses.addAll(this.devModeExcludedClasses); - excludedClasses.addAll(parseClasses(commaDelimitedClasses)); - this.devModeExcludedClasses = Collections.unmodifiableSet(excludedClasses); + devModeExcludedClasses = toNewClassesSet(devModeExcludedClasses, commaDelimitedClasses); } - private Set<Class<?>> parseClasses(String commaDelimitedClasses) { - Set<String> classNames = commaDelimitedStringToSet(commaDelimitedClasses); - Set<Class<?>> classes = new HashSet<>(); + private static Set<String> toNewClassesSet(Set<String> oldClasses, String newDelimitedClasses) { + Set<String> classNames = commaDelimitedStringToSet(newDelimitedClasses); + validateClasses(classNames); + Set<String> excludedClasses = new HashSet<>(oldClasses); + excludedClasses.addAll(classNames); + return unmodifiableSet(excludedClasses); + } + + private static void validateClasses(Set<String> classNames) throws ConfigurationException { for (String className : classNames) { try { - classes.add(Class.forName(className)); + ClassLoader.getSystemClassLoader().loadClass(className); } catch (ClassNotFoundException e) { throw new ConfigurationException("Cannot load class for exclusion/exemption configuration: " + className, e); } } - return classes; } @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) protected void setExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { - Set<Pattern> excludedPackageNamePatterns = new HashSet<>(); - excludedPackageNamePatterns.addAll(this.excludedPackageNamePatterns); - excludedPackageNamePatterns.addAll(parseExcludedPackageNamePatterns(commaDelimitedPackagePatterns)); - this.excludedPackageNamePatterns = Collections.unmodifiableSet(excludedPackageNamePatterns); + excludedPackageNamePatterns = toNewPatternsSet(excludedPackageNamePatterns, commaDelimitedPackagePatterns); } @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false) protected void setDevModeExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { - Set<Pattern> excludedPackageNamePatterns = new HashSet<>(); - excludedPackageNamePatterns.addAll(this.devModeExcludedPackageNamePatterns); - excludedPackageNamePatterns.addAll(parseExcludedPackageNamePatterns(commaDelimitedPackagePatterns)); - this.devModeExcludedPackageNamePatterns = Collections.unmodifiableSet(excludedPackageNamePatterns); + devModeExcludedPackageNamePatterns = toNewPatternsSet(devModeExcludedPackageNamePatterns, commaDelimitedPackagePatterns); } - private Set<Pattern> parseExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) { - try { - return commaDelimitedStringToSet(commaDelimitedPackagePatterns) - .stream().map(Pattern::compile).collect(toSet()); - } catch (PatternSyntaxException e) { - throw new ConfigurationException( - "Excluded package name patterns could not be parsed due to invalid regex: " + commaDelimitedPackagePatterns, e); + private static Set<Pattern> toNewPatternsSet(Set<Pattern> oldPatterns, String newDelimitedPatterns) throws ConfigurationException { + Set<String> patterns = commaDelimitedStringToSet(newDelimitedPatterns); + Set<Pattern> newPatterns = new HashSet<>(oldPatterns); + for (String pattern: patterns) { + try { + newPatterns.add(Pattern.compile(pattern)); + } catch (PatternSyntaxException e) { + throw new ConfigurationException("Excluded package name patterns could not be parsed due to invalid regex: " + pattern, e); + } } + return unmodifiableSet(newPatterns); } @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES, required = false) protected void setExcludedPackageNames(String commaDelimitedPackageNames) { - Set<String> excludedPackageNames = new HashSet<>(); - excludedPackageNames.addAll(this.excludedPackageNames); - excludedPackageNames.addAll(parseExcludedPackageNames(commaDelimitedPackageNames)); - this.excludedPackageNames = Collections.unmodifiableSet(excludedPackageNames); + excludedPackageNames = toNewPackageNamesSet(excludedPackageNames, commaDelimitedPackageNames); } @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_NAMES, required = false) protected void setDevModeExcludedPackageNames(String commaDelimitedPackageNames) { - Set<String> excludedPackageNames = new HashSet<>(); - excludedPackageNames.addAll(this.devModeExcludedPackageNames); - excludedPackageNames.addAll(parseExcludedPackageNames(commaDelimitedPackageNames)); - this.devModeExcludedPackageNames = Collections.unmodifiableSet(excludedPackageNames); + devModeExcludedPackageNames = toNewPackageNamesSet(devModeExcludedPackageNames, commaDelimitedPackageNames); + } + + private static Set<String> toNewPackageNamesSet(Set<String> oldPackageNames, String newDelimitedPackageNames) throws ConfigurationException { + Set<String> packageNames = commaDelimitedStringToSet(newDelimitedPackageNames) + .stream().map(s -> strip(s, ".")).collect(toSet()); + if (packageNames.stream().anyMatch(s -> s.matches("(.*?)\\s(.*?)"))) { + throw new ConfigurationException("Excluded package names could not be parsed due to erroneous whitespace characters: " + newDelimitedPackageNames); + } + Set<String> newPackageNames = new HashSet<>(oldPackageNames); + newPackageNames.addAll(packageNames); + return unmodifiableSet(newPackageNames); } @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) public void setExcludedPackageExemptClasses(String commaDelimitedClasses) { - Set<Class<?>> excludedPackageExemptClasses = new HashSet<>(); - excludedPackageExemptClasses.addAll(this.excludedPackageExemptClasses); - excludedPackageExemptClasses.addAll(parseClasses(commaDelimitedClasses)); - this.excludedPackageExemptClasses = Collections.unmodifiableSet(excludedPackageExemptClasses); + excludedPackageExemptClasses = toNewClassesSet(excludedPackageExemptClasses, commaDelimitedClasses); } @Inject(value = StrutsConstants.STRUTS_DEV_MODE_EXCLUDED_PACKAGE_EXEMPT_CLASSES, required = false) public void setDevModeExcludedPackageExemptClasses(String commaDelimitedClasses) { - Set<Class<?>> excludedPackageExemptClasses = new HashSet<>(); - excludedPackageExemptClasses.addAll(this.devModeExcludedPackageExemptClasses); - excludedPackageExemptClasses.addAll(parseClasses(commaDelimitedClasses)); - this.devModeExcludedPackageExemptClasses = Collections.unmodifiableSet(excludedPackageExemptClasses); - } - - private Set<String> parseExcludedPackageNames(String commaDelimitedPackageNames) { - Set<String> parsedSet = commaDelimitedStringToSet(commaDelimitedPackageNames) - .stream().map(s -> strip(s, ".")).collect(toSet()); - if (parsedSet.stream().anyMatch(s -> s.matches("(.*?)\\s(.*?)"))) { - throw new ConfigurationException("Excluded package names could not be parsed due to erroneous whitespace characters: " + commaDelimitedPackageNames); - } - return parsedSet; + devModeExcludedPackageExemptClasses = toNewClassesSet(devModeExcludedPackageExemptClasses, commaDelimitedClasses); } - public Set<Class<?>> getExcludedClasses() { + public Set<String> getExcludedClasses() { return excludedClasses; } @@ -282,7 +269,7 @@ public class OgnlUtil { return excludedPackageNames; } - public Set<Class<?>> getExcludedPackageExemptClasses() { + public Set<String> getExcludedPackageExemptClasses() { return excludedPackageExemptClasses; } 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 48d36a4fd..f65c322b8 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java @@ -35,8 +35,10 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static java.text.MessageFormat.format; import static java.util.Collections.emptySet; import static java.util.Collections.unmodifiableSet; +import static java.util.stream.Collectors.toSet; /** * Allows access decisions to be made on the basis of whether a member is static or not. @@ -49,10 +51,10 @@ public class SecurityMemberAccess implements MemberAccess { private final boolean allowStaticFieldAccess; private Set<Pattern> excludeProperties = emptySet(); private Set<Pattern> acceptProperties = emptySet(); - private Set<Class<?>> excludedClasses = emptySet(); + private Set<String> excludedClasses = emptySet(); private Set<Pattern> excludedPackageNamePatterns = emptySet(); private Set<String> excludedPackageNames = emptySet(); - private Set<Class<?>> excludedPackageExemptClasses = emptySet(); + private Set<String> excludedPackageExemptClasses = emptySet(); private boolean disallowProxyMemberAccess; /** @@ -223,7 +225,7 @@ public class SecurityMemberAccess implements MemberAccess { List<Class<?>> classesToCheck = Arrays.asList(targetClass, memberClass); for (Class<?> clazz : classesToCheck) { - if (!excludedPackageExemptClasses.contains(clazz) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz))) { + if (!excludedPackageExemptClasses.contains(clazz.getName()) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz))) { return true; } } @@ -257,7 +259,7 @@ public class SecurityMemberAccess implements MemberAccess { } protected boolean isClassExcluded(Class<?> clazz) { - return excludedClasses.contains(clazz); + return excludedClasses.contains(clazz.getName()); } protected boolean isAcceptableProperty(String name) { @@ -322,14 +324,14 @@ public class SecurityMemberAccess implements MemberAccess { */ @Deprecated public void setExcludedClasses(Set<Class<?>> excludedClasses) { - useExcludedClasses(excludedClasses); + useExcludedClasses(excludedClasses.stream().map(Class::getName).collect(toSet())); } - public void useExcludedClasses(Set<Class<?>> excludedClasses) { - Set<Class<?>> newExcludedClasses = new HashSet<>(excludedClasses); - newExcludedClasses.add(Object.class); + public void useExcludedClasses(Set<String> excludedClasses) { + Set<String> newExcludedClasses = new HashSet<>(excludedClasses); + newExcludedClasses.add(Object.class.getName()); if (!allowStaticFieldAccess) { - newExcludedClasses.add(Class.class); + newExcludedClasses.add(Class.class.getName()); } this.excludedClasses = unmodifiableSet(newExcludedClasses); } @@ -363,10 +365,10 @@ public class SecurityMemberAccess implements MemberAccess { */ @Deprecated public void setExcludedPackageExemptClasses(Set<Class<?>> excludedPackageExemptClasses) { - this.excludedPackageExemptClasses = excludedPackageExemptClasses; + useExcludedPackageExemptClasses(excludedPackageExemptClasses.stream().map(Class::getName).collect(toSet())); } - public void useExcludedPackageExemptClasses(Set<Class<?>> excludedPackageExemptClasses) { + public void useExcludedPackageExemptClasses(Set<String> excludedPackageExemptClasses) { this.excludedPackageExemptClasses = excludedPackageExemptClasses; } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java index c644bfed4..0eaf517a4 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java @@ -35,8 +35,8 @@ public class OgnlUtilStrutsTest extends StrutsInternalTestCase { ognlUtil.setExcludedClasses(""); ognlUtil.setExcludedPackageNames(""); ognlUtil.setExcludedPackageNamePatterns(""); - assertTrue(ognlUtil.getExcludedClasses().size() > 0); - assertTrue(ognlUtil.getExcludedPackageNames().size() > 0); + assertFalse(ognlUtil.getExcludedClasses().isEmpty()); + assertFalse(ognlUtil.getExcludedPackageNames().isEmpty()); try { ognlUtil.getExcludedClasses().clear(); diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 0cafd799b..736889886 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -903,7 +903,7 @@ public class OgnlUtilTest extends XWorkTestCase { assertEquals(foo.getTitle(), expression); SecurityMemberAccess sma = (SecurityMemberAccess) ((OgnlContext) context).getMemberAccess(); - assertFalse(sma.isAccessible(context, sma, sma.getClass().getDeclaredMethod("setExcludedClasses", Set.class), "excludedClasses")); + assertFalse(sma.isAccessible(context, sma, sma.getClass().getDeclaredMethod("useExcludedClasses", Set.class), "excludedClasses")); } public void testNullProperties() { @@ -1366,7 +1366,7 @@ public class OgnlUtilTest extends XWorkTestCase { } public void testOgnlUtilExcludedAdditivity() { - Set<Class<?>> excludedClasses; + Set<String> excludedClasses; Set<Pattern> excludedPackageNamePatterns; Iterator<Pattern> excludedPackageNamePatternsIterator; Set<String> excludedPackageNames; @@ -1376,17 +1376,17 @@ public class OgnlUtilTest extends XWorkTestCase { excludedClasses = ognlUtil.getExcludedClasses(); assertNotNull("initial excluded classes null?", excludedClasses); assertEquals("initial excluded classes size not 2 after adds?", 2, excludedClasses.size()); - assertTrue("String not in exclusions?", excludedClasses.contains(String.class)); - assertTrue("Integer not in exclusions?", excludedClasses.contains(Integer.class)); + assertTrue("String not in exclusions?", excludedClasses.contains(String.class.getName())); + assertTrue("Integer not in exclusions?", excludedClasses.contains(Integer.class.getName())); ognlUtil.setExcludedClasses("java.lang.Boolean,java.lang.Double"); internalTestOgnlUtilExclusionsImmutable(ognlUtil); excludedClasses = ognlUtil.getExcludedClasses(); assertNotNull("updated excluded classes null?", excludedClasses); assertEquals("updated excluded classes size not 4 after adds?", 4, excludedClasses.size()); - assertTrue("String not in exclusions?", excludedClasses.contains(String.class)); - assertTrue("Integer not in exclusions?", excludedClasses.contains(Integer.class)); - assertTrue("String not in exclusions?", excludedClasses.contains(Boolean.class)); - assertTrue("Integer not in exclusions?", excludedClasses.contains(Double.class)); + assertTrue("String not in exclusions?", excludedClasses.contains(String.class.getName())); + assertTrue("Integer not in exclusions?", excludedClasses.contains(Integer.class.getName())); + assertTrue("String not in exclusions?", excludedClasses.contains(Boolean.class.getName())); + assertTrue("Integer not in exclusions?", excludedClasses.contains(Double.class.getName())); ognlUtil.setExcludedPackageNamePatterns("fakepackage1.*,fakepackage2.*"); internalTestOgnlUtilExclusionsImmutable(ognlUtil); @@ -1617,7 +1617,7 @@ public class OgnlUtilTest extends XWorkTestCase { } private void internalTestInitialEmptyOgnlUtilExclusions(OgnlUtil ognlUtilParam) { - Set<Class<?>> excludedClasses = ognlUtilParam.getExcludedClasses(); + Set<String> excludedClasses = ognlUtilParam.getExcludedClasses(); assertNotNull("parameter (default) exluded classes null?", excludedClasses); assertTrue("parameter (default) exluded classes not empty?", excludedClasses.isEmpty()); @@ -1632,65 +1632,34 @@ public class OgnlUtilTest extends XWorkTestCase { private void internalTestOgnlUtilExclusionsImmutable(OgnlUtil ognlUtilParam) { Pattern somePattern = Pattern.compile("SomeRegexPattern"); - Set<Class<?>> excludedClasses = ognlUtilParam.getExcludedClasses(); - assertNotNull("parameter exluded classes null?", excludedClasses); - try { - excludedClasses.clear(); - fail("parameter excluded classes modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } - try { - excludedClasses.add(Integer.class); - fail("parameter excluded classes modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } - try { - excludedClasses.remove(Integer.class); - fail("parameter excluded classes modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } - - Set<Pattern> excludedPackageNamePatterns = ognlUtilParam.getExcludedPackageNamePatterns(); - assertNotNull("parameter exluded package name patterns null?", excludedPackageNamePatterns); - try { - excludedPackageNamePatterns.clear(); - fail("parameter excluded package name patterns modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } - try { - excludedPackageNamePatterns.add(somePattern); - fail("parameter excluded package name patterns modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } - try { - excludedPackageNamePatterns.remove(somePattern); - fail("parameter excluded package name patterns modifiable?"); - } catch (UnsupportedOperationException uoe) { - // Expected failure - } + assertImmutability(ognlUtilParam.getExcludedClasses(), Integer.class.getName()); + assertImmutability(ognlUtilParam.getExcludedPackageNamePatterns(), somePattern); + assertImmutability(ognlUtilParam.getExcludedPackageNames(), "somepackagename"); + assertImmutability(ognlUtilParam.getExcludedPackageExemptClasses(), Integer.class.getName()); + } - Set<String> excludedPackageNames = ognlUtilParam.getExcludedPackageNames(); - assertNotNull("parameter exluded package names null?", excludedPackageNames); + public static <T> void assertImmutability(Collection<T> collection, T item) { + assertNotNull("Collection is null", collection); try { - excludedPackageNames.clear(); - fail("parameter excluded package names modifiable?"); + if (!collection.isEmpty()) { + collection.clear(); + fail("Collection could be cleared"); + } } catch (UnsupportedOperationException uoe) { // Expected failure } try { - excludedPackageNames.add("somepackagename"); - fail("parameter excluded package names modifiable?"); + collection.add(item); + fail("Collection could be added to"); } catch (UnsupportedOperationException uoe) { // Expected failure } try { - excludedPackageNames.remove("somepackagename"); - fail("parameter excluded package names modifiable?"); + int prevSize = collection.size(); + collection.remove(item); + if (prevSize != collection.size()) { + fail("Collection could be removed from"); + } } catch (UnsupportedOperationException uoe) { // Expected failure } 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 1a6d9697d..b7b1bce09 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java @@ -25,7 +25,6 @@ import org.junit.Test; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; @@ -33,6 +32,7 @@ import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; +import static java.util.Collections.singletonList; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -73,8 +73,8 @@ public class SecurityMemberAccessTest { String propertyName = "stringField"; Member member = FooBar.class.getDeclaredMethod(formGetterName(propertyName)); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(FooBar.class); + Set<String> excluded = new HashSet<>(); + excluded.add(FooBar.class.getName()); sma.useExcludedClasses(excluded); // when @@ -116,8 +116,8 @@ public class SecurityMemberAccessTest { String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(BarInterface.class); + Set<String> excluded = new HashSet<>(); + excluded.add(BarInterface.class.getName()); sma.useExcludedClasses(excluded); // when @@ -133,8 +133,8 @@ public class SecurityMemberAccessTest { String propertyName = "fooLogic"; Member member = FooBar.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(BarInterface.class); + Set<String> excluded = new HashSet<>(); + excluded.add(BarInterface.class.getName()); sma.useExcludedClasses(excluded); // when @@ -150,8 +150,8 @@ public class SecurityMemberAccessTest { String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(BarInterface.class); + Set<String> excluded = new HashSet<>(); + excluded.add(BarInterface.class.getName()); sma.useExcludedClasses(excluded); // when @@ -167,8 +167,8 @@ public class SecurityMemberAccessTest { String propertyName = "barLogic"; Member member = BarInterface.class.getMethod(propertyName); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(FooInterface.class); + Set<String> excluded = new HashSet<>(); + excluded.add(FooInterface.class.getName()); sma.useExcludedClasses(excluded); // when @@ -202,8 +202,8 @@ public class SecurityMemberAccessTest { excluded.add(Pattern.compile("^" + FooBar.class.getPackage().getName().replaceAll("\\.", "\\\\.") + ".*")); sma.useExcludedPackageNamePatterns(excluded); - Set<Class<?>> allowed = new HashSet<>(); - allowed.add(FooBar.class); + Set<String> allowed = new HashSet<>(); + allowed.add(FooBar.class.getName()); sma.useExcludedPackageExemptClasses(allowed); String propertyName = "stringField"; @@ -240,8 +240,8 @@ public class SecurityMemberAccessTest { excluded.add(FooBar.class.getPackage().getName()); sma.useExcludedPackageNames(excluded); - Set<Class<?>> allowed = new HashSet<>(); - allowed.add(FooBar.class); + Set<String> allowed = new HashSet<>(); + allowed.add(FooBar.class.getName()); sma.useExcludedPackageExemptClasses(allowed); String propertyName = "stringField"; @@ -262,8 +262,8 @@ public class SecurityMemberAccessTest { sma.useExcludedPackageNames(excluded); // Exemption must exist for both classes (target and member) if they both match a banned package - Set<Class<?>> allowed = new HashSet<>(); - allowed.add(BarInterface.class); + Set<String> allowed = new HashSet<>(); + allowed.add(BarInterface.class.getName()); sma.useExcludedPackageExemptClasses(allowed); String propertyName = "barLogic"; @@ -284,9 +284,9 @@ public class SecurityMemberAccessTest { sma.useExcludedPackageNames(excluded); // Exemption must exist for both classes (target and member) if they both match a banned package - Set<Class<?>> allowed = new HashSet<>(); - allowed.add(BarInterface.class); - allowed.add(FooBar.class); + Set<String> allowed = new HashSet<>(); + allowed.add(BarInterface.class.getName()); + allowed.add(FooBar.class.getName()); sma.useExcludedPackageExemptClasses(allowed); String propertyName = "barLogic"; @@ -344,7 +344,7 @@ public class SecurityMemberAccessTest { @Test public void testAccessStaticMethod() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = StaticTester.class.getMethod("sayHello"); @@ -357,7 +357,7 @@ public class SecurityMemberAccessTest { @Test public void testAccessStaticField() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -371,7 +371,7 @@ public class SecurityMemberAccessTest { public void testBlockedStaticFieldWhenFlagIsTrue() throws Exception { // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -383,7 +383,7 @@ public class SecurityMemberAccessTest { // public static final test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.class.getField("MIN_VALUE"); @@ -395,7 +395,7 @@ public class SecurityMemberAccessTest { // package static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("PACKAGE_STRING"); @@ -407,7 +407,7 @@ public class SecurityMemberAccessTest { // package final static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("FINAL_PACKAGE_STRING"); @@ -419,7 +419,7 @@ public class SecurityMemberAccessTest { // protected static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("PROTECTED_STRING"); @@ -431,7 +431,7 @@ public class SecurityMemberAccessTest { // protected final static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("FINAL_PROTECTED_STRING"); @@ -443,7 +443,7 @@ public class SecurityMemberAccessTest { // private static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("PRIVATE_STRING"); @@ -455,7 +455,7 @@ public class SecurityMemberAccessTest { // private final static test // given assignNewSma(true); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when method = StaticTester.getFieldByName("FINAL_PRIVATE_STRING"); @@ -469,7 +469,6 @@ public class SecurityMemberAccessTest { public void testBlockedStaticFieldWhenFlagIsFalse() throws Exception { // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -481,7 +480,6 @@ public class SecurityMemberAccessTest { // public static final test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.class.getField("MIN_VALUE"); @@ -493,7 +491,6 @@ public class SecurityMemberAccessTest { // package static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PACKAGE_STRING"); @@ -505,7 +502,6 @@ public class SecurityMemberAccessTest { // package final static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PACKAGE_STRING"); @@ -517,7 +513,6 @@ public class SecurityMemberAccessTest { // protected static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PROTECTED_STRING"); @@ -529,7 +524,6 @@ public class SecurityMemberAccessTest { // protected final static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PROTECTED_STRING"); @@ -541,7 +535,6 @@ public class SecurityMemberAccessTest { // private static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("PRIVATE_STRING"); @@ -553,7 +546,6 @@ public class SecurityMemberAccessTest { // private final static test // given assignNewSma(false); - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); // when method = StaticTester.getFieldByName("FINAL_PRIVATE_STRING"); @@ -566,7 +558,7 @@ public class SecurityMemberAccessTest { @Test public void testBlockedStaticFieldWhenClassIsExcluded() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Arrays.asList(Class.class, StaticTester.class))); + sma.useExcludedClasses(new HashSet<>(Arrays.asList(Class.class.getName(), StaticTester.class.getName()))); // when Member method = StaticTester.class.getField("MAX_VALUE"); @@ -579,7 +571,7 @@ public class SecurityMemberAccessTest { @Test public void testBlockStaticMethodAccess() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = StaticTester.class.getMethod("sayHello"); @@ -592,7 +584,7 @@ public class SecurityMemberAccessTest { @Test public void testBlockAccessIfClassIsExcluded() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -605,7 +597,7 @@ public class SecurityMemberAccessTest { @Test public void testBlockAccessIfClassIsExcluded_2() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(ClassLoader.class.getName()))); // when Member method = ClassLoader.class.getMethod("loadClass", String.class); @@ -619,7 +611,7 @@ public class SecurityMemberAccessTest { @Test public void testAllowAccessIfClassIsNotExcluded() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(ClassLoader.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(ClassLoader.class.getName()))); // when Member method = Class.class.getMethod("getClassLoader"); @@ -632,7 +624,7 @@ public class SecurityMemberAccessTest { @Test public void testIllegalArgumentExceptionExpectedForTargetMemberMismatch() throws Exception { // given - sma.useExcludedClasses(new HashSet<>(Collections.singletonList(Class.class))); + sma.useExcludedClasses(new HashSet<>(singletonList(Class.class.getName()))); // when Member method = ClassLoader.class.getMethod("loadClass", String.class); @@ -669,12 +661,12 @@ public class SecurityMemberAccessTest { sma.useExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("ognl.,javax.")); - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(Object.class); - excluded.add(Runtime.class); - excluded.add(System.class); - excluded.add(Class.class); - excluded.add(ClassLoader.class); + Set<String> excluded = new HashSet<>(); + excluded.add(Object.class.getName()); + excluded.add(Runtime.class.getName()); + excluded.add(System.class.getName()); + excluded.add(Class.class.getName()); + excluded.add(ClassLoader.class.getName()); sma.useExcludedClasses(excluded); String propertyName = "doubleValue"; @@ -737,8 +729,8 @@ public class SecurityMemberAccessTest { @Test public void testAccessMemberAccessIsAccessible() throws Exception { // given - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(ognl.MemberAccess.class); + Set<String> excluded = new HashSet<>(); + excluded.add(ognl.MemberAccess.class.getName()); sma.useExcludedClasses(excluded); String propertyName = "excludedClasses"; @@ -755,8 +747,8 @@ public class SecurityMemberAccessTest { @Test public void testAccessMemberAccessIsBlocked() throws Exception { // given - Set<Class<?>> excluded = new HashSet<>(); - excluded.add(SecurityMemberAccess.class); + Set<String> excluded = new HashSet<>(); + excluded.add(SecurityMemberAccess.class.getName()); sma.useExcludedClasses(excluded); String propertyName = "excludedClasses";