You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by ku...@apache.org on 2023/08/30 02:36:25 UTC

[struts] branch WW-5341-classloaders updated (ace4036fa -> 7c1c674a0)

This is an automated email from the ASF dual-hosted git repository.

kusal pushed a change to branch WW-5341-classloaders
in repository https://gitbox.apache.org/repos/asf/struts.git


 discard ace4036fa WW-5341 Add unit test for excluded pattern validation
 discard dfb33b628 WW-5341 Clean up OgnlUtilTest
 discard 71fa91d82 WW-5341 Split package exclusion check
 discard 358798dc8 WW-5341 Move proxy check to be first
 discard d8e688ba6 WW-5341 Further refactor of OgnlUtil and SecurityMemberAccess to store excluded classes as Strings
     new c57015749 WW-5341 Further refactor of OgnlUtil and SecurityMemberAccess to store excluded classes as Strings
     new a9666272e WW-5341 Move proxy check to be first
     new 1bbcc17c7 WW-5341 Split package exclusion check
     new a352132f0 WW-5341 Clean up OgnlUtilTest
     new 7c1c674a0 WW-5341 Add unit test for excluded pattern validation

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (ace4036fa)
            \
             N -- N -- N   refs/heads/WW-5341-classloaders (7c1c674a0)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


[struts] 02/05: WW-5341 Move proxy check to be first

Posted by ku...@apache.org.
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 a9666272eb270ca836e537c90252d4e1f232ecb1
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Tue Aug 29 09:21:01 2023 +1000

    WW-5341 Move proxy check to be first
---
 .../main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java | 5 +++++
 1 file changed, 5 insertions(+)

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 f65c322b8..e75805d71 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -111,6 +111,11 @@ public class SecurityMemberAccess implements MemberAccess {
             throw new IllegalArgumentException("Target does not match member!");
         }
 
