This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 3
in repository https://gitbox.apache.org/repos/asf/freemarker.git
The following commit(s) were added to refs/heads/3 by this push:
new bf4a0c1 Forward ported from 2.3-gae:
MemberSelectorListMemberAccessPolicy related cleanup: Don't store the exception
inside the MemberSelector
bf4a0c1 is described below
commit bf4a0c1f071351dffefe918e74b5bc92cfe17bed
Author: ddekany <[email protected]>
AuthorDate: Mon Jan 13 21:13:44 2020 +0100
Forward ported from 2.3-gae: MemberSelectorListMemberAccessPolicy related
cleanup: Don't store the exception inside the MemberSelector
---
.../apache/freemarker/core/ConfigurationTest.java | 11 +-
.../core/model/impl/DefaultObjectWrapperTest.java | 1 +
... MemberSelectorListMemberAccessPolicyTest.java} | 26 +++--
.../core/model/impl/DefaultMemberAccessPolicy.java | 27 +++--
.../impl/MemberSelectorListMemberAccessPolicy.java | 130 +++++++--------------
5 files changed, 83 insertions(+), 112 deletions(-)
diff --git
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java
index 56d5390..6e3eab7 100644
---
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java
+++
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java
@@ -733,12 +733,19 @@ public class ConfigurationTest {
assertEquals(1000L * 60 * 60 * 2, (Object)
cfgB.getTemplateUpdateDelayMilliseconds());
}
- 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/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java
index 9a1dddb..f83a333 100644
---
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java
+++
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperTest.java
@@ -147,6 +147,7 @@ public class DefaultObjectWrapperTest {
new WhitelistMemberAccessPolicy(
WhitelistMemberAccessPolicy.MemberSelector.parse(
Arrays.asList(SomeBean.class.getName() +
".getX()"),
+ false,
DefaultObjectWrapperTest.class.getClassLoader()));
DefaultObjectWrapper bw = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
diff --git
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/MemberSelectorListAccessPolicyTest.java
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicyTest.java
similarity index 96%
rename from
freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/MemberSelectorListAccessPolicyTest.java
rename to
freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicyTest.java
index 1e667b2..b8aeedd 100644
---
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/MemberSelectorListAccessPolicyTest.java
+++
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/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/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java
index 4713f2a..57659eb 100644
---
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java
+++
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java
@@ -99,17 +99,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/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicy.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicy.java
index 6317b1a..de82405 100644
---
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicy.java
+++
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberSelectorListMemberAccessPolicy.java
@@ -30,8 +30,6 @@ import java.util.StringTokenizer;
import org.apache.freemarker.core.util._ClassUtils;
import org.apache.freemarker.core.util._NullArgumentException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Superclass for member-selector-list-based member access policies, like
{@link WhitelistMemberAccessPolicy}.
@@ -65,9 +63,10 @@ import org.slf4j.LoggerFactory;
* with different return types (which is possible on the bytecode level) but
the same parameter types, then all
* variants of it is matched, or none is. Similarly, the type of fields isn't
used either, only the name of the field
* matters.
+ *
+ * @since 2.3.30
*/
public abstract class MemberSelectorListMemberAccessPolicy implements
MemberAccessPolicy {
- private static final Logger LOG =
LoggerFactory.getLogger(MemberSelectorListMemberAccessPolicy.class);
enum ListType {
/** Only matched members will be exposed. */
@@ -85,15 +84,15 @@ 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
*/
public final static class MemberSelector {
private final Class<?> upperBoundType;
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
@@ -106,8 +105,6 @@ public abstract class MemberSelectorListMemberAccessPolicy
implements MemberAcce
this.method = method;
this.constructor = null;
this.field = null;
- this.exception = null;
- this.exceptionMemberSelectorString = null;
}
/**
@@ -121,8 +118,6 @@ public abstract class MemberSelectorListMemberAccessPolicy
implements MemberAcce
this.method = null;
this.constructor = constructor;
this.field = null;
- this.exception = null;
- this.exceptionMemberSelectorString = null;
}
/**
@@ -136,32 +131,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;
@@ -169,7 +142,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;
@@ -177,7 +150,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;
@@ -185,27 +158,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
@@ -223,9 +182,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(
@@ -253,11 +215,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)) {
@@ -290,27 +248,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 | SecurityException e)
{
- return new MemberSelector(upperBoundClass, e,
cleanedStr);
- }
+ argClass = classLoader.loadClass(argClassName);
}
argTypes[i] = _ClassUtils.getArrayClass(argClass,
arrayDimensions);
}
- try {
- return memberName.equals(upperBoundClass.getSimpleName())
- ? new MemberSelector(upperBoundClass,
upperBoundClass.getConstructor(argTypes))
- : new MemberSelector(upperBoundClass,
upperBoundClass.getMethod(memberName, argTypes));
- } catch (NoSuchMethodException | 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 | SecurityException e) {
- return new MemberSelector(upperBoundClass, e, cleanedStr);
- }
+ return new MemberSelector(upperBoundClass,
upperBoundClass.getField(memberName));
}
}
@@ -318,13 +264,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) {
- List<MemberSelector> parsedMemberSelectors = new
ArrayList<>(memberSelectors.size());
+ 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;
@@ -360,12 +317,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);
@@ -377,34 +329,30 @@ public abstract class
MemberSelectorListMemberAccessPolicy implements MemberAcce
}
}
- @Override
public final ClassMemberAccessPolicy forClass(final Class<?> contextClass)
{
return new ClassMemberAccessPolicy() {
- @Override
public boolean isMethodExposed(Method method) {
return matchResultToIsExposedResult(
methodMatcher.matches(contextClass, method)
|| matchAnnotation != null
&&
_MethodUtils.getInheritableAnnotation(contextClass, method, matchAnnotation)
- != null);
+ != null);
}
- @Override
public boolean isConstructorExposed(Constructor<?> constructor) {
return matchResultToIsExposedResult(
constructorMatcher.matches(contextClass, constructor)
|| matchAnnotation != null
&&
_MethodUtils.getInheritableAnnotation(contextClass, constructor,
matchAnnotation)
- != null);
+ != null);
}
- @Override
public boolean isFieldExposed(Field field) {
return matchResultToIsExposedResult(
fieldMatcher.matches(contextClass, field)
|| matchAnnotation != null
&&
_MethodUtils.getInheritableAnnotation(contextClass, field, matchAnnotation)
- != null);
+ != null);
}
};
}
