Minor cleanup/consistency changes for 3 modules.

- Made a private ConcurrentMap reference final, made initial sets immutable (consistency).
- Made sets for Accepted and Excluded patterns checkers immutable in 2 modules (consistency).
- Added @OverRide annotations missing from a few methods in 2 modules.
- Updated the 3 relevant unit tests to verify immutable states of various sets.

(cherry picked from commit 881e1b2)

# Conflicts:
#	core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java

@@ -53,7 +53,7 @@ public class OgnlUtil {
 
     private static final Logger LOG = LogManager.getLogger(OgnlUtil.class);
 
-    private ConcurrentMap<String, Object> expressions = new ConcurrentHashMap<>();
+    private final ConcurrentMap<String, Object> expressions = new ConcurrentHashMap<>();
     private final ConcurrentMap<Class, BeanInfo> beanInfoCache = new ConcurrentHashMap<>();
     private TypeConverter defaultConverter;
 
@@ -73,6 +73,9 @@ public OgnlUtil() {
         excludedClasses = new HashSet<>();
         excludedPackageNamePatterns = new HashSet<>();
         excludedPackageNames = new HashSet<>();
+        excludedClasses = Collections.unmodifiableSet(excludedClasses);
+        excludedPackageNamePatterns = Collections.unmodifiableSet(excludedPackageNamePatterns);
+        excludedPackageNames = Collections.unmodifiableSet(excludedPackageNames);
     }
 
     @Inject

core/src/main/java/com/opensymphony/xwork2/security/DefaultAcceptedPatternsChecker.java

@@ -25,6 +25,7 @@
 import org.apache.struts2.StrutsConstants;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -48,27 +49,39 @@ protected void setOverrideAcceptedPatterns(String acceptablePatterns) {
         LOG.warn("Overriding accepted patterns [{}] with [{}], be aware that this affects all instances and safety of your application!",
                     acceptedPatterns, acceptablePatterns);
         acceptedPatterns = new HashSet<>();
-        for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) {
-            acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        try {
+            for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) {
+                acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            acceptedPatterns = Collections.unmodifiableSet(acceptedPatterns);
         }
     }
 
     @Inject(value = StrutsConstants.STRUTS_ADDITIONAL_ACCEPTED_PATTERNS, required = false)
     protected void setAdditionalAcceptedPatterns(String acceptablePatterns) {
         LOG.warn("Adding additional global patterns [{}] to accepted patterns!", acceptablePatterns);
-        for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) {
-            acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        acceptedPatterns = new HashSet<>(acceptedPatterns);  // Make mutable before adding
+        try {
+            for (String pattern : TextParseUtil.commaDelimitedStringToSet(acceptablePatterns)) {
+                acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            acceptedPatterns = Collections.unmodifiableSet(acceptedPatterns);
         }
     }
 
+    @Override
     public void setAcceptedPatterns(String commaDelimitedPatterns) {
         setAcceptedPatterns(TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns));
     }
 
+    @Override
     public void setAcceptedPatterns(String[] additionalPatterns) {
         setAcceptedPatterns(new HashSet<>(Arrays.asList(additionalPatterns)));
     }
 
+    @Override
     public void setAcceptedPatterns(Set<String> patterns) {
         if (acceptedPatterns == null) {
             // Limit unwanted log entries (for 1st call, acceptedPatterns null)
@@ -78,11 +91,16 @@ public void setAcceptedPatterns(Set<String> patterns) {
                         acceptedPatterns, patterns);
         }
         acceptedPatterns = new HashSet<>(patterns.size());
-        for (String pattern : patterns) {
-            acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        try {
+            for (String pattern : patterns) {
+                acceptedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            acceptedPatterns = Collections.unmodifiableSet(acceptedPatterns);
         }
     }
 
+    @Override
     public IsAccepted isAccepted(String value) {
         for (Pattern acceptedPattern : acceptedPatterns) {
             if (acceptedPattern.matcher(value).matches()) {
@@ -93,6 +111,7 @@ public IsAccepted isAccepted(String value) {
         return IsAccepted.no(acceptedPatterns.toString());
     }
 
+    @Override
     public Set<Pattern> getAcceptedPatterns() {
         return acceptedPatterns;
     }

core/src/main/java/com/opensymphony/xwork2/security/DefaultExcludedPatternsChecker.java

@@ -26,6 +26,7 @@
 import org.apache.struts2.StrutsConstants;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.regex.Pattern;
@@ -54,17 +55,26 @@ protected void setOverrideExcludePatterns(String excludePatterns) {
             // Limit unwanted log entries (when excludedPatterns null/empty - usually 1st call)
             LOG.debug("Overriding excluded patterns with [{}]", excludePatterns);
         }
-        excludedPatterns = new HashSet<Pattern>();
-        for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
-            excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        excludedPatterns = new HashSet<>();
+        try {
+            for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
+                excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            excludedPatterns = Collections.unmodifiableSet(excludedPatterns);
         }
     }
 
     @Inject(value = StrutsConstants.STRUTS_ADDITIONAL_EXCLUDED_PATTERNS, required = false)
     public void setAdditionalExcludePatterns(String excludePatterns) {
         LOG.debug("Adding additional global patterns [{}] to excluded patterns!", excludePatterns);
-        for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
-            excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        excludedPatterns = new HashSet<>(excludedPatterns);  // Make mutable before adding
+        try {
+            for (String pattern : TextParseUtil.commaDelimitedStringToSet(excludePatterns)) {
+                excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            excludedPatterns = Collections.unmodifiableSet(excludedPatterns);
         }
     }
 
@@ -76,14 +86,17 @@ protected void setDynamicMethodInvocation(String dmiValue) {
         }
     }
 
+    @Override
     public void setExcludedPatterns(String commaDelimitedPatterns) {
         setExcludedPatterns(TextParseUtil.commaDelimitedStringToSet(commaDelimitedPatterns));
     }
 
+    @Override
     public void setExcludedPatterns(String[] patterns) {
         setExcludedPatterns(new HashSet<>(Arrays.asList(patterns)));
     }
 
+    @Override
     public void setExcludedPatterns(Set<String> patterns) {
         if (excludedPatterns != null && excludedPatterns.size() > 0) {
             LOG.warn("Replacing excluded patterns [{}] with [{}], be aware that this affects all instances and safety of your application!",
@@ -93,11 +106,16 @@ public void setExcludedPatterns(Set<String> patterns) {
             LOG.debug("Sets excluded patterns to [{}]", patterns);
         }
         excludedPatterns = new HashSet<>(patterns.size());
-        for (String pattern : patterns) {
-            excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+        try {
+            for (String pattern : patterns) {
+                excludedPatterns.add(Pattern.compile(pattern, Pattern.CASE_INSENSITIVE));
+            }
+        } finally {
+            excludedPatterns = Collections.unmodifiableSet(excludedPatterns);
         }
     }
 
+    @Override
     public IsExcluded isExcluded(String value) {
         for (Pattern excludedPattern : excludedPatterns) {
             if (excludedPattern.matcher(value).matches()) {
@@ -108,6 +126,7 @@ public IsExcluded isExcluded(String value) {
         return IsExcluded.no(excludedPatterns);
     }
 
+    @Override
     public Set<Pattern> getExcludedPatterns() {
         return excludedPatterns;
     }

core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java

@@ -36,6 +36,7 @@
 import java.lang.reflect.Method;
 import java.text.DateFormat;
 import java.util.*;
+import java.util.regex.Pattern;
 
 public class OgnlUtilTest extends XWorkTestCase {
 
@@ -954,11 +955,174 @@ public void testCallMethod() throws Exception {
         assertEquals(expected.getMessage(), "It isn't a simple method which can be called!");
     }
 
+    public void testXworkTestCaseOgnlUtilExclusions() throws Exception {
+        internalTestInitialEmptyOgnlUtilExclusions(ognlUtil);
+        internalTestOgnlUtilExclusionsImmutable(ognlUtil);
+    }
+
+    public void testDefaultOgnlUtilExclusions() throws Exception {
+        OgnlUtil basicOgnlUtil = new OgnlUtil();
+
+        internalTestInitialEmptyOgnlUtilExclusions(basicOgnlUtil);
+        internalTestOgnlUtilExclusionsImmutable(basicOgnlUtil);
+    }
+
+    public void testOgnlUtilExcludedAdditivity() throws Exception {
+        Set<Class<?>> excludedClasses;
+        Set<Pattern> excludedPackageNamePatterns;
+        Iterator<Pattern> excludedPackageNamePatternsIterator;
+        Set<String> excludedPackageNames;
+        Set<String> patternStrings = new HashSet<>();
+
+        ognlUtil.setExcludedClasses("java.lang.String,java.lang.Integer");
+        internalTestOgnlUtilExclusionsImmutable(ognlUtil);
+        excludedClasses = ognlUtil.getExcludedClasses();
+        assertNotNull("initial exluded classes null?", excludedClasses);
+        assertTrue("initial exluded classes size not 2 after adds?", excludedClasses.size() == 2);
+        assertTrue("String not in exclusions?", excludedClasses.contains(String.class));
+        assertTrue("Integer not in exclusions?", excludedClasses.contains(Integer.class));
+        ognlUtil.setExcludedClasses("java.lang.Boolean,java.lang.Double");
+        internalTestOgnlUtilExclusionsImmutable(ognlUtil);
+        excludedClasses = ognlUtil.getExcludedClasses();
+        assertNotNull("updated exluded classes null?", excludedClasses);
+        assertTrue("updated exluded classes size not 4 after adds?", excludedClasses.size() == 4);
+        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));
+
+        ognlUtil.setExcludedPackageNamePatterns("fakepackage1.*,fakepackage2.*");
+        internalTestOgnlUtilExclusionsImmutable(ognlUtil);
+        excludedPackageNamePatterns = ognlUtil.getExcludedPackageNamePatterns();
+        assertNotNull("initial exluded package name patterns null?", excludedPackageNamePatterns);
+        assertTrue("initial exluded package name patterns size not 2 after adds?", excludedPackageNamePatterns.size() == 2);
+        excludedPackageNamePatternsIterator = excludedPackageNamePatterns.iterator();
+        patternStrings.clear();
+        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 exluded package name patterns null?", excludedPackageNamePatterns);
+        assertTrue("updated exluded package name patterns size not 4 after adds?", excludedPackageNamePatterns.size() == 4);
+        excludedPackageNamePatternsIterator = excludedPackageNamePatterns.iterator();
+        patternStrings.clear();
+        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);
+        assertTrue("initial exluded package names not 2 after adds?", excludedPackageNames.size() == 2);
+        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);
+        assertTrue("updated exluded package names not 4 after adds?", excludedPackageNames.size() == 4);
+        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"));
+    }
+
+    private void internalTestInitialEmptyOgnlUtilExclusions(OgnlUtil ognlUtilParam) throws Exception {
+        Set<Class<?>> 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) throws Exception {
+        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
+        }
+
+        Set<String> excludedPackageNames = ognlUtilParam.getExcludedPackageNames();
+        assertNotNull("parameter exluded package names null?", excludedPackageNames);
+        try {
+            excludedPackageNames.clear();
+            fail("parameter excluded package names modifiable?");
+        } catch (UnsupportedOperationException uoe) {
+            // Expected failure
+        }
+        try {
+            excludedPackageNames.add("somepackagename");
+            fail("parameter excluded package names modifiable?");
+        } catch (UnsupportedOperationException uoe) {
+            // Expected failure
+        }
+        try {
+            excludedPackageNames.remove("somepackagename");
+            fail("parameter excluded package names modifiable?");
+        } catch (UnsupportedOperationException uoe) {
+            // Expected failure
+        }
+    }
+
     public void testAccessContext() throws Exception {
         Map<String, Object> context = ognlUtil.createDefaultContext(null);
 
         Foo foo = new Foo();
-        
+
         Object result = ognlUtil.getValue("#context", context, null);
         Object root = ognlUtil.getValue("#root", context, foo);
         Object that = ognlUtil.getValue("#this", context, foo);