+        if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
+            LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+            return false;
+        }
+
         if (!checkPublicMemberAccess(memberModifiers)) {
             LOG.warn("Access to non-public [{}] is blocked!", member);
             return false;


[struts] 03/05: WW-5341 Split package exclusion check

Posted by ku...@apache.org.
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 1bbcc17c747210eaea27a0e9182971b7bca075d0
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Tue Aug 29 09:21:30 2023 +1000

    WW-5341 Split package exclusion check
---
 .../xwork2/ognl/SecurityMemberAccess.java          | 28 ++++++----------------
 .../xwork2/ognl/SecurityMemberAccessTest.java      |  6 ++---
 .../com/test/ExternalSecurityMemberAccess.java     |  4 ++--
 3 files changed, 12 insertions(+), 26 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 e75805d71..3e1c69d5d 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -152,19 +152,16 @@ public class SecurityMemberAccess implements MemberAccess {
             LOG.warn("The use of the default (unnamed) package is discouraged!");
         }
 
-        if (isPackageExcluded(targetClass, memberClass)) {
-            LOG.warn(
-                    "Package [{}] of target class [{}] of target [{}] or package [{}] of member [{}] are excluded!",
+        if (isPackageExcluded(targetClass)) {
+            LOG.warn("Package [{}] of target class [{}] of target [{}] is excluded!",
                     targetClass.getPackage(),
                     targetClass,
-                    target,
-                    memberClass.getPackage(),
-                    member);
+                    target);
             return false;
         }
 
-        if (disallowProxyMemberAccess && ProxyUtil.isProxyMember(member, target)) {
-            LOG.warn("Access to proxy is blocked! Target class [{}] of target [{}], member [{}]", targetClass, target, member);
+        if (isPackageExcluded(memberClass)) {
+            LOG.warn("Package [{}] of member [{}] are excluded!", memberClass.getPackage(), member);
             return false;
         }
 
@@ -222,19 +219,8 @@ public class SecurityMemberAccess implements MemberAccess {
         return false;
     }
 
-    protected boolean isPackageExcluded(Class<?> targetClass, Class<?> memberClass) {
-        if (targetClass == null || memberClass == null) {
-            throw new IllegalArgumentException(
-                    "Parameters should never be null - if member is static, targetClass should be the same as memberClass.");
-        }
-
-        List<Class<?>> classesToCheck = Arrays.asList(targetClass, memberClass);
-        for (Class<?> clazz : classesToCheck) {
-            if (!excludedPackageExemptClasses.contains(clazz.getName()) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz))) {
-                return true;
-            }
-        }
-        return false;
+    protected boolean isPackageExcluded(Class<?> clazz) {
+        return !excludedPackageExemptClasses.contains(clazz.getName()) && (isExcludedPackageNames(clazz) || isExcludedPackageNamePatterns(clazz));
     }
 
     public static String toPackageName(Class<?> clazz) {
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 b7b1bce09..15a9885c9 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -309,7 +309,7 @@ public class SecurityMemberAccessTest {
         Class<?> clazz = Class.forName("PackagelessAction");
 
         // when
-        boolean actual = sma.isPackageExcluded(clazz, clazz);
+        boolean actual = sma.isPackageExcluded(clazz);
 
         // then
         assertFalse("default package is excluded!", actual);
@@ -325,7 +325,7 @@ public class SecurityMemberAccessTest {
         Class<?> clazz = Class.forName("PackagelessAction");
 
         // when
-        boolean actual = sma.isPackageExcluded(clazz, clazz);
+        boolean actual = sma.isPackageExcluded(clazz);
 
         // then
         assertTrue("default package isn't excluded!", actual);
@@ -768,7 +768,7 @@ public class SecurityMemberAccessTest {
         sma.useExcludedPackageNames(TextParseUtil.commaDelimitedStringToSet("java.lang"));
 
         // when
-        boolean actual = sma.isPackageExcluded(String.class, String.class);
+        boolean actual = sma.isPackageExcluded(String.class);
 
         // then
         assertTrue("package java.lang. is accessible!", actual);
diff --git a/core/src/test/java/com/test/ExternalSecurityMemberAccess.java b/core/src/test/java/com/test/ExternalSecurityMemberAccess.java
index 41183fa39..a53f1736b 100644
--- a/core/src/test/java/com/test/ExternalSecurityMemberAccess.java
+++ b/core/src/test/java/com/test/ExternalSecurityMemberAccess.java
@@ -27,7 +27,7 @@ class ExternalSecurityMemberAccess extends SecurityMemberAccess {
     }
 
     @Override
-    public boolean isPackageExcluded(Class<?> targetClass, Class<?> memberClass) {
-        return super.isPackageExcluded(targetClass, memberClass);
+    public boolean isPackageExcluded(Class<?> clazz) {
+        return super.isPackageExcluded(clazz);
     }
 }


[struts] 01/05: WW-5341 Further refactor of OgnlUtil and SecurityMemberAccess to store excluded classes as Strings

Posted by ku...@apache.org.
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 c57015749dd6af2a00693edef4395bb1cb279c9e
Author: Kusal Kithul-Godage <gi...@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     | 128 ++++++++++-----------
 .../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, 146 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..bbfdd72bb 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -25,6 +25,7 @@ import com.opensymphony.xwork2.inject.Inject;
 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
 import com.opensymphony.xwork2.util.CompoundRoot;
 import com.opensymphony.xwork2.util.reflection.ReflectionException;
+import jdk.internal.reflect.Reflection;
 import ognl.ClassResolver;
 import ognl.Ognl;
 import ognl.OgnlContext;
@@ -43,7 +44,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 +53,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 +80,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 +128,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 +178,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) throws ConfigurationException {
+        Set<String> classNames = commaDelimitedStringToSet(newDelimitedClasses);
+        validateClasses(classNames, Reflection.getCallerClass().getClassLoader());
+        Set<String> excludedClasses = new HashSet<>(oldClasses);
+        excludedClasses.addAll(classNames);
+        return unmodifiableSet(excludedClasses);
+    }
+
+    private static void validateClasses(Set<String> classNames, ClassLoader validatingClassLoader) throws ConfigurationException {
         for (String className : classNames) {
             try {
-                classes.add(Class.forName(className));
+                validatingClassLoader.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 +270,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";


[struts] 04/05: WW-5341 Clean up OgnlUtilTest

Posted by ku...@apache.org.
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 a352132f00baf771c25451281758f6764c277787
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Tue Aug 29 09:36:12 2023 +1000

    WW-5341 Clean up OgnlUtilTest
---
 .../com/opensymphony/xwork2/ognl/OgnlUtil.java     |  3 +-
 .../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 65 +++++++++++-----------
 2 files changed, 35 insertions(+), 33 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 bbfdd72bb..6b90bd674 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -102,8 +102,7 @@ public class OgnlUtil {
     @Deprecated
     public OgnlUtil() {
         // Instantiate default Expression and BeanInfo caches (factories must be non-null).
-        this(new DefaultOgnlExpressionCacheFactory<>(),
-                new DefaultOgnlBeanInfoCacheFactory<>());
+        this(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>());
     }
 
     /**
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 736889886..3dbcbdafd 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -238,7 +238,9 @@ public class OgnlUtilTest extends XWorkTestCase {
         // Method expression which executes with success
         try {
             ognlUtil.callMethod("getBar()", context, foo);
-            assertTrue("Successfully executed method expression must have been cached", ognlUtil.expressionCacheSize() == 1);
+            assertEquals("Successfully executed method expression must have been cached",
+                    1,
+                    ognlUtil.expressionCacheSize());
         } catch (Exception ex) {
             fail("Method expression execution should have succeeded here. Exception: " + ex);
         }
@@ -541,7 +543,7 @@ public class OgnlUtilTest extends XWorkTestCase {
         assertEquals(b1.getSomethingElse(), b2.getSomethingElse());
 
         // id properties did not
-        assertEquals(b2.getId(), new Long(2));
+        assertEquals(b2.getId(), Long.valueOf(2));
 
     }
 
@@ -959,8 +961,8 @@ public class OgnlUtilTest extends XWorkTestCase {
         assertNotNull(beans);
         assertEquals(22, beans.size());
         assertEquals("Hello Santa", beans.get("title"));
-        assertEquals(new Long("123"), beans.get("ALong"));
-        assertEquals(new Integer("44"), beans.get("number"));
+        assertEquals(Long.valueOf("123"), beans.get("ALong"));
+        assertEquals(Integer.valueOf("44"), beans.get("number"));
         assertEquals(bar, beans.get("bar"));
         assertEquals(Boolean.TRUE, beans.get("useful"));
     }
@@ -971,7 +973,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Map<String, Object> beans = ognlUtil.getBeanMap(bar);
         assertEquals(2, beans.size());
-        assertEquals(new Integer("1"), beans.get("id"));
+        assertEquals(Integer.valueOf("1"), beans.get("id"));
         assertEquals("There is no read method for bar", beans.get("bar"));
     }
 
@@ -1345,13 +1347,13 @@ public class OgnlUtilTest extends XWorkTestCase {
     public void testDefaultOgnlUtilAlternateConstructorArguments() {
         // Code coverage test for the OgnlUtil alternate constructor method, and verify expected behaviour.
         try {
-            OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<String, Object>(), null);
+            OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), null);
             fail("null beanInfoCacheFactory should result in exception");
         } catch (IllegalArgumentException iaex) {
             // expected result
         }
         try {
-            OgnlUtil basicOgnlUtil = new OgnlUtil(null, new DefaultOgnlBeanInfoCacheFactory<Class<?>, BeanInfo>());
+            OgnlUtil basicOgnlUtil = new OgnlUtil(null, new DefaultOgnlBeanInfoCacheFactory<>());
             fail("null expressionCacheFactory should result in exception");
         } catch (IllegalArgumentException iaex) {
             // expected result
@@ -1359,7 +1361,8 @@ public class OgnlUtilTest extends XWorkTestCase {
     }
 
     public void testDefaultOgnlUtilExclusionsAlternateConstructorPopulated() {
-        OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<String, Object>(), new DefaultOgnlBeanInfoCacheFactory<Class<?>, BeanInfo>());
+        OgnlUtil basicOgnlUtil = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(),
+                new DefaultOgnlBeanInfoCacheFactory<>());
 
         internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil);
         internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil);
@@ -1734,7 +1737,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
     public void testGetExcludedClassesAlternateConstructorPopulated() {
         // Getter should return an immutable collection
-        OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<String, Object>(), new DefaultOgnlBeanInfoCacheFactory<Class<?>, BeanInfo>());
+        OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>());
         util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL");
         assertEquals(util.getExcludedClasses().size(), 3);
         try {
@@ -1762,7 +1765,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
     public void testGetExcludedPackageNamePatternsAlternateConstructorPopulated() {
         // Getter should return an immutable collection
-        OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<String, Object>(), new DefaultOgnlBeanInfoCacheFactory<Class<?>, BeanInfo>());
+        OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>());
         util.setExcludedPackageNamePatterns("java.lang.");
         assertEquals(util.getExcludedPackageNamePatterns().size(), 1);
         try {
@@ -1779,24 +1782,24 @@ public class OgnlUtilTest extends XWorkTestCase {
         assertEquals("Initial evictionLimit did not match initial value", 2, defaultCache.getEvictionLimit());
         defaultCache.setEvictionLimit(3);
         assertEquals("Updated evictionLimit did not match updated value", 3, defaultCache.getEvictionLimit());
-        String lookupResult = defaultCache.get(Integer.valueOf(0));
+        String lookupResult = defaultCache.get(0);
         assertNull("Lookup of empty cache returned non-null value ?", lookupResult);
-        defaultCache.put(Integer.valueOf(0), "Zero");
-        lookupResult = defaultCache.get(Integer.valueOf(0));
+        defaultCache.put(0, "Zero");
+        lookupResult = defaultCache.get(0);
         assertEquals("Retrieved value does not match put value ?", "Zero", lookupResult);
-        defaultCache.put(Integer.valueOf(1), "One");
-        defaultCache.put(Integer.valueOf(2), "Two");
+        defaultCache.put(1, "One");
+        defaultCache.put(2, "Two");
         assertEquals("Default cache not size evictionlimit after adding three values ?", defaultCache.getEvictionLimit(), defaultCache.size());
-        lookupResult = defaultCache.get(Integer.valueOf(2));
+        lookupResult = defaultCache.get(2);
         assertEquals("Retrieved value does not match put value ?", "Two", lookupResult);
-        defaultCache.put(Integer.valueOf(3), "Three");
+        defaultCache.put(3, "Three");
         assertEquals("Default cache not size zero after an add that exceeded the evection limit ?", 0, defaultCache.size());
-        lookupResult = defaultCache.get(Integer.valueOf(0));
+        lookupResult = defaultCache.get(0);
         assertNull("Lookup of value 0 (should have been evicted with everything) returned non-null value ?", lookupResult);
-        lookupResult = defaultCache.get(Integer.valueOf(3));
+        lookupResult = defaultCache.get(3);
         assertNull("Lookup of value 3 (should have been evicted with everything) returned non-null value ?", lookupResult);
-        defaultCache.putIfAbsent(Integer.valueOf(2), "Two");
-        lookupResult = defaultCache.get(Integer.valueOf(2));
+        defaultCache.putIfAbsent(2, "Two");
+        lookupResult = defaultCache.get(2);
         assertEquals("Retrieved value does not match put value ?", "Two", lookupResult);
         defaultCache.clear();
         assertEquals("Default cache not empty after clear ?", 0, defaultCache.size());
@@ -1807,22 +1810,22 @@ public class OgnlUtilTest extends XWorkTestCase {
         assertEquals("Initial evictionLimit did not match initial value", 2, lruCache.getEvictionLimit());
         lruCache.setEvictionLimit(3);
         assertEquals("Updated evictionLimit did not match updated value", 3, lruCache.getEvictionLimit());
-        String lookupResult = lruCache.get(Integer.valueOf(0));
+        String lookupResult = lruCache.get(0);
         assertNull("Lookup of empty cache returned non-null value ?", lookupResult);
-        lruCache.put(Integer.valueOf(0), "Zero");
-        lookupResult = lruCache.get(Integer.valueOf(0));
+        lruCache.put(0, "Zero");
+        lookupResult = lruCache.get(0);
         assertEquals("Retrieved value does not match put value ?", "Zero", lookupResult);
-        lruCache.put(Integer.valueOf(1), "One");
-        lruCache.put(Integer.valueOf(2), "Two");
+        lruCache.put(1, "One");
+        lruCache.put(2, "Two");
         assertEquals("LRU cache not size evictionlimit after adding three values ?", lruCache.getEvictionLimit(), lruCache.size());
-        lookupResult = lruCache.get(Integer.valueOf(2));
+        lookupResult = lruCache.get(2);
         assertEquals("Retrieved value does not match put value ?", "Two", lookupResult);
-        lruCache.put(Integer.valueOf(3), "Three");
+        lruCache.put(3, "Three");
         assertEquals("LRU cache not size evictionlimit after adding  values ?", lruCache.getEvictionLimit(), lruCache.size());
-        lookupResult = lruCache.get(Integer.valueOf(0));
+        lookupResult = lruCache.get(0);
         assertNull("Lookup of value 0 (should have dropped off LRU cache) returned non-null value ?", lookupResult);
-        lruCache.putIfAbsent(Integer.valueOf(2), "Two");
-        lookupResult = lruCache.get(Integer.valueOf(2));
+        lruCache.putIfAbsent(2, "Two");
+        lookupResult = lruCache.get(2);
         assertEquals("Retrieved value does not match put value ?", "Two", lookupResult);
         lruCache.clear();
         assertEquals("LRU cache not empty after clear ?", 0, lruCache.size());


[struts] 05/05: WW-5341 Add unit test for excluded pattern validation

Posted by ku...@apache.org.
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 7c1c674a023d30fcb50c8ff6e57455641208ba94
Author: Kusal Kithul-Godage <gi...@kusal.io>
AuthorDate: Tue Aug 29 09:44:50 2023 +1000

    WW-5341 Add unit test for excluded pattern validation
---
 core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 4 ++++
 1 file changed, 4 insertions(+)

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 3dbcbdafd..8694d9721 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -1763,6 +1763,10 @@ public class OgnlUtilTest extends XWorkTestCase {
         }
     }
 
+    public void testInvalidExcludedPackageNamePatterns() {
+        assertThrows(ConfigurationException.class, () -> ognlUtil.setExcludedPackageNamePatterns("["));
+    }
+
     public void testGetExcludedPackageNamePatternsAlternateConstructorPopulated() {
         // Getter should return an immutable collection
         OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(), new DefaultOgnlBeanInfoCacheFactory<>());