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 e124a89 Forward ported from 2.3-gae: Fixed issue where StaticModel
didn't consider the MemberAccessPolicy when exposing fields (as that wasn't
filtered at all before 2.3.30). Also simplified related ClassIntrospector API a
bit.
e124a89 is described below
commit e124a892a18a053f86c1db7e834d0645a714cb5e
Author: ddekany <[email protected]>
AuthorDate: Tue Jan 14 08:41:42 2020 +0100
Forward ported from 2.3-gae: Fixed issue where StaticModel didn't consider
the MemberAccessPolicy when exposing fields (as that wasn't filtered at all
before 2.3.30). Also simplified related ClassIntrospector API a bit.
---
...DefaultObjectWrapperMemberAccessPolicyTest.java | 195 ++++++++++++++++++---
.../model/impl/AllowAllMemberAccessPolicy.java | 53 ++++++
.../core/model/impl/ClassIntrospector.java | 56 +++---
.../freemarker/core/model/impl/StaticModel.java | 9 +-
4 files changed, 255 insertions(+), 58 deletions(-)
diff --git
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperMemberAccessPolicyTest.java
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperMemberAccessPolicyTest.java
index 4747f3e..39c3030 100644
---
a/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperMemberAccessPolicyTest.java
+++
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperMemberAccessPolicyTest.java
@@ -144,6 +144,56 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
}
@Test
+ public void testExposeAllWithCustomMemberAccessPolicy() throws
TemplateException {
+ {
+ DefaultObjectWrapper ow = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .exposureLevel(DefaultObjectWrapper.EXPOSE_ALL)
+ .exposeFields(true)
+
.memberAccessPolicy(AllowNothingMemberAccessPolicy.INSTANCE)
+ .build();
+
+ TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C());
+ // Because the MemberAccessPolicy is ignored:
+ assertNotNull(objM.get("publicField1"));
+ assertNotNull(objM.get("m1"));
+ assertNotNull(objM.get("M1"));
+ assertNotNull(objM.get("notify"));
+ assertNull(objM.get("STATIC_FIELD")); // Because it's static
+
+ TemplateHashModel staticModel = (TemplateHashModel)
ow.getStaticModels().get(C.class.getName());
+ assertNotNull(staticModel.get("M1"));
+ assertNotNull(staticModel.get("STATIC_FIELD"));
+ }
+ {
+ DefaultObjectWrapper ow = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .exposeFields(true)
+
.memberAccessPolicy(AllowNothingMemberAccessPolicy.INSTANCE)
+ .build();
+
+ TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C());
+ // Because the MemberAccessPolicy is ignored:
+ assertNull(objM.get("publicField1"));
+ assertNull(objM.get("m1"));
+ assertNull(objM.get("M1"));
+ assertNull(objM.get("notify"));
+ assertNull(objM.get("STATIC_FIELD"));
+
+ TemplateHashModel staticModel = (TemplateHashModel)
ow.getStaticModels().get(C.class.getName());
+ try {
+ assertNull(staticModel.get("M1"));
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ try {
+ assertNull(staticModel.get("STATIC_FIELD"));
+ fail();
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+ }
+
+ @Test
public void testExposeFieldsWithDefaultMemberAccessPolicy() throws
TemplateException {
DefaultObjectWrapper.Builder owb = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0);
owb.setExposeFields(true);
@@ -378,29 +428,29 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
@Test
public void testMemberAccessPolicyAndNewBI() throws IOException,
TemplateException, NoSuchMethodException {
- DefaultObjectWrapper.Builder owb = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0);
- owb.setMemberAccessPolicy(new MemberAccessPolicy() {
- @Override
- public ClassMemberAccessPolicy forClass(Class<?> contextClass) {
- return new ClassMemberAccessPolicy() {
- @Override
- public boolean isMethodExposed(Method method) {
- return true;
- }
-
- @Override
- public boolean isConstructorExposed(Constructor<?>
constructor) {
- return
constructor.getDeclaringClass().equals(CustomModel.class);
- }
-
+ DefaultObjectWrapper ow = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .memberAccessPolicy(new MemberAccessPolicy() {
@Override
- public boolean isFieldExposed(Field field) {
- return true;
+ public ClassMemberAccessPolicy forClass(Class<?>
contextClass) {
+ return new ClassMemberAccessPolicy() {
+ @Override
+ public boolean isMethodExposed(Method method) {
+ return true;
+ }
+
+ @Override
+ public boolean isConstructorExposed(Constructor<?>
constructor) {
+ return
constructor.getDeclaringClass().equals(CustomModel.class);
+ }
+
+ @Override
+ public boolean isFieldExposed(Field field) {
+ return true;
+ }
+ };
}
- };
- }
- });
- DefaultObjectWrapper ow = owb.build();
+ })
+ .build();
String templateSourceCode = "${'" + CustomModel.class.getName() +
"'?new()} "
+ "<#attempt>${'" + OtherCustomModel.class.getName() +
"'?new()}<#recover>failed</#attempt>";
@@ -433,6 +483,71 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
}
}
+ @Test
+ public void testMemberAccessPolicyAndStatics() throws TemplateException {
+ DefaultObjectWrapper ow = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0)
+ .memberAccessPolicy(new MemberAccessPolicy() {
+ public ClassMemberAccessPolicy forClass(Class<?>
contextClass) {
+ return new ClassMemberAccessPolicy() {
+ public boolean isMethodExposed(Method method) {
+ return method.getName().equals("m1");
+ }
+
+ public boolean isConstructorExposed(Constructor<?>
constructor) {
+ return false;
+ }
+
+ public boolean isFieldExposed(Field field) {
+ String name = field.getName();
+ return name.equals("F1") || name.equals("V1");
+ }
+ };
+ }
+ })
+ .build();
+ TemplateHashModel statics = (TemplateHashModel)
ow.getStaticModels().get(Statics.class.getName());
+
+ assertEquals(1, ((TemplateNumberModel)
statics.get("F1")).getAsNumber());
+ try {
+ statics.get("F2");
+ fail();
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+
+ assertEquals(1, ((TemplateNumberModel)
statics.get("V1")).getAsNumber());
+ try {
+ statics.get("V2");
+ fail();
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+
+ assertEquals(1,
+ ((TemplateNumberModel) ((JavaMethodModel) statics.get("m1"))
+ .execute(new TemplateModel[0], null))
+ .getAsNumber());
+ try {
+ assertNull(statics.get("m2"));
+ fail();
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+
+ @Test
+ public void testMemberAccessPolicyAndStatics2() throws TemplateException {
+ DefaultObjectWrapper ow = new
DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0).build();
+ TemplateHashModel sys = (TemplateHashModel)
ow.getStaticModels().get(System.class.getName());
+ assertNotNull(sys.get("currentTimeMillis"));
+ try {
+ sys.get("exit");
+ fail();
+ } catch (TemplateException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+
private static Object getHashValue(ObjectWrapperAndUnwrapper ow,
TemplateHashModel objM, String key)
throws TemplateException {
return ow.unwrap(objM.get(key));
@@ -501,6 +616,8 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
public String wait(int otherOverload) {
return "safe wait(" + otherOverload + ")";
}
+
+ public static void M1() { }
}
public static class CExtended extends C {
@@ -547,5 +664,41 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
}
}
+ public static class Statics {
+ public static final int F1 = 1;
+ public static final int F2 = 2;
+ public static int V1 = 1;
+ public static int V2 = 2;
+ public static int m1() {
+ return 1;
+ }
+ public static int m2() {
+ return 2;
+ }
+ }
+
+ public static class AllowNothingMemberAccessPolicy implements
MemberAccessPolicy {
+ private static final AllowNothingMemberAccessPolicy INSTANCE = new
AllowNothingMemberAccessPolicy();
+
+ @Override
+ public ClassMemberAccessPolicy forClass(Class<?> contextClass) {
+ return new ClassMemberAccessPolicy() {
+ @Override
+ public boolean isMethodExposed(Method method) {
+ return false;
+ }
+
+ @Override
+ public boolean isConstructorExposed(Constructor<?>
constructor) {
+ return false;
+ }
+
+ @Override
+ public boolean isFieldExposed(Field field) {
+ return false;
+ }
+ };
+ }
+ }
}
diff --git
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/AllowAllMemberAccessPolicy.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/AllowAllMemberAccessPolicy.java
new file mode 100644
index 0000000..f96838b
--- /dev/null
+++
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/AllowAllMemberAccessPolicy.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core.model.impl;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+final class AllowAllMemberAccessPolicy implements MemberAccessPolicy {
+ public static final AllowAllMemberAccessPolicy INSTANCE = new
AllowAllMemberAccessPolicy();
+
+ private AllowAllMemberAccessPolicy() {
+ }
+
+ public static final ClassMemberAccessPolicy CLASS_POLICY_INSTANCE = new
ClassMemberAccessPolicy() {
+ @Override
+ public boolean isMethodExposed(Method method) {
+ return true;
+ }
+
+ @Override
+ public boolean isConstructorExposed(Constructor<?> constructor) {
+ return true;
+ }
+
+ @Override
+ public boolean isFieldExposed(Field field) {
+ return true;
+ }
+ };
+
+ @Override
+ public ClassMemberAccessPolicy forClass(Class<?> contextClass) {
+ return CLASS_POLICY_INSTANCE;
+ }
+}
diff --git
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java
index e48c658..b1650ff 100644
---
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java
+++
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassIntrospector.java
@@ -256,26 +256,26 @@ class ClassIntrospector {
*/
private Map<Object, Object> createClassIntrospectionData(Class<?> clazz) {
final Map<Object, Object> introspData = new HashMap<>();
- ClassMemberAccessPolicy classMemberAccessPolicy =
getClassMemberAccessPolicyIfNotIgnored(clazz);
+ ClassMemberAccessPolicy effClassMemberAccessPolicy =
getEffectiveClassMemberAccessPolicy(clazz);
if (exposeFields) {
- addFieldsToClassIntrospectionData(introspData, clazz,
classMemberAccessPolicy);
+ addFieldsToClassIntrospectionData(introspData, clazz,
effClassMemberAccessPolicy);
}
final Map<ExecutableMemberSignature, List<Method>> accessibleMethods =
discoverAccessibleMethods(clazz);
- addGenericGetToClassIntrospectionData(introspData, accessibleMethods,
classMemberAccessPolicy);
+ addGenericGetToClassIntrospectionData(introspData, accessibleMethods,
effClassMemberAccessPolicy);
if (exposureLevel != DefaultObjectWrapper.EXPOSE_NOTHING) {
try {
- addBeanInfoToClassIntrospectionData(introspData, clazz,
accessibleMethods, classMemberAccessPolicy);
+ addBeanInfoToClassIntrospectionData(introspData, clazz,
accessibleMethods, effClassMemberAccessPolicy);
} catch (IntrospectionException e) {
LOG.warn("Couldn't properly perform introspection for class
{}", clazz.getName(), e);
introspData.clear(); // FIXME NBC: Don't drop everything here.
}
}
- addConstructorsToClassIntrospectionData(introspData, clazz,
classMemberAccessPolicy);
+ addConstructorsToClassIntrospectionData(introspData, clazz,
effClassMemberAccessPolicy);
if (introspData.size() > 1) {
return introspData;
@@ -288,10 +288,10 @@ class ClassIntrospector {
}
private void addFieldsToClassIntrospectionData(Map<Object, Object>
introspData, Class<?> clazz,
- ClassMemberAccessPolicy classMemberAccessPolicy) throws
SecurityException {
+ ClassMemberAccessPolicy effClassMemberAccessPolicy) throws
SecurityException {
for (Field field : clazz.getFields()) {
if ((field.getModifiers() & Modifier.STATIC) == 0) {
- if (classMemberAccessPolicy == null ||
classMemberAccessPolicy.isFieldExposed(field)) {
+ if (effClassMemberAccessPolicy.isFieldExposed(field)) {
introspData.put(field.getName(), field);
}
}
@@ -300,7 +300,7 @@ class ClassIntrospector {
private void addBeanInfoToClassIntrospectionData(
Map<Object, Object> introspData, Class<?> clazz,
Map<ExecutableMemberSignature, List<Method>> accessibleMethods,
- ClassMemberAccessPolicy classMemberAccessPolicy) throws
IntrospectionException {
+ ClassMemberAccessPolicy effClassMemberAccessPolicy) throws
IntrospectionException {
BeanInfo beanInfo = Introspector.getBeanInfo(clazz);
List<PropertyDescriptor> pdas = getPropertyDescriptors(beanInfo,
clazz);
int pdasLength = pdas.size();
@@ -308,7 +308,7 @@ class ClassIntrospector {
for (int i = pdasLength - 1; i >= 0; --i) {
addPropertyDescriptorToClassIntrospectionData(
introspData, pdas.get(i), clazz,
- accessibleMethods, classMemberAccessPolicy);
+ accessibleMethods, effClassMemberAccessPolicy);
}
if (exposureLevel < DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) {
@@ -320,7 +320,7 @@ class ClassIntrospector {
IdentityHashMap<Method, Void> argTypesUsedByIndexerPropReaders =
null;
for (int i = mdsSize - 1; i >= 0; --i) {
final Method method =
getMatchingAccessibleMethod(mds.get(i).getMethod(), accessibleMethods);
- if (method != null && isMethodExposed(classMemberAccessPolicy,
method)) {
+ if (method != null &&
effClassMemberAccessPolicy.isMethodExposed(method)) {
decision.setDefaults(method);
if (methodAppearanceFineTuner != null) {
if (decisionInput == null) {
@@ -337,7 +337,7 @@ class ClassIntrospector {
(decision.getReplaceExistingProperty()
|| !(introspData.get(propDesc.getName())
instanceof FastPropertyDescriptor))) {
addPropertyDescriptorToClassIntrospectionData(
- introspData, propDesc, clazz,
accessibleMethods, classMemberAccessPolicy);
+ introspData, propDesc, clazz,
accessibleMethods, effClassMemberAccessPolicy);
}
String methodKey = decision.getExposeMethodAs();
@@ -647,33 +647,34 @@ class ClassIntrospector {
private void addPropertyDescriptorToClassIntrospectionData(Map<Object,
Object> introspData,
PropertyDescriptor pd, Class<?> clazz,
Map<ExecutableMemberSignature, List<Method>> accessibleMethods,
- ClassMemberAccessPolicy classMemberAccessPolicy) {
+ ClassMemberAccessPolicy effClassMemberAccessPolicy) {
Method readMethod = getMatchingAccessibleMethod(pd.getReadMethod(),
accessibleMethods);
- if (readMethod != null && isMethodExposed(classMemberAccessPolicy,
readMethod)) {
+ if (readMethod != null &&
effClassMemberAccessPolicy.isMethodExposed(readMethod)) {
introspData.put(pd.getName(), new
FastPropertyDescriptor(readMethod));
}
}
private void addGenericGetToClassIntrospectionData(Map<Object, Object>
introspData,
- Map<ExecutableMemberSignature, List<Method>> accessibleMethods,
ClassMemberAccessPolicy classMemberAccessPolicy) {
+ Map<ExecutableMemberSignature, List<Method>> accessibleMethods,
+ ClassMemberAccessPolicy effClassMemberAccessPolicy) {
Method genericGet = getFirstAccessibleMethod(
GET_STRING_SIGNATURE, accessibleMethods);
if (genericGet == null) {
genericGet = getFirstAccessibleMethod(
GET_OBJECT_SIGNATURE, accessibleMethods);
}
- if (genericGet != null && isMethodExposed(classMemberAccessPolicy,
genericGet)) {
+ if (genericGet != null &&
effClassMemberAccessPolicy.isMethodExposed(genericGet)) {
introspData.put(GENERIC_GET_KEY, genericGet);
}
}
private void addConstructorsToClassIntrospectionData(final Map<Object,
Object> introspData,
- Class<?> clazz, ClassMemberAccessPolicy classMemberAccessPolicy) {
+ Class<?> clazz, ClassMemberAccessPolicy
effClassMemberAccessPolicy) {
try {
Constructor<?>[] ctorsUnfiltered = clazz.getConstructors();
List<Constructor<?>> ctors = new
ArrayList<Constructor<?>>(ctorsUnfiltered.length);
for (Constructor<?> ctor : ctorsUnfiltered) {
- if (classMemberAccessPolicy == null ||
classMemberAccessPolicy.isConstructorExposed(ctor)) {
+ if (effClassMemberAccessPolicy.isConstructorExposed(ctor)) {
ctors.add(ctor);
}
}
@@ -787,23 +788,12 @@ class ClassIntrospector {
}
/**
- * Returns the {@link ClassMemberAccessPolicy}, or {@code null} if it
should be ignored because of other settings.
- * (Ideally, all such rules should be contained in {@link
ClassMemberAccessPolicy} alone, but that interface was
- * added late in history.)
- *
- * @see #isMethodExposed(ClassMemberAccessPolicy, Method)
+ * Returns the {@link ClassMemberAccessPolicy} to actually use, which is
not just
+ * {@link DefaultObjectWrapper#getMemberAccessPolicy()} if {@link
DefaultObjectWrapper#getExposureLevel()} is more allowing than
+ * {@link DefaultObjectWrapper#EXPOSE_SAFE}. {@link
DefaultObjectWrapper#EXPOSE_NOTHING} though is not factored in here.
*/
- ClassMemberAccessPolicy getClassMemberAccessPolicyIfNotIgnored(Class
containingClass) {
- return exposureLevel < DefaultObjectWrapper.EXPOSE_SAFE ? null :
memberAccessPolicy.forClass(containingClass);
- }
-
- /**
- * @param classMemberAccessPolicyIfNotIgnored
- * The value returned by {@link
#getClassMemberAccessPolicyIfNotIgnored(Class)}
- */
- static boolean isMethodExposed(ClassMemberAccessPolicy
classMemberAccessPolicyIfNotIgnored, Method method) {
- return classMemberAccessPolicyIfNotIgnored == null
- || classMemberAccessPolicyIfNotIgnored.isMethodExposed(method);
+ ClassMemberAccessPolicy getEffectiveClassMemberAccessPolicy(Class<?>
containingClass) {
+ return exposureLevel < DefaultObjectWrapper.EXPOSE_SAFE ?
AllowAllMemberAccessPolicy.CLASS_POLICY_INSTANCE :
memberAccessPolicy.forClass(containingClass);
}
private static Map<Method, Class<?>[]> getArgTypesByMethod(Map<Object,
Object> classInfo) {
diff --git
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/StaticModel.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/StaticModel.java
index c586355..e5e0e9d 100644
---
a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/StaticModel.java
+++
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/StaticModel.java
@@ -118,10 +118,13 @@ final class StaticModel implements TemplateHashModelEx {
return;
}
+ ClassMemberAccessPolicy effClassMemberAccessPolicy =
+
wrapper.getClassIntrospector().getEffectiveClassMemberAccessPolicy(clazz);
+
Field[] fields = clazz.getFields();
for (Field field : fields) {
int mod = field.getModifiers();
- if (Modifier.isPublic(mod) && Modifier.isStatic(mod)) {
+ if (Modifier.isPublic(mod) && Modifier.isStatic(mod) &&
effClassMemberAccessPolicy.isFieldExposed(field)) {
if (Modifier.isFinal(mod)) {
try {
// public static final fields are evaluated once and
@@ -139,13 +142,11 @@ final class StaticModel implements TemplateHashModelEx {
}
}
if (wrapper.getExposureLevel() <
DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) {
- ClassMemberAccessPolicy classMemberAccessPolicy =
-
wrapper.getClassIntrospector().getClassMemberAccessPolicyIfNotIgnored(clazz);
Method[] methods = clazz.getMethods();
for (Method method : methods) {
int mod = method.getModifiers();
if (Modifier.isPublic(mod) && Modifier.isStatic(mod)
- &&
ClassIntrospector.isMethodExposed(classMemberAccessPolicy, method)) {
+ && effClassMemberAccessPolicy.isMethodExposed(method))
{
String name = method.getName();
Object obj = map.get(name);
if (obj instanceof Method) {
