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";


Reply via email to