This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5343-sec-extend
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 9640f5bde09d07076b82c6204ac26a6a2e80e897
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Thu Nov 23 00:56:28 2023 +1100
WW-5343 Migrate tests to SecurityMemberAccessTest
---
.../xwork2/ognl/OgnlUtilStrutsTest.java | 27 ---
.../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 247 ---------------------
.../xwork2/ognl/SecurityMemberAccessTest.java | 84 +++++++
3 files changed, 84 insertions(+), 274 deletions(-)
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 0eaf517a4..abaa86144 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilStrutsTest.java
@@ -31,33 +31,6 @@ public class OgnlUtilStrutsTest extends
StrutsInternalTestCase {
ognlUtil = container.getInstance(OgnlUtil.class);
}
- public void testDefaultExcludes() {
- ognlUtil.setExcludedClasses("");
- ognlUtil.setExcludedPackageNames("");
- ognlUtil.setExcludedPackageNamePatterns("");
- assertFalse(ognlUtil.getExcludedClasses().isEmpty());
- assertFalse(ognlUtil.getExcludedPackageNames().isEmpty());
-
- try {
- ognlUtil.getExcludedClasses().clear();
- fail("Missing the expected Exception");
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- }
- try {
- ognlUtil.getExcludedPackageNames().clear();
- fail("Missing the expected Exception");
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- }
- try {
- ognlUtil.getExcludedPackageNamePatterns().clear();
- fail("Missing the expected Exception");
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- }
- }
-
public void testAccessToSizeMethod() throws Exception {
// given
TestArrayBean bean = new TestArrayBean();
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 b1ed266a0..16f1c3850 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -57,15 +57,9 @@ import java.util.Calendar;
import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
-import java.util.Set;
-import java.util.regex.Pattern;
-
-import static org.junit.Assert.assertThrows;
public class OgnlUtilTest extends XWorkTestCase {
@@ -1337,18 +1331,6 @@ public class OgnlUtilTest extends XWorkTestCase {
assertEquals(expected.getMessage(), "It isn't a simple method which
can be called!");
}
- public void testXworkTestCaseOgnlUtilExclusions() {
- internalTestInitialEmptyOgnlUtilExclusions(ognlUtil);
- internalTestOgnlUtilExclusionsImmutable(ognlUtil);
- }
-
- public void testDefaultOgnlUtilExclusions() {
- OgnlUtil basicOgnlUtil = new OgnlUtil();
-
- internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil);
- internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil);
- }
-
public void testDefaultOgnlUtilAlternateConstructorArguments() {
// Code coverage test for the OgnlUtil alternate constructor method,
and verify expected behaviour.
try {
@@ -1365,85 +1347,6 @@ public class OgnlUtilTest extends XWorkTestCase {
}
}
- public void testDefaultOgnlUtilExclusionsAlternateConstructorPopulated() {
- OgnlUtil basicOgnlUtil = new OgnlUtil(new
DefaultOgnlExpressionCacheFactory<>(),
- new DefaultOgnlBeanInfoCacheFactory<>(),
- new StrutsOgnlGuard());
-
- internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil);
- internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil);
- }
-
- public void testOgnlUtilExcludedAdditivity() {
- Set<String> excludedClasses;
- Set<Pattern> excludedPackageNamePatterns;
- Iterator<Pattern> excludedPackageNamePatternsIterator;
- Set<String> excludedPackageNames;
-
- ognlUtil.setExcludedClasses("java.lang.String,java.lang.Integer");
- internalTestOgnlUtilExclusionsImmutable(ognlUtil);
- 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.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.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);
- excludedPackageNamePatterns =
ognlUtil.getExcludedPackageNamePatterns();
- assertNotNull("initial excluded package name patterns null?",
excludedPackageNamePatterns);
- assertEquals("initial excluded package name patterns size not 2 after
adds?", 2, excludedPackageNamePatterns.size());
- excludedPackageNamePatternsIterator =
excludedPackageNamePatterns.iterator();
- Set<String> patternStrings = new HashSet<>();
- while (excludedPackageNamePatternsIterator.hasNext()) {
- Pattern pattern = excludedPackageNamePatternsIterator.next();
- patternStrings.add(pattern.pattern());
- }
- assertTrue("fakepackage1.* not in exclusions?",
patternStrings.contains("fakepackage1.*"));
- assertTrue("fakepackage2.* not in exclusions?",
patternStrings.contains("fakepackage2.*"));
-
ognlUtil.setExcludedPackageNamePatterns("fakepackage3.*,fakepackage4.*");
- internalTestOgnlUtilExclusionsImmutable(ognlUtil);
- excludedPackageNamePatterns =
ognlUtil.getExcludedPackageNamePatterns();
- assertNotNull("updated excluded package name patterns null?",
excludedPackageNamePatterns);
- assertEquals("updated excluded package name patterns size not 4 after
adds?", 4, excludedPackageNamePatterns.size());
- excludedPackageNamePatternsIterator =
excludedPackageNamePatterns.iterator();
- patternStrings = new HashSet<>();
- while (excludedPackageNamePatternsIterator.hasNext()) {
- Pattern pattern = excludedPackageNamePatternsIterator.next();
- patternStrings.add(pattern.pattern());
- }
- assertTrue("fakepackage1.* not in exclusions?",
patternStrings.contains("fakepackage1.*"));
- assertTrue("fakepackage2.* not in exclusions?",
patternStrings.contains("fakepackage2.*"));
- assertTrue("fakepackage3.* not in exclusions?",
patternStrings.contains("fakepackage3.*"));
- assertTrue("fakepackage4.* not in exclusions?",
patternStrings.contains("fakepackage4.*"));
-
-
ognlUtil.setExcludedPackageNames("fakepackage1.package,fakepackage2.package");
- internalTestOgnlUtilExclusionsImmutable(ognlUtil);
- excludedPackageNames = ognlUtil.getExcludedPackageNames();
- assertNotNull("initial exluded package names null?",
excludedPackageNames);
- assertEquals("initial exluded package names not 2 after adds?", 2,
excludedPackageNames.size());
- assertTrue("fakepackage1.package not in exclusions?",
excludedPackageNames.contains("fakepackage1.package"));
- assertTrue("fakepackage2.package not in exclusions?",
excludedPackageNames.contains("fakepackage2.package"));
-
ognlUtil.setExcludedPackageNames("fakepackage3.package,fakepackage4.package");
- internalTestOgnlUtilExclusionsImmutable(ognlUtil);
- excludedPackageNames = ognlUtil.getExcludedPackageNames();
- assertNotNull("updated exluded package names null?",
excludedPackageNames);
- assertEquals("updated exluded package names not 4 after adds?", 4,
excludedPackageNames.size());
- assertTrue("fakepackage1.package not in exclusions?",
excludedPackageNames.contains("fakepackage1.package"));
- assertTrue("fakepackage2.package not in exclusions?",
excludedPackageNames.contains("fakepackage2.package"));
- assertTrue("fakepackage3.package not in exclusions?",
excludedPackageNames.contains("fakepackage3.package"));
- assertTrue("fakepackage4.package not in exclusions?",
excludedPackageNames.contains("fakepackage4.package"));
- }
-
/**
* Ensure getValue:
* 1) When allowStaticFieldAccess true - Permits public static field
access,
@@ -1625,55 +1528,6 @@ public class OgnlUtilTest extends XWorkTestCase {
}
}
- private void internalTestInitialEmptyOgnlUtilExclusions(OgnlUtil
ognlUtilParam) {
- Set<String> excludedClasses = ognlUtilParam.getExcludedClasses();
- assertNotNull("parameter (default) exluded classes null?",
excludedClasses);
- assertTrue("parameter (default) exluded classes not empty?",
excludedClasses.isEmpty());
-
- Set<Pattern> excludedPackageNamePatterns =
ognlUtilParam.getExcludedPackageNamePatterns();
- assertNotNull("parameter (default) exluded package name patterns
null?", excludedPackageNamePatterns);
- assertTrue("parameter (default) exluded package name patterns not
empty?", excludedPackageNamePatterns.isEmpty());
-
- Set<String> excludedPackageNames =
ognlUtilParam.getExcludedPackageNames();
- assertNotNull("parameter (default) exluded package names null?",
excludedPackageNames);
- assertTrue("parameter (default) exluded package names not empty?",
excludedPackageNames.isEmpty());
- }
-
- private void internalTestOgnlUtilExclusionsImmutable(OgnlUtil
ognlUtilParam) {
- Pattern somePattern = Pattern.compile("SomeRegexPattern");
- assertImmutability(ognlUtilParam.getExcludedClasses(),
Integer.class.getName());
- assertImmutability(ognlUtilParam.getExcludedPackageNamePatterns(),
somePattern);
- assertImmutability(ognlUtilParam.getExcludedPackageNames(),
"somepackagename");
- assertImmutability(ognlUtilParam.getExcludedPackageExemptClasses(),
Integer.class.getName());
- }
-
- public static <T> void assertImmutability(Collection<T> collection, T
item) {
- assertNotNull("Collection is null", collection);
- try {
- if (!collection.isEmpty()) {
- collection.clear();
- fail("Collection could be cleared");
- }
- } catch (UnsupportedOperationException uoe) {
- // Expected failure
- }
- try {
- collection.add(item);
- fail("Collection could be added to");
- } catch (UnsupportedOperationException uoe) {
- // Expected failure
- }
- try {
- int prevSize = collection.size();
- collection.remove(item);
- if (prevSize != collection.size()) {
- fail("Collection could be removed from");
- }
- } catch (UnsupportedOperationException uoe) {
- // Expected failure
- }
- }
-
public void testAccessContext() throws Exception {
Map<String, Object> context = ognlUtil.createDefaultContext(null);
@@ -1692,107 +1546,6 @@ public class OgnlUtilTest extends XWorkTestCase {
assertSame(that, root);
}
- public void testSetExcludedPackageNames() {
- assertThrows(ConfigurationException.class, () ->
ognlUtil.setExcludedPackageNames("java.lang\njava.awt"));
- assertThrows(ConfigurationException.class, () ->
ognlUtil.setExcludedPackageNames("java.lang\tjava.awt"));
- ConfigurationException e = assertThrows(ConfigurationException.class,
() -> ognlUtil.setExcludedPackageNames("java.lang java.awt"));
- assertTrue(e.getMessage().contains("erroneous whitespace characters"));
- }
-
- public void testGetExcludedPackageNames() {
- // Getter should return an immutable collection
- OgnlUtil util = new OgnlUtil();
- util.setExcludedPackageNames("java.lang,java.awt");
- assertEquals(util.getExcludedPackageNames().size(), 2);
- try {
- util.getExcludedPackageNames().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedPackageNames().size(), 2);
- }
- }
-
- public void testGetExcludedPackageNamesAlternateConstructorPopulated() {
- // Getter should return an immutable collection
- OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(),
- new DefaultOgnlBeanInfoCacheFactory<>(),
- new StrutsOgnlGuard());
- util.setExcludedPackageNames("java.lang,java.awt");
- assertEquals(util.getExcludedPackageNames().size(), 2);
- try {
- util.getExcludedPackageNames().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedPackageNames().size(), 2);
- }
- }
-
- public void testGetExcludedClasses() {
- // Getter should return an immutable collection
- OgnlUtil util = new OgnlUtil();
-
util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL");
- assertEquals(util.getExcludedClasses().size(), 3);
- try {
- util.getExcludedClasses().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedClasses().size(), 3);
- }
- }
-
- public void testGetExcludedClassesAlternateConstructorPopulated() {
- // Getter should return an immutable collection
- OgnlUtil util = new OgnlUtil(new DefaultOgnlExpressionCacheFactory<>(),
- new DefaultOgnlBeanInfoCacheFactory<>(),
- new StrutsOgnlGuard());
-
util.setExcludedClasses("java.lang.Runtime,java.lang.ProcessBuilder,java.net.URL");
- assertEquals(util.getExcludedClasses().size(), 3);
- try {
- util.getExcludedClasses().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedClasses().size(), 3);
- }
- }
-
- public void testGetExcludedPackageNamePatterns() {
- // Getter should return an immutable collection
- OgnlUtil util = new OgnlUtil();
- util.setExcludedPackageNamePatterns("java.lang.");
- assertEquals(util.getExcludedPackageNamePatterns().size(), 1);
- try {
- util.getExcludedPackageNamePatterns().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedPackageNamePatterns().size(), 1);
- }
- }
-
- 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<>(),
- new StrutsOgnlGuard());
- util.setExcludedPackageNamePatterns("java.lang.");
- assertEquals(util.getExcludedPackageNamePatterns().size(), 1);
- try {
- util.getExcludedPackageNamePatterns().clear();
- } catch (Exception ex) {
- assertTrue(ex instanceof UnsupportedOperationException);
- } finally {
- assertEquals(util.getExcludedPackageNamePatterns().size(), 1);
- }
- }
-
public void testOgnlUtilDefaultCacheClass() throws OgnlException {
OgnlDefaultCache<Integer, String> defaultCache = new
OgnlDefaultCache<>(2, 16, 0.75f);
assertEquals("Initial evictionLimit did not match initial value", 2,
defaultCache.getEvictionLimit());
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 70165fcde..ebbc797ee 100644
---
a/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
+++
b/core/src/test/java/com/opensymphony/xwork2/ognl/SecurityMemberAccessTest.java
@@ -19,21 +19,29 @@
package com.opensymphony.xwork2.ognl;
import com.opensymphony.xwork2.TestBean;
+import com.opensymphony.xwork2.config.ConfigurationException;
import com.opensymphony.xwork2.test.TestBean2;
+import com.opensymphony.xwork2.util.Foo;
import ognl.MemberAccess;
+import org.apache.commons.lang3.reflect.FieldUtils;
import org.junit.Before;
import org.junit.Test;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -54,6 +62,82 @@ public class SecurityMemberAccessTest {
sma = new SecurityMemberAccess(allowStaticFieldAccess);
}
+ private <T> T reflectField(String fieldName) throws IllegalAccessException
{
+ return (T) FieldUtils.readField(sma, fieldName, true);
+ }
+
+ @Test
+ public void defaultExclusionList() throws Exception {
+ Set<String> excludedClasses = reflectField("excludedClasses");
+ assertThat(excludedClasses).containsExactly(Object.class.getName());
+
+ assignNewSma(false);
+ excludedClasses = reflectField("excludedClasses");
+
assertThat(excludedClasses).containsExactlyInAnyOrder(Object.class.getName(),
Class.class.getName());
+ }
+
+ @Test
+ public void configurationCollectionsImmutable() throws Exception {
+ List<String> fields = Arrays.asList(
+ "excludedClasses",
+ "excludedPackageNames",
+ "excludedPackageNamePatterns",
+ "excludedPackageExemptClasses",
+ "allowlistClasses",
+ "allowlistPackageNames",
+ "excludeProperties",
+ "acceptProperties");
+ for (String field : fields) {
+ Collection<String> fieldVal = reflectField(field);
+ assertThrows(UnsupportedOperationException.class, () ->
fieldVal.add("foo"));
+ if (!fieldVal.isEmpty()) {
+ assertThrows(UnsupportedOperationException.class, () ->
fieldVal.remove(fieldVal.iterator().next()));
+ assertThrows(UnsupportedOperationException.class,
fieldVal::clear);
+ }
+ }
+ }
+
+ @Test
+ public void exclusionListsAreAdditive_classes() throws Exception {
+ Collection<String> fieldVal = reflectField("excludedClasses");
+ Set<String> existing = new HashSet<>(fieldVal);
+
+ Collection<String> newExcludedClasses =
Arrays.asList(FooBar.class.getName(), String.class.getName());
+ sma.useExcludedClasses(String.join(",", newExcludedClasses));
+ existing.addAll(newExcludedClasses);
+
+ fieldVal = reflectField("excludedClasses");
+ assertThat(fieldVal).containsExactlyInAnyOrderElementsOf(existing);
+ }
+
+ @Test
+ public void exclusionListsAreAdditive_packages() throws Exception {
+ sma.useExcludedPackageNames(Foo.class.getPackage().getName());
+ Collection<String> fieldVal = reflectField("excludedPackageNames");
+ Set<String> existing = new HashSet<>(fieldVal);
+
+ Collection<String> newExcludedPackages =
Arrays.asList(FooBar.class.getPackage().getName(),
String.class.getPackage().getName());
+ sma.useExcludedPackageNames(String.join(",", newExcludedPackages));
+ existing.addAll(newExcludedPackages);
+
+ fieldVal = reflectField("excludedPackageNames");
+ assertThat(fieldVal).containsExactlyInAnyOrderElementsOf(existing);
+ }
+
+ @Test
+ public void useExcludedPackageNames() {
+ assertThrows(ConfigurationException.class, () ->
sma.useExcludedPackageNames("java.lang\njava.awt"));
+ assertThrows(ConfigurationException.class, () ->
sma.useExcludedPackageNames("java.lang\tjava.awt"));
+ ConfigurationException e = assertThrows(ConfigurationException.class,
() -> sma.useExcludedPackageNames("java.lang java.awt"));
+ assertTrue(e.getMessage().contains("erroneous whitespace characters"));
+ }
+
+ @Test
+ public void useExcludedPackagePatterns() {
+ ConfigurationException e = assertThrows(ConfigurationException.class,
() -> sma.useExcludedPackageNamePatterns("["));
+ assertTrue(e.getMessage().contains("invalid regex"));
+ }
+
@Test
public void testWithoutClassExclusion() throws Exception {
// given
