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/29 08:39:51 UTC
[struts] 03/07: WW-5341 Further refactor of OgnlUtil and SecurityMemberAccess to store excluded classes as Strings
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 <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 | 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";