You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2018/06/20 05:17:58 UTC

[struts] 02/03: Makes OgnlUtil more immutable

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

lukaszlenart pushed a commit to branch struts-2-5-x
in repository https://gitbox.apache.org/repos/asf/struts.git

commit 3ec7fa9d80f0e04b385b4846fef6f3f3c20e3a5e
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Tue May 29 12:58:44 2018 +0200

    Makes OgnlUtil more immutable
---
 .../com/opensymphony/xwork2/ognl/OgnlUtil.java     | 51 +++++++++++++++++-----
 .../com/opensymphony/xwork2/ognl/OgnlUtilTest.java | 12 ++---
 2 files changed, 45 insertions(+), 18 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 0f2d78e..ad4f9d5 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -62,14 +62,36 @@ public class OgnlUtil {
     private boolean enableExpressionCache = true;
     private boolean enableEvalExpression;
 
-    private Set<Class<?>> excludedClasses = Collections.emptySet();
-    private Set<Pattern> excludedPackageNamePatterns = Collections.emptySet();
-    private Set<String> excludedPackageNames = Collections.emptySet();
+    private Set<Class<?>> excludedClasses;
+    private Set<Pattern> excludedPackageNamePatterns;
+    private Set<String> excludedPackageNames;
 
     private Container container;
     private boolean allowStaticMethodAccess;
     private boolean disallowProxyMemberAccess;
 
+    public OgnlUtil(
+        @Inject(value = XWorkConstants.OGNL_EXCLUDED_CLASSES, required = false)
+        String commaDelimitedClasses,
+        @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+        String commaDelimitedPackagePatterns,
+        @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAMES, required = false)
+        String commaDelimitedPackageNames
+    ) {
+        excludedClasses = Collections.unmodifiableSet(parseExcludedClasses(commaDelimitedClasses));
+        excludedPackageNamePatterns = Collections.unmodifiableSet(parseExcludedPackageNamePatterns(commaDelimitedPackagePatterns));
+        excludedPackageNames = Collections.unmodifiableSet(parseExcludedPackageNames(commaDelimitedPackageNames));
+    }
+
+    /**
+     * Constructor used by internal DI
+     */
+    public OgnlUtil() {
+        excludedClasses = Collections.emptySet();
+        excludedPackageNamePatterns = Collections.emptySet();
+        excludedPackageNames = Collections.emptySet();
+    }
+
     @Inject
     public void setXWorkConverter(XWorkConverter conv) {
         this.defaultConverter = new OgnlTypeConverterWrapper(conv);
@@ -94,8 +116,7 @@ public class OgnlUtil {
         }
     }
 
-    @Inject(value = XWorkConstants.OGNL_EXCLUDED_CLASSES, required = false)
-    public void setExcludedClasses(String commaDelimitedClasses) {
+    private Set<Class<?>> parseExcludedClasses(String commaDelimitedClasses) {
         Set<String> classNames = TextParseUtil.commaDelimitedStringToSet(commaDelimitedClasses);
         Set<Class<?>> classes = new HashSet<>();
 
@@ -107,11 +128,11 @@ public class OgnlUtil {
             }
         }
 
-        excludedClasses = Collections.unmodifiableSet(classes);
+        return classes;
     }
 
-    @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
-    public void setExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
+
+    private Set<Pattern> parseExcludedPackageNamePatterns(String commaDelimitedPackagePatterns) {
         Set<String> packagePatterns = TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackagePatterns);
         Set<Pattern> packageNamePatterns = new HashSet<>();
 
@@ -119,12 +140,11 @@ public class OgnlUtil {
             packageNamePatterns.add(Pattern.compile(pattern));
         }
 
-        excludedPackageNamePatterns = Collections.unmodifiableSet(packageNamePatterns);
+        return packageNamePatterns;
     }
 
