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 66a2704 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.
66a2704 is described below
commit 66a2704c5c04c50212edac6c68f94986829572e7
Author: ddekany <[email protected]>
AuthorDate: Tue Jan 14 07:33:57 2020 +0100
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.
---
.../ext/beans/AllowAllMemberAccessPolicy.java | 53 +++++++
.../freemarker/ext/beans/ClassIntrospector.java | 57 +++-----
.../java/freemarker/ext/beans/StaticModel.java | 11 +-
...DefaultObjectWrapperMemberAccessPolicyTest.java | 160 +++++++++++++++++++++
4 files changed, 242 insertions(+), 39 deletions(-)
diff --git a/src/main/java/freemarker/ext/beans/AllowAllMemberAccessPolicy.java
b/src/main/java/freemarker/ext/beans/AllowAllMemberAccessPolicy.java
new file mode 100644
index 0000000..dd9a1fc
--- /dev/null
+++ b/src/main/java/freemarker/ext/beans/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 freemarker.ext.beans;
+
+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/src/main/java/freemarker/ext/beans/ClassIntrospector.java
b/src/main/java/freemarker/ext/beans/ClassIntrospector.java
index 630bf95..aa3bafb 100644
--- a/src/main/java/freemarker/ext/beans/ClassIntrospector.java
+++ b/src/main/java/freemarker/ext/beans/ClassIntrospector.java
@@ -272,26 +272,26 @@ class ClassIntrospector {
*/
private Map<Object, Object> createClassIntrospectionData(Class<?> clazz) {
final Map<Object, Object> introspData = new HashMap<Object, Object>();
- 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 != BeansWrapper.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, e);
introspData.clear(); // FIXME NBC: Don't drop everything here.
}
}
- addConstructorsToClassIntrospectionData(introspData, clazz,
classMemberAccessPolicy);
+ addConstructorsToClassIntrospectionData(introspData, clazz,
effClassMemberAccessPolicy);
if (introspData.size() > 1) {
return introspData;
@@ -304,12 +304,12 @@ class ClassIntrospector {
}
private void addFieldsToClassIntrospectionData(Map<Object, Object>
introspData, Class<?> clazz,
- ClassMemberAccessPolicy classMemberAccessPolicy) throws
SecurityException {
+ ClassMemberAccessPolicy effClassMemberAccessPolicy) throws
SecurityException {
Field[] fields = clazz.getFields();
for (int i = 0; i < fields.length; i++) {
Field field = fields[i];
if ((field.getModifiers() & Modifier.STATIC) == 0) {
- if (classMemberAccessPolicy == null ||
classMemberAccessPolicy.isFieldExposed(field)) {
+ if (effClassMemberAccessPolicy.isFieldExposed(field)) {
introspData.put(field.getName(), field);
}
}
@@ -319,7 +319,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();
@@ -327,7 +327,7 @@ class ClassIntrospector {
for (int i = pdasLength - 1; i >= 0; --i) {
addPropertyDescriptorToClassIntrospectionData(
introspData, pdas.get(i),
- accessibleMethods, classMemberAccessPolicy);
+ accessibleMethods, effClassMemberAccessPolicy);
}
if (exposureLevel < BeansWrapper.EXPOSE_PROPERTIES_ONLY) {
@@ -339,7 +339,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) {
@@ -356,7 +356,7 @@ class ClassIntrospector {
(decision.getReplaceExistingProperty()
|| !(introspData.get(propDesc.getName())
instanceof FastPropertyDescriptor))) {
addPropertyDescriptorToClassIntrospectionData(
- introspData, propDesc, accessibleMethods,
classMemberAccessPolicy);
+ introspData, propDesc, accessibleMethods,
effClassMemberAccessPolicy);
}
String methodKey = decision.getExposeMethodAs();
@@ -667,9 +667,9 @@ class ClassIntrospector {
private void addPropertyDescriptorToClassIntrospectionData(Map<Object,
Object> introspData,
PropertyDescriptor pd,
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)) {
readMethod = null;
}
@@ -677,7 +677,7 @@ class ClassIntrospector {
if (pd instanceof IndexedPropertyDescriptor) {
indexedReadMethod = getMatchingAccessibleMethod(
((IndexedPropertyDescriptor) pd).getIndexedReadMethod(),
accessibleMethods);
- if (indexedReadMethod != null &&
!isMethodExposed(classMemberAccessPolicy, indexedReadMethod)) {
+ if (indexedReadMethod != null &&
!effClassMemberAccessPolicy.isMethodExposed(indexedReadMethod)) {
indexedReadMethod = null;
}
if (indexedReadMethod != null) {
@@ -695,23 +695,23 @@ class ClassIntrospector {
private void addGenericGetToClassIntrospectionData(Map<Object, Object>
introspData,
Map<ExecutableMemberSignature, List<Method>> accessibleMethods,
- ClassMemberAccessPolicy classMemberAccessPolicy) {
+ 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);
}
}
@@ -828,23 +828,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 BeansWrapper#getMemberAccessPolicy()} if {@link
BeansWrapper#getExposureLevel()} is more allowing than
+ * {@link BeansWrapper#EXPOSE_SAFE}. {@link BeansWrapper#EXPOSE_NOTHING}
though is not factored in here.
*/
- ClassMemberAccessPolicy getClassMemberAccessPolicyIfNotIgnored(Class
containingClass) {
- return exposureLevel < BeansWrapper.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 < BeansWrapper.EXPOSE_SAFE ?
AllowAllMemberAccessPolicy.CLASS_POLICY_INSTANCE :
memberAccessPolicy.forClass(containingClass);
}
private boolean is2321Bugfixed() {
diff --git a/src/main/java/freemarker/ext/beans/StaticModel.java
b/src/main/java/freemarker/ext/beans/StaticModel.java
index fc7504d..cd1bf97 100644
--- a/src/main/java/freemarker/ext/beans/StaticModel.java
+++ b/src/main/java/freemarker/ext/beans/StaticModel.java
@@ -106,10 +106,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
@@ -127,14 +130,12 @@ final class StaticModel implements TemplateHashModelEx {
}
}
if (wrapper.getExposureLevel() < BeansWrapper.EXPOSE_PROPERTIES_ONLY) {
- ClassMemberAccessPolicy classMemberAccessPolicy =
-
wrapper.getClassIntrospector().getClassMemberAccessPolicyIfNotIgnored(clazz);
Method[] methods = clazz.getMethods();
for (int i = 0; i < methods.length; ++i) {
Method method = methods[i];
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) {
@@ -149,7 +150,7 @@ final class StaticModel implements TemplateHashModelEx {
if (obj != null) {
if (LOG.isInfoEnabled()) {
LOG.info("Overwriting value [" + obj + "] for
" +
- " key '" + name + "' with [" + method
+
+ " key '" + name + "' with [" + method +
"] in static model for " +
clazz.getName());
}
}
diff --git
a/src/test/java/freemarker/ext/beans/DefaultObjectWrapperMemberAccessPolicyTest.java
b/src/test/java/freemarker/ext/beans/DefaultObjectWrapperMemberAccessPolicyTest.java
index f9fcf47..0938541 100644
---
a/src/test/java/freemarker/ext/beans/DefaultObjectWrapperMemberAccessPolicyTest.java
+++
b/src/test/java/freemarker/ext/beans/DefaultObjectWrapperMemberAccessPolicyTest.java
@@ -152,6 +152,56 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
}
@Test
+ public void testExposeAllWithCustomMemberAccessPolicy() throws
TemplateModelException {
+ {
+ DefaultObjectWrapperBuilder owb = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30);
+ owb.setExposureLevel(DefaultObjectWrapper.EXPOSE_ALL);
+ owb.setExposeFields(true);
+ owb.setMemberAccessPolicy(AllowNothingMemberAccessPolicy.INSTANCE);
+ DefaultObjectWrapper ow = owb.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"));
+ }
+ {
+ DefaultObjectWrapperBuilder owb = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30);
+ owb.setExposeFields(true);
+ owb.setMemberAccessPolicy(AllowNothingMemberAccessPolicy.INSTANCE);
+ DefaultObjectWrapper ow = owb.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 (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ try {
+ assertNull(staticModel.get("STATIC_FIELD"));
+ fail();
+ } catch (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+ }
+
+ @Test
public void testExposeFieldsWithDefaultMemberAccessPolicy() throws
TemplateModelException {
DefaultObjectWrapperBuilder owb = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30);
owb.setExposeFields(true);
@@ -408,6 +458,78 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
}
}
+ @Test
+ public void testMemberAccessPolicyAndStatics() throws TemplateException {
+ DefaultObjectWrapperBuilder owb = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30);
+ owb.setMemberAccessPolicy(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");
+ }
+ };
+ }
+ });
+ DefaultObjectWrapper ow = owb.build();
+ TemplateHashModel statics = (TemplateHashModel)
ow.getStaticModels().get(Statics.class.getName());
+
+ assertEquals(1, ((TemplateNumberModel)
statics.get("F1")).getAsNumber());
+ try {
+ statics.get("F2");
+ fail();
+ } catch (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+
+ assertEquals(1, ((TemplateNumberModel)
statics.get("V1")).getAsNumber());
+ try {
+ statics.get("V2");
+ fail();
+ } catch (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+
+ assertEquals(1,
+ ((TemplateNumberModel) ((TemplateMethodModelEx)
statics.get("m1"))
+ .exec(Collections.emptyList()))
+ .getAsNumber());
+ try {
+ assertNull(statics.get("m2"));
+ fail();
+ } catch (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+
+ @Test
+ public void testMemberAccessPolicyAndStatics2() throws TemplateException {
+ DefaultObjectWrapper defaultOw = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30).build();
+
+ DefaultObjectWrapperBuilder owb = new
DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_30);
+ owb.setMemberAccessPolicy(LegacyDefaultMemberAccessPolicy.INSTANCE);
+ DefaultObjectWrapper legacyDefaultOw = owb.build();
+
+ for (BeansWrapper ow : new BeansWrapper[] { defaultOw, legacyDefaultOw
}) {
+ TemplateHashModel sys = (TemplateHashModel)
ow.getStaticModels().get(System.class.getName());
+ assertNotNull(sys.get("currentTimeMillis"));
+ try {
+ sys.get("exit");
+ fail();
+ } catch (TemplateModelException e) {
+ assertThat(e.getMessage(), containsString("No such key"));
+ }
+ }
+ }
+
private static Object getHashValue(ObjectWrapperAndUnwrapper ow,
TemplateHashModel objM, String key)
throws TemplateModelException {
return ow.unwrap(objM.get(key));
@@ -475,6 +597,8 @@ public class DefaultObjectWrapperMemberAccessPolicyTest {
public String wait(int otherOverload) {
return "safe wait(" + otherOverload + ")";
}
+
+ public static void M1() { }
}
public static class CExtended extends C {
@@ -519,4 +643,40 @@ 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;
+ }
+ };
+ }
+ }
}
