MemberSelectorListMemberAccessPolicy related cleanup: Don't store the exception inside the MemberSelector
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 @@
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.List;
import java.util.StringTokenizer;
-import freemarker.log.Logger;
import freemarker.template.utility.ClassUtil;
import freemarker.template.utility.NullArgumentException;
@@ -68,8 +67,6 @@
* @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 @@
/**
* 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 @@
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 @@
this.method = method;
this.constructor = null;
this.field = null;
- this.exception = null;
- this.exceptionMemberSelectorString = null;
}
/**
@@ -124,8 +117,6 @@
this.method = null;
this.constructor = constructor;
this.field = null;
- this.exception = null;
- this.exceptionMemberSelectorString = null;
}
/**
@@ -139,32 +130,10 @@
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 @@
/**
* 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 @@
/**
* 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 @@
/**
* 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 @@
* {@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 @@
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 @@
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 @@
* 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 @@
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 org.junit.Test;
-public class MemberSelectorListAccessPolicyTest {
+public class MemberSelectorListMemberAccessPolicyTest {
@Test
public void testEmpty() throws NoSuchMethodException, NoSuchFieldException {
@@ -427,17 +427,25 @@
}
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 @@
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 @@
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();
