Merge pull request #726 from atlassian/WW-5332-package-validation

WW-5332 Add validation for package name parsing

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

@@ -96,8 +96,8 @@ public class OgnlUtil {
     @Deprecated
     public OgnlUtil() {
         // Instantiate default Expression and BeanInfo caches (factories must be non-null).
-        this(new DefaultOgnlExpressionCacheFactory<String, Object>(),
-             new DefaultOgnlBeanInfoCacheFactory<Class<?>, BeanInfo>());
+        this(new DefaultOgnlExpressionCacheFactory<>(),
+                new DefaultOgnlBeanInfoCacheFactory<>());
     }
 
     /**
@@ -261,7 +261,11 @@ public class OgnlUtil {
     }
 
     private Set<String> parseExcludedPackageNames(String commaDelimitedPackageNames) {
-        return TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames);
+        Set<String> parsedSet = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames);
+        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;
     }
 
     public Set<Class<?>> getExcludedClasses() {

core/src/main/java/com/opensymphony/xwork2/util/TextParseUtil.java

@@ -20,9 +20,12 @@ package com.opensymphony.xwork2.util;
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.conversion.impl.XWorkConverter;
-import com.opensymphony.xwork2.inject.Container;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 
 /**
@@ -241,13 +244,13 @@ public class TextParseUtil {
     /**
      * Tests if given string is not null and not empty when excluding of empty
      * elements is requested.
-     * 
+     *
      * @param str String to check.
      * @param excludeEmptyElements Whether empty elements shall be excluded.
      * @return True if given string can be included in collection.
      */
     private static boolean shallBeIncluded(String str, boolean excludeEmptyElements) {
-        return !excludeEmptyElements || ((str != null) && (str.length() > 0));
+        return !excludeEmptyElements || str != null && !str.isEmpty();
     }
 
     /**
@@ -256,14 +259,7 @@ public class TextParseUtil {
      * @return A set from comma delimited Strings.
      */
     public static Set<String> commaDelimitedStringToSet(String s) {
-        Set<String> set = new HashSet<>();
-        String[] split = s.split(",");
-        for (String aSplit : split) {
-            String trimmed = aSplit.trim();
-            if (trimmed.length() > 0)
-                set.add(trimmed);
-        }
-        return set;
+        return Arrays.stream(s.split(",")).map(String::trim).filter(s1 -> !s1.isEmpty()).collect(Collectors.toSet());
     }
 
 
@@ -287,7 +283,7 @@ public class TextParseUtil {
      *
      * @author tm_jee
      */
-    public static interface ParsedValueEvaluator {
+    public interface ParsedValueEvaluator {
 
     	/**
     	 * Evaluated the value parsed by Ognl value stack.

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

@@ -64,6 +64,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import static org.junit.Assert.assertThrows;
+
 public class OgnlUtilTest extends XWorkTestCase {
 
     // Fields for static field access test
@@ -1712,6 +1714,13 @@ 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();
@@ -1728,7 +1737,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
     public void testGetExcludedPackageNamesAlternateConstructorPopulated() {
         // 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.setExcludedPackageNames("java.lang,java.awt");
         assertEquals(util.getExcludedPackageNames().size(), 2);
         try {