freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [freemarker] branch 2.3-gae updated: MemberSelectorListMemberAccessPolicy related cleanup: Don't store the exception inside the MemberSelector
Date Mon, 13 Jan 2020 20:15:00 GMT
This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git


The following commit(s) were added to refs/heads/2.3-gae by this push:
     new 583b9d0  MemberSelectorListMemberAccessPolicy related cleanup: Don't store the exception
inside  the MemberSelector
583b9d0 is described below

commit 583b9d05dc415ab6c3be20257818d451d33ce435
Author: ddekany <ddekany@apache.org>
AuthorDate: Mon Jan 13 21:13:32 2020 +0100

    MemberSelectorListMemberAccessPolicy related cleanup: Don't store the exception inside
 the MemberSelector
---
 .../ext/beans/DefaultMemberAccessPolicy.java       |  27 +++--
 .../MemberSelectorListMemberAccessPolicy.java      | 120 ++++++---------------
 ... MemberSelectorListMemberAccessPolicyTest.java} |  26 +++--
 .../freemarker/template/ConfigurationTest.java     |  11 +-
 .../template/DefaultObjectWrapperTest.java         |   2 +-
 5 files changed, 75 insertions(+), 111 deletions(-)

diff --git a/src/main/java/freemarker/ext/beans/DefaultMemberAccessPolicy.java b/src/main/java/freemarker/ext/beans/DefaultMemberAccessPolicy.java
index c27f662..71067c9 100644
--- a/src/main/java/freemarker/ext/beans/DefaultMemberAccessPolicy.java
+++ b/src/main/java/freemarker/ext/beans/DefaultMemberAccessPolicy.java
@@ -101,17 +101,24 @@ public final class DefaultMemberAccessPolicy implements MemberAccessPolicy
{
                             throw new IllegalStateException("Unhandled rule: " + rule);
                         }
                     } else {
-                        MemberSelector memberSelector =
-                                MemberSelector.parse(line, classLoader);
-                        Class<?> upperBoundType = memberSelector.getUpperBoundType();
-                        if (upperBoundType != null) {
-                            if (!whitelistRuleFinalClasses.contains(upperBoundType)
-                                    && !whitelistRuleNonFinalClasses.contains(upperBoundType)
-                                    && !typesWithBlacklistUnlistedRule.contains(upperBoundType))
{
-                                throw new IllegalStateException("Type without rule: " + upperBoundType.getName());
+                        MemberSelector memberSelector;
+                        try {
+                            memberSelector = MemberSelector.parse(line, classLoader);
+                        } catch (ClassNotFoundException | NoSuchMethodException | NoSuchFieldException
e) {
+                            // Can happen if we run on an older Java than the list was made
for
+                            memberSelector = null;
+                        }
+                        if (memberSelector != null) {
+                            Class<?> upperBoundType = memberSelector.getUpperBoundType();
+                            if (upperBoundType != null) {
+                                if (!whitelistRuleFinalClasses.contains(upperBoundType)
+                                        && !whitelistRuleNonFinalClasses.contains(upperBoundType)
+                                        && !typesWithBlacklistUnlistedRule.contains(upperBoundType))
{
+                                    throw new IllegalStateException("Type without rule: "
+ upperBoundType.getName());
+                                }
+                                // We always do the same, as "blacklistUnlistedMembers" is
also defined via a whitelist:
+                                whitelistMemberSelectors.add(memberSelector);
                             }
-                            // We always do the same, as "blacklistUnlistedMembers" is also
defined via a whitelist:
-                            whitelistMemberSelectors.add(memberSelector);
                         }
                     }
                 }
diff --git a/src/main/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicy.java
b/src/main/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicy.java
index e0af091..a1eee12 100644
--- a/src/main/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicy.java
+++ b/src/main/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicy.java
@@ -28,7 +28,6 @@ import java.util.Collection;
 import java.util.List;
 import java.util.StringTokenizer;
 
-import freemarker.log.Logger;
 import freemarker.template.utility.ClassUtil;
 import freemarker.template.utility.NullArgumentException;
 
@@ -68,8 +67,6 @@ import freemarker.template.utility.NullArgumentException;
  * @since 2.3.30
  */
 public abstract class MemberSelectorListMemberAccessPolicy implements MemberAccessPolicy
{
-    private static final Logger LOG = Logger.getLogger("freemarker.beans");
-
     enum ListType {
         /** Only matched members will be exposed. */
         WHITELIST,
@@ -86,7 +83,7 @@ public abstract class MemberSelectorListMemberAccessPolicy implements MemberAcce
     /**
      * A condition that matches some type members. See {@link MemberSelectorListMemberAccessPolicy}
documentation for more.
      * Exactly one of these will be non-{@code null}:
-     * {@link #getMethod()}, {@link #getConstructor()}, {@link #getField()}, {@link #getException()}.
+     * {@link #getMethod()}, {@link #getConstructor()}, {@link #getField()}.
      *
      * @since 2.3.30
      */
@@ -95,8 +92,6 @@ public abstract class MemberSelectorListMemberAccessPolicy implements MemberAcce
         private final Method method;
         private final Constructor<?> constructor;
         private final Field field;
-        private final Exception exception;
-        private final String exceptionMemberSelectorString;
 
         /**
          * Use if you want to match methods similar to the specified one, in types that are
{@code instanceof} of
@@ -109,8 +104,6 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
             this.method = method;
             this.constructor = null;
             this.field = null;
-            this.exception = null;
-            this.exceptionMemberSelectorString = null;
         }
 
         /**
@@ -124,8 +117,6 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
             this.method = null;
             this.constructor = constructor;
             this.field = null;
-            this.exception = null;
-            this.exceptionMemberSelectorString = null;
         }
 
         /**
@@ -139,32 +130,10 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
             this.method = null;
             this.constructor = null;
             this.field = field;
-            this.exception = null;
-            this.exceptionMemberSelectorString = null;
         }
 
         /**
-         * Used to store the result of a parsing that's failed for a reason that we can skip
on runtime (typically,
-         * when a missing class or member was referred).
-         *
-         * @param upperBoundType {@code null} if resolving the upper bound type itself failed.
-         * @param exception Not {@code null}
-         * @param exceptionMemberSelectorString Not {@code null}; the selector whose resolution
has failed, used in
-         *      the log message.
-         */
-        public MemberSelector(Class<?> upperBoundType, Exception exception, String
exceptionMemberSelectorString) {
-            NullArgumentException.check("exception", exception);
-            NullArgumentException.check("exceptionMemberSelectorString", exceptionMemberSelectorString);
-            this.upperBoundType = upperBoundType;
-            this.method = null;
-            this.constructor = null;
-            this.field = null;
-            this.exception = exception;
-            this.exceptionMemberSelectorString = exceptionMemberSelectorString;
-        }
-
-        /**
-         * Maybe {@code null} if {@link #getException()} is non-{@code null}.
+         * Not {@code null}.
          */
         public Class<?> getUpperBoundType() {
             return upperBoundType;
@@ -172,7 +141,7 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
 
         /**
          * Maybe {@code null};
-         * set if the selector matches methods similar to the returned one, and there was
no exception.
+         * set if the selector matches methods similar to the returned one.
          */
         public Method getMethod() {
             return method;
@@ -180,7 +149,7 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
 
         /**
          * Maybe {@code null};
-         * set if the selector matches constructors similar to the returned one, and there
was no exception.
+         * set if the selector matches constructors similar to the returned one.
          */
         public Constructor<?> getConstructor() {
             return constructor;
@@ -188,27 +157,13 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
 
         /**
          * Maybe {@code null};
-         * set if the selector matches fields similar to the returned one, and there was
no exception.
+         * set if the selector matches fields similar to the returned one.
          */
         public Field getField() {
             return field;
         }
 
         /**
-         * Maybe {@code null}
-         */
-        public Exception getException() {
-            return exception;
-        }
-
-        /**
-         * Maybe {@code null}
-         */
-        public String getExceptionMemberSelectorString() {
-            return exceptionMemberSelectorString;
-        }
-
-        /**
          * Parses a member selector that was specified with a string.
          *
          * @param classLoader
@@ -226,9 +181,12 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
          *      {@code []}. In the member name, like {@code com.example.MyClass.myMember},
the class refers to the so
          *      called "upper bound class". Regarding that and inheritance rules see the
class level documentation.
          *
-         * @return The {@link MemberSelector}, which might has non-{@code null} {@link MemberSelector#exception}.
+         * @throws ClassNotFoundException If a type referred in the member selector can't
be found.
+         * @throws NoSuchMethodException If the method or constructor referred in the member
selector can't be found.
+         * @throws NoSuchFieldException If the field referred in the member selector can't
be found.
          */
-        public static MemberSelector parse(String memberSelectorString, ClassLoader classLoader)
{
+        public static MemberSelector parse(String memberSelectorString, ClassLoader classLoader)
throws
+                ClassNotFoundException, NoSuchMethodException, NoSuchFieldException {
             if (memberSelectorString.contains("<") || memberSelectorString.contains(">")
                     || memberSelectorString.contains("...") || memberSelectorString.contains(";"))
{
                 throw new IllegalArgumentException(
@@ -256,11 +214,7 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
                 throw new IllegalArgumentException("Malformed whitelist entry (malformed
upper bound class name): "
                         + memberSelectorString);
             }
-            try {
-                upperBoundClass = classLoader.loadClass(upperBoundClassStr);
-            } catch (ClassNotFoundException e) {
-                return new MemberSelector(null, e, cleanedStr);
-            }
+            upperBoundClass = classLoader.loadClass(upperBoundClassStr);
 
             String memberName = cleanedStr.substring(postClassDotIdx + 1, postMemberNameIdx);
             if (!isWellFormedJavaIdentifier(memberName)) {
@@ -293,33 +247,15 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
                             throw new IllegalArgumentException(
                                     "Malformed whitelist entry (malformed argument class
name): " + memberSelectorString);
                         }
-                        try {
-                            argClass = classLoader.loadClass(argClassName);
-                        } catch (ClassNotFoundException e) {
-                            return new MemberSelector(upperBoundClass, e, cleanedStr);
-                        } catch (SecurityException e) {
-                            return new MemberSelector(upperBoundClass, e, cleanedStr);
-                        }
+                        argClass = classLoader.loadClass(argClassName);
                     }
                     argTypes[i] = ClassUtil.getArrayClass(argClass, arrayDimensions);
                 }
-                try {
-                    return memberName.equals(upperBoundClass.getSimpleName())
-                            ? new MemberSelector(upperBoundClass, upperBoundClass.getConstructor(argTypes))
-                            : new MemberSelector(upperBoundClass, upperBoundClass.getMethod(memberName,
argTypes));
-                } catch (NoSuchMethodException e) {
-                    return new MemberSelector(upperBoundClass, e, cleanedStr);
-                } catch (SecurityException e) {
-                    return new MemberSelector(upperBoundClass, e, cleanedStr);
-                }
+                return memberName.equals(upperBoundClass.getSimpleName())
+                        ? new MemberSelector(upperBoundClass, upperBoundClass.getConstructor(argTypes))
+                        : new MemberSelector(upperBoundClass, upperBoundClass.getMethod(memberName,
argTypes));
             } else {
-                try {
-                    return new MemberSelector(upperBoundClass, upperBoundClass.getField(memberName));
-                } catch (NoSuchFieldException e) {
-                    return new MemberSelector(upperBoundClass, e, cleanedStr);
-                } catch (SecurityException e) {
-                    return new MemberSelector(upperBoundClass, e, cleanedStr);
-                }
+                return new MemberSelector(upperBoundClass, upperBoundClass.getField(memberName));
             }
         }
 
@@ -327,13 +263,24 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
          * Convenience method to parse all member selectors in the collection (see {@link
#parse(String, ClassLoader)}),
          * while also filtering out blank and comment lines; see {@link #parse(String, ClassLoader)},
          * and {@link #isIgnoredLine(String)}.
+         *
+         * @param ignoreMissingClassOrMember If {@code true}, members selectors that throw
exceptions because the
+         *      referred type or member can't be found will be silently ignored. If {@code
true}, same exceptions
+         *      will be just thrown by this method.
          */
-        public static List<MemberSelector> parse(Collection<String> memberSelectors,
-                ClassLoader classLoader) {
+        public static List<MemberSelector> parse(
+                Collection<String> memberSelectors, boolean ignoreMissingClassOrMember,
ClassLoader classLoader)
+                throws ClassNotFoundException, NoSuchMethodException, NoSuchFieldException
{
             List<MemberSelector> parsedMemberSelectors = new ArrayList<MemberSelector>(memberSelectors.size());
             for (String memberSelector : memberSelectors) {
                 if (!isIgnoredLine(memberSelector)) {
-                    parsedMemberSelectors.add(parse(memberSelector, classLoader));
+                    try {
+                        parsedMemberSelectors.add(parse(memberSelector, classLoader));
+                    } catch (ClassNotFoundException | NoSuchFieldException | NoSuchMethodException
e) {
+                        if (!ignoreMissingClassOrMember) {
+                            throw e;
+                        }
+                    }
                 }
             }
             return parsedMemberSelectors;
@@ -369,12 +316,7 @@ public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAcce
         fieldMatcher = new FieldMatcher();
         for (MemberSelector memberSelector : memberSelectors) {
             Class<?> upperBoundClass = memberSelector.upperBoundType;
-            if (memberSelector.exception != null) {
-                if (LOG.isDebugEnabled()) {
-                    LOG.debug("Member selector ignored due to error: " + memberSelector.getExceptionMemberSelectorString(),
-                            memberSelector.exception);
-                }
-            } else if (memberSelector.constructor != null) {
+            if (memberSelector.constructor != null) {
                 constructorMatcher.addMatching(upperBoundClass, memberSelector.constructor);
             } else if (memberSelector.method != null) {
                 methodMatcher.addMatching(upperBoundClass, memberSelector.method);
diff --git a/src/test/java/freemarker/ext/beans/MemberSelectorListAccessPolicyTest.java b/src/test/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicyTest.java
similarity index 96%
rename from src/test/java/freemarker/ext/beans/MemberSelectorListAccessPolicyTest.java
rename to src/test/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicyTest.java
index 1250c2a..2ba2458 100644
--- a/src/test/java/freemarker/ext/beans/MemberSelectorListAccessPolicyTest.java
+++ b/src/test/java/freemarker/ext/beans/MemberSelectorListMemberAccessPolicyTest.java
@@ -27,7 +27,7 @@ import java.util.Arrays;
 
 import org.junit.Test;
 
-public class MemberSelectorListAccessPolicyTest {
+public class MemberSelectorListMemberAccessPolicyTest {
 
     @Test
     public void testEmpty() throws NoSuchMethodException, NoSuchFieldException {
@@ -427,17 +427,25 @@ public class MemberSelectorListAccessPolicyTest {
     }
 
     private static WhitelistMemberAccessPolicy newWhitelistMemberAccessPolicy(String... memberSelectors)
{
-        return new WhitelistMemberAccessPolicy(
-                MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
-                        Arrays.asList(memberSelectors),
-                        MemberSelectorListAccessPolicyTest.class.getClassLoader()));
+        try {
+            return new WhitelistMemberAccessPolicy(
+                    MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
+                            Arrays.asList(memberSelectors), false,
+                            MemberSelectorListMemberAccessPolicyTest.class.getClassLoader()));
+        } catch (ClassNotFoundException | NoSuchMethodException | NoSuchFieldException e)
{
+            throw new RuntimeException(e);
+        }
     }
 
     private static BlacklistMemberAccessPolicy newBlacklistMemberAccessPolicy(String... memberSelectors)
{
-        return new BlacklistMemberAccessPolicy(
-                MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
-                        Arrays.asList(memberSelectors),
-                        MemberSelectorListAccessPolicyTest.class.getClassLoader()));
+        try {
+            return new BlacklistMemberAccessPolicy(
+                    MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
+                            Arrays.asList(memberSelectors), false,
+                            MemberSelectorListMemberAccessPolicyTest.class.getClassLoader()));
+        } catch (ClassNotFoundException | NoSuchMethodException | NoSuchFieldException e)
{
+            throw new RuntimeException(e);
+        }
     }
 
     public static class C1 {
diff --git a/src/test/java/freemarker/template/ConfigurationTest.java b/src/test/java/freemarker/template/ConfigurationTest.java
index 4b77eef..3ab21cc 100644
--- a/src/test/java/freemarker/template/ConfigurationTest.java
+++ b/src/test/java/freemarker/template/ConfigurationTest.java
@@ -1901,12 +1901,19 @@ public class ConfigurationTest extends TestCase {
         assertFalse(cfg.getFallbackOnNullLoopVariable());
     }
 
-    public static final MemberAccessPolicy CONFIG_TEST_MEMBER_ACCESS_POLICY =
-            new WhitelistMemberAccessPolicy(MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
+    public static final MemberAccessPolicy CONFIG_TEST_MEMBER_ACCESS_POLICY;
+    static {
+        try {
+            CONFIG_TEST_MEMBER_ACCESS_POLICY = new WhitelistMemberAccessPolicy(MemberSelectorListMemberAccessPolicy.MemberSelector.parse(
                     ImmutableList.<String>of(
                             File.class.getName() + ".getName()",
                             File.class.getName() + ".isFile()"),
+                    false,
                     ConfigurationTest.class.getClassLoader()));
+        } catch (ClassNotFoundException | NoSuchMethodException | NoSuchFieldException e)
{
+            throw new RuntimeException(e);
+        }
+    }
 
     @Test
     public void testMemberAccessPolicySetting() throws TemplateException {
diff --git a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
index 12f4954..0719f00 100644
--- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
+++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
@@ -328,7 +328,7 @@ public class DefaultObjectWrapperTest {
             WhitelistMemberAccessPolicy memberAccessPolicy =
                     new WhitelistMemberAccessPolicy(
                             WhitelistMemberAccessPolicy.MemberSelector.parse(
-                                    Arrays.asList(SomeBean.class.getName() + ".getX()"),
+                                    Arrays.asList(SomeBean.class.getName() + ".getX()"),
false,
                                     DefaultObjectWrapperTest.class.getClassLoader()));
             builder.setMemberAccessPolicy(memberAccessPolicy);
             DefaultObjectWrapper bw = builder.build();


Mime
View raw message