-    @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAMES, required = false)
-    public void setExcludedPackageNames(String commaDelimitedPackageNames) {
-        excludedPackageNames = Collections.unmodifiableSet(TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames));
+    private Set<String> parseExcludedPackageNames(String commaDelimitedPackageNames) {
+        return TextParseUtil.commaDelimitedStringToSet(commaDelimitedPackageNames);
     }
 
     public Set<Class<?>> getExcludedClasses() {
@@ -698,6 +718,13 @@ public class OgnlUtil {
         return Ognl.createDefaultContext(root, resolver, defaultConverter, memberAccess);
     }
 
+    protected void addExcludedClasses(String commaDelimitedClasses) {
+        Set<Class<?>> existingClasses = new HashSet<>(excludedClasses);
+        existingClasses.addAll(parseExcludedClasses(commaDelimitedClasses));
+
+        excludedClasses = Collections.unmodifiableSet(existingClasses);
+    }
+
     private interface OgnlTask<T> {
         T execute(Object tree) throws OgnlException;
     }
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 741a396..a9af6e2 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -41,7 +41,7 @@ public class OgnlUtilTest extends XWorkTestCase {
         ognlUtil = container.getInstance(OgnlUtil.class);
     }
     
-    public void testCanSetADependentObject() throws Exception {
+    public void testCanSetADependentObject() {
         String dogName = "fido";
 
         OgnlRuntime.setNullHandler(Owner.class, new NullHandler() {
@@ -653,7 +653,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Exception expected = null;
         try {
-            ognlUtil.setExcludedClasses(Object.class.getName());
+            ognlUtil.addExcludedClasses(Object.class.getName());
             ognlUtil.setValue("class.classLoader.defaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true);
             fail();
         } catch (OgnlException e) {
@@ -669,7 +669,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Exception expected = null;
         try {
-            ognlUtil.setExcludedClasses(Object.class.getName());
+            ognlUtil.addExcludedClasses(Object.class.getName());
             ognlUtil.setValue("Class.ClassLoader.DefaultAssertionStatus", ognlUtil.createDefaultContext(foo), foo, true);
             fail();
         } catch (OgnlException e) {
@@ -685,7 +685,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Exception expected = null;
         try {
-            ognlUtil.setExcludedClasses(Object.class.getName());
+            ognlUtil.addExcludedClasses(Object.class.getName());
             ognlUtil.setValue("class['classLoader']['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true);
             fail();
         } catch (OgnlException e) {
@@ -716,7 +716,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Exception expected = null;
         try {
-            ognlUtil.setExcludedClasses(Object.class.getName());
+            ognlUtil.addExcludedClasses(Object.class.getName());
             ognlUtil.setValue("class[\"classLoader\"]['defaultAssertionStatus']", ognlUtil.createDefaultContext(foo), foo, true);
             fail();
         } catch (OgnlException e) {
@@ -762,7 +762,7 @@ public class OgnlUtilTest extends XWorkTestCase {
 
         Exception expected = null;
         try {
-            ognlUtil.setExcludedClasses(Runtime.class.getName());
+            ognlUtil.addExcludedClasses(Runtime.class.getName());
             ognlUtil.setValue("@java.lang.Runtime@getRuntime().exec('mate')", ognlUtil.createDefaultContext(foo), foo, true);
             fail();
         } catch (OgnlException e) {


Re: [struts] 02/03: Makes OgnlUtil more immutable

Posted by Yasser Zamani <ya...@apache.org>.

On 6/20/2018 9:47 AM, lukaszlenart@apache.org wrote:
> +    public OgnlUtil(
> +        @Inject(value = XWorkConstants.OGNL_EXCLUDED_CLASSES, required = false)
> +        String commaDelimitedClasses,
> +        @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
> +        String commaDelimitedPackagePatterns,
> +        @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAMES, required = false)
> +        String commaDelimitedPackageNames
> +    ) {
> +        excludedClasses = Collections.unmodifiableSet(parseExcludedClasses(commaDelimitedClasses));
> +        excludedPackageNamePatterns = Collections.unmodifiableSet(parseExcludedPackageNamePatterns(commaDelimitedPackagePatterns));
> +        excludedPackageNames = Collections.unmodifiableSet(parseExcludedPackageNames(commaDelimitedPackageNames));
> +    }
> +
> +    /**
> +     * Constructor used by internal DI
> +     */
> +    public OgnlUtil() {
> +        excludedClasses = Collections.emptySet();
> +        excludedPackageNamePatterns = Collections.emptySet();
> +        excludedPackageNames = Collections.emptySet();
> +    }
> +

This doesn't work. Execution never reach the first contractor and
exclusions are always empty. I tried to solve it via [1] but I get this
error: No mapping found for dependency [type=java.lang.String,
name='struts.excludedClasses'] in public
com.opensymphony.xwork2.ognl.OgnlUtil(java.lang.String,java.lang.String,java.lang.String).

[1]
Index: core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
(revision a75eddb5154f9705bd2bac39f704512c8453e0b9)
+++ core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
(revision )
@@ -70,12 +70,13 @@
     private boolean allowStaticMethodAccess;
     private boolean disallowProxyMemberAccess;

+    @Inject
     public OgnlUtil(
-        @Inject(value = XWorkConstants.OGNL_EXCLUDED_CLASSES, required
= false)
+        @Inject(value = StrutsConstants.STRUTS_EXCLUDED_CLASSES,
required = false)
         String commaDelimitedClasses,
-        @Inject(value =
XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
+        @Inject(value =
StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS, required = false)
         String commaDelimitedPackagePatterns,
-        @Inject(value = XWorkConstants.OGNL_EXCLUDED_PACKAGE_NAMES,
required = false)
+        @Inject(value = StrutsConstants.STRUTS_EXCLUDED_PACKAGE_NAMES,
required = false)
         String commaDelimitedPackageNames
     ) {
         excludedClasses =
Collections.unmodifiableSet(parseExcludedClasses(commaDelimitedClasses));