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
commit cbb4425ccfb8c1befaddaccec76e14a7f2416e25 Author: ddekany <[email protected]> AuthorDate: Thu Dec 19 00:40:49 2019 +0100 Forward ported from 2.3-gae: Added freemarker.ext.beans.MemberAccessPolicy interface, and the memberAccessPolicy property to BeansWrapper, and subclasses like DefaultObjectWrapper. This allows users to implement their own program logic to decide what members of classes will be exposed to the templates. The legacy "unsafe methods" mechanism also builds on the same now, and by setting a custom MemberAccessPolicy you completely replace that. --- ...DefaultObjectWrapperMemberAccessPolicyTest.java | 403 +++++++++++++++++++++ .../core/model/impl/ClassIntrospector.java | 142 ++++++-- .../core/model/impl/ClassMemberAccessPolicy.java | 36 ++ ...Methods.java => DefaultMemberAccessPolicy.java} | 64 +++- .../core/model/impl/DefaultObjectWrapper.java | 19 +- .../core/model/impl/MemberAccessPolicy.java | 34 ++ .../freemarker/core/model/impl/StaticModel.java | 4 +- 7 files changed, 654 insertions(+), 48 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 new file mode 100644 index 0000000..0343049 --- /dev/null +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapperMemberAccessPolicyTest.java @@ -0,0 +1,403 @@ +/* + * 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 static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; + +import org.apache.freemarker.core.Configuration; +import org.apache.freemarker.core.TemplateException; +import org.apache.freemarker.core.model.ObjectWrapperAndUnwrapper; +import org.apache.freemarker.core.model.TemplateFunctionModel; +import org.apache.freemarker.core.model.TemplateHashModel; +import org.apache.freemarker.core.model.TemplateModel; +import org.junit.Test; + +public class DefaultObjectWrapperMemberAccessPolicyTest { + + @Test + public void testMethodsWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper ow = createDefaultMemberAccessPolicyObjectWrapper(); + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + + assertNotNull(objM.get("m1")); + assertEquals("m2(true)", exec(ow, objM.get("m2"), true)); + assertEquals("staticM()", exec(ow, objM.get("staticM"))); + + assertEquals("x", getHashValue(ow, objM, "x")); + assertNotNull(objM.get("getX")); + assertNotNull(objM.get("setX")); + + assertNull(objM.get("notPublic")); + + assertNull(objM.get("notify")); + + // Because it was overridden, we allow it historically. + assertNotNull(objM.get("run")); + + assertEquals("safe wait(1)", exec(ow, objM.get("wait"), 1L)); + try { + exec(ow, objM.get("wait")); // 0 arg overload is not visible, a it's "unsafe" + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString("wait(int)")); + } + } + + @Test + public void testFieldsWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper ow = createDefaultMemberAccessPolicyObjectWrapper(); + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + assertFieldsNotExposed(objM); + } + + private void assertFieldsNotExposed(TemplateHashModel objM) throws TemplateException { + assertNull(objM.get("publicField1")); + assertNull(objM.get("publicField2")); + assertNonPublicFieldsNotExposed(objM); + } + + private void assertNonPublicFieldsNotExposed(TemplateHashModel objM) throws TemplateException { + assertNull(objM.get("nonPublicField1")); + assertNull(objM.get("nonPublicField2")); + + // Strangely, public static fields are banned historically, while static methods aren't. + assertNull(objM.get("STATIC_FIELD")); + } + + @Test + public void testGenericGetWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper ow = createDefaultMemberAccessPolicyObjectWrapper(); + + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new CWithGenericGet()); + + assertEquals("get(x)", getHashValue(ow, objM, "x")); + } + + @Test + public void testConstructorsWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper ow = createDefaultMemberAccessPolicyObjectWrapper(); + assertNonPublicConstructorNotExposed(ow); + + assertEquals(CWithConstructor.class, + ow.newInstance(CWithConstructor.class, new TemplateModel[0], null) + .getClass()); + + assertEquals(CWithOverloadedConstructor.class, + ow.newInstance(CWithOverloadedConstructor.class, new TemplateModel[0], null) + .getClass()); + + assertEquals(CWithOverloadedConstructor.class, + ow.newInstance(CWithOverloadedConstructor.class, new TemplateModel[] {new SimpleNumber(1)}, null) + .getClass()); + } + + private void assertNonPublicConstructorNotExposed(DefaultObjectWrapper ow) { + try { + ow.newInstance(C.class, new TemplateModel[0], null); + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString("constructor")); + } + } + + @Test + public void testExposeAllWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setExposureLevel(DefaultObjectWrapper.EXPOSE_ALL); + DefaultObjectWrapper ow = owb.build(); + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + // Because the MemberAccessPolicy is ignored: + assertNotNull(objM.get("notify")); + assertFieldsNotExposed(objM); + } + + @Test + public void testExposeFieldsWithDefaultMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setExposeFields(true); + DefaultObjectWrapper ow = owb.build(); + { + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + assertNull(objM.get("notify")); + assertEquals(1, getHashValue(ow, objM, "publicField1")); + assertEquals(2, getHashValue(ow, objM, "publicField2")); + assertNonPublicFieldsNotExposed(objM); + } + + { + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new CExtended()); + assertNull(objM.get("notify")); + assertEquals(1, getHashValue(ow, objM, "publicField1")); + assertEquals(2, getHashValue(ow, objM, "publicField2")); + assertEquals(3, getHashValue(ow, objM, "publicField3")); + assertNonPublicFieldsNotExposed(objM); + } + } + + @Test + public void testMethodsWithCustomMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setMemberAccessPolicy(new MemberAccessPolicy() { + public ClassMemberAccessPolicy forClass(Class<?> containingClass) { + return new ClassMemberAccessPolicy() { + public boolean isMethodExposed(Method method) { + String name = method.getName(); + Class<?>[] paramTypes = method.getParameterTypes(); + return name.equals("m3") + || (name.equals("m2") + && (paramTypes.length == 0 || paramTypes[0].equals(boolean.class))); + } + + public boolean isConstructorExposed(Constructor<?> constructor) { + return true; + } + + public boolean isFieldExposed(Field field) { + return true; + } + }; + } + }); + DefaultObjectWrapper ow = owb.build(); + + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + assertNull(objM.get("m1")); + assertEquals("m3()", exec(ow, objM.get("m3"))); + assertEquals("m2()", exec(ow, objM.get("m2"))); + assertEquals("m2(true)", exec(ow, objM.get("m2"), true)); + try { + exec(ow, objM.get("m2"), 1); + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString("overload")); + } + + assertNull(objM.get("notify")); + } + + @Test + public void testFieldsWithCustomMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setExposeFields(true); + owb.setMemberAccessPolicy(new MemberAccessPolicy() { + public ClassMemberAccessPolicy forClass(Class<?> containingClass) { + return new ClassMemberAccessPolicy() { + public boolean isMethodExposed(Method method) { + return true; + } + + public boolean isConstructorExposed(Constructor<?> constructor) { + return true; + } + + public boolean isFieldExposed(Field field) { + return field.getName().equals("publicField1") + || field.getName().equals("nonPublicField1"); + } + }; + } + }); + DefaultObjectWrapper ow = owb.build(); + + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new C()); + + assertNonPublicFieldsNotExposed(objM); + assertEquals(1, getHashValue(ow, objM, "publicField1")); + assertNull(getHashValue(ow, objM, "publicField2")); + } + + @Test + public void testGenericGetWithCustomMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setMemberAccessPolicy(new MemberAccessPolicy() { + public ClassMemberAccessPolicy forClass(Class<?> containingClass) { + return new ClassMemberAccessPolicy() { + public boolean isMethodExposed(Method method) { + return false; + } + + public boolean isConstructorExposed(Constructor<?> constructor) { + return true; + } + + public boolean isFieldExposed(Field field) { + return true; + } + }; + } + }); + DefaultObjectWrapper ow = owb.build(); + + TemplateHashModel objM = (TemplateHashModel) ow.wrap(new CWithGenericGet()); + assertNull(getHashValue(ow, objM, "x")); + } + + @Test + public void testConstructorsWithCustomMemberAccessPolicy() throws TemplateException { + DefaultObjectWrapper.Builder owb = new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0); + owb.setMemberAccessPolicy(new MemberAccessPolicy() { + public ClassMemberAccessPolicy forClass(Class<?> containingClass) { + return new ClassMemberAccessPolicy() { + public boolean isMethodExposed(Method method) { + return true; + } + + public boolean isConstructorExposed(Constructor<?> constructor) { + return constructor.getDeclaringClass() == CWithOverloadedConstructor.class + && constructor.getParameterTypes().length == 1; + } + + public boolean isFieldExposed(Field field) { + return true; + } + }; + } + }); + DefaultObjectWrapper ow = owb.build(); + + assertNonPublicConstructorNotExposed(ow); + + try { + assertEquals(CWithConstructor.class, + ow.newInstance(CWithConstructor.class, new TemplateModel[0], null).getClass()); + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString("constructor")); + } + + try { + ow.newInstance(CWithOverloadedConstructor.class, new TemplateModel[0], null); + fail(); + } catch (TemplateException e) { + assertThat(e.getMessage(), containsString("constructor")); + } + + assertEquals(CWithOverloadedConstructor.class, + ow.newInstance(CWithOverloadedConstructor.class, + new TemplateModel[] {new SimpleNumber(1)}, null).getClass()); + } + + private static DefaultObjectWrapper createDefaultMemberAccessPolicyObjectWrapper() { + return new DefaultObjectWrapper.Builder(Configuration.VERSION_3_0_0).build(); + } + + private static Object getHashValue(ObjectWrapperAndUnwrapper ow, TemplateHashModel objM, String key) + throws TemplateException { + return ow.unwrap(objM.get(key)); + } + + private static Object exec(ObjectWrapperAndUnwrapper ow, TemplateModel objM, Object... args) throws TemplateException { + assertThat(objM, instanceOf(TemplateFunctionModel.class)); + TemplateModel[] argModels = new TemplateModel[args.length]; + for (int i = 0; i < args.length; i++) { + argModels[i] = ow.wrap(args[i]); + } + Object returnValue = ((TemplateFunctionModel) objM).execute(argModels, null, null); + return unwrap(ow, returnValue); + } + + private static Object unwrap(ObjectWrapperAndUnwrapper ow, Object returnValue) throws TemplateException { + return returnValue instanceof TemplateModel ? ow.unwrap((TemplateModel) returnValue) : returnValue; + } + + public static class C extends Thread { + public static final int STATIC_FIELD = 1; + public int publicField1 = 1; + public int publicField2 = 2; + protected int nonPublicField1 = 1; + private int nonPublicField2 = 2; + + // Non-public + C() { + + } + + void notPublic() { + } + + public void m1() { + } + + public String m2() { + return "m2()"; + } + + public String m2(int otherOverload) { + return "m2(" + otherOverload + ")"; + } + + public String m2(boolean otherOverload) { + return "m2(" + otherOverload + ")"; + } + + public String m3() { + return "m3()"; + } + + public static String staticM() { + return "staticM()"; + } + + public String getX() { + return "x"; + } + + public void setX(String x) { + } + + public String wait(int otherOverload) { + return "safe wait(" + otherOverload + ")"; + } + + @Override + public void run() { + return; + } + } + + public static class CExtended extends C { + public int publicField3 = 3; + } + + public static class CWithGenericGet extends Thread { + public String get(String key) { + return "get(" + key + ")"; + } + } + + public static class CWithConstructor implements TemplateModel { + public CWithConstructor() { + } + } + + public static class CWithOverloadedConstructor implements TemplateModel { + public CWithOverloadedConstructor() { + } + + public CWithOverloadedConstructor(int x) { + } + } + +} 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 951d379..b0676d6 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 @@ -47,7 +47,9 @@ import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.apache.freemarker.core.Configuration; import org.apache.freemarker.core.Version; +import org.apache.freemarker.core._CoreAPI; import org.apache.freemarker.core.util.BugException; import org.apache.freemarker.core.util.CommonBuilder; import org.apache.freemarker.core.util._JavaVersions; @@ -130,8 +132,10 @@ class ClassIntrospector { final int exposureLevel; final boolean exposeFields; + final MemberAccessPolicy memberAccessPolicy; final MethodAppearanceFineTuner methodAppearanceFineTuner; final MethodSorter methodSorter; + final Version incompatibleImprovements; /** See {@link #getHasSharedInstanceRestrictions()} */ final private boolean hasSharedInstanceRestrictions; @@ -170,8 +174,10 @@ class ClassIntrospector { exposureLevel = builder.getExposureLevel(); exposeFields = builder.getExposeFields(); + memberAccessPolicy = builder.getMemberAccessPolicy(); methodAppearanceFineTuner = builder.getMethodAppearanceFineTuner(); methodSorter = builder.getMethodSorter(); + this.incompatibleImprovements = builder.getIncompatibleImprovements(); this.sharedLock = sharedLock; @@ -245,25 +251,26 @@ class ClassIntrospector { */ private Map<Object, Object> createClassIntrospectionData(Class<?> clazz) { final Map<Object, Object> introspData = new HashMap<>(); + ClassMemberAccessPolicy classMemberAccessPolicy = getClassMemberAccessPolicyIfNotIgnored(clazz); if (exposeFields) { - addFieldsToClassIntrospectionData(introspData, clazz); + addFieldsToClassIntrospectionData(introspData, clazz, classMemberAccessPolicy); } final Map<MethodSignature, List<Method>> accessibleMethods = discoverAccessibleMethods(clazz); - addGenericGetToClassIntrospectionData(introspData, accessibleMethods); + addGenericGetToClassIntrospectionData(introspData, accessibleMethods, classMemberAccessPolicy); if (exposureLevel != DefaultObjectWrapper.EXPOSE_NOTHING) { try { - addBeanInfoToClassIntrospectionData(introspData, clazz, accessibleMethods); + addBeanInfoToClassIntrospectionData(introspData, clazz, accessibleMethods, classMemberAccessPolicy); } 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); + addConstructorsToClassIntrospectionData(introspData, clazz, classMemberAccessPolicy); if (introspData.size() > 1) { return introspData; @@ -275,18 +282,20 @@ class ClassIntrospector { } } - private void addFieldsToClassIntrospectionData(Map<Object, Object> introspData, Class<?> clazz) - throws SecurityException { + private void addFieldsToClassIntrospectionData(Map<Object, Object> introspData, Class<?> clazz, + ClassMemberAccessPolicy classMemberAccessPolicy) throws SecurityException { for (Field field : clazz.getFields()) { if ((field.getModifiers() & Modifier.STATIC) == 0) { - introspData.put(field.getName(), field); + if (classMemberAccessPolicy == null || classMemberAccessPolicy.isFieldExposed(field)) { + introspData.put(field.getName(), field); + } } } } private void addBeanInfoToClassIntrospectionData( - Map<Object, Object> introspData, Class<?> clazz, Map<MethodSignature, List<Method>> accessibleMethods) - throws IntrospectionException { + Map<Object, Object> introspData, Class<?> clazz, Map<MethodSignature, List<Method>> accessibleMethods, + ClassMemberAccessPolicy classMemberAccessPolicy) throws IntrospectionException { BeanInfo beanInfo = Introspector.getBeanInfo(clazz); List<PropertyDescriptor> pdas = getPropertyDescriptors(beanInfo, clazz); int pdasLength = pdas.size(); @@ -294,7 +303,7 @@ class ClassIntrospector { for (int i = pdasLength - 1; i >= 0; --i) { addPropertyDescriptorToClassIntrospectionData( introspData, pdas.get(i), clazz, - accessibleMethods); + accessibleMethods, classMemberAccessPolicy); } if (exposureLevel < DefaultObjectWrapper.EXPOSE_PROPERTIES_ONLY) { @@ -306,7 +315,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 && isAllowedToExpose(method)) { + if (method != null && isMethodExposed(classMemberAccessPolicy, method)) { decision.setDefaults(method); if (methodAppearanceFineTuner != null) { if (decisionInput == null) { @@ -323,7 +332,7 @@ class ClassIntrospector { (decision.getReplaceExistingProperty() || !(introspData.get(propDesc.getName()) instanceof FastPropertyDescriptor))) { addPropertyDescriptorToClassIntrospectionData( - introspData, propDesc, clazz, accessibleMethods); + introspData, propDesc, clazz, accessibleMethods, classMemberAccessPolicy); } String methodKey = decision.getExposeMethodAs(); @@ -632,39 +641,51 @@ class ClassIntrospector { } private void addPropertyDescriptorToClassIntrospectionData(Map<Object, Object> introspData, - PropertyDescriptor pd, Class<?> clazz, Map<MethodSignature, List<Method>> accessibleMethods) { + PropertyDescriptor pd, Class<?> clazz, Map<MethodSignature, List<Method>> accessibleMethods, + ClassMemberAccessPolicy classMemberAccessPolicy) { Method readMethod = getMatchingAccessibleMethod(pd.getReadMethod(), accessibleMethods); - if (readMethod != null && isAllowedToExpose(readMethod)) { + if (readMethod != null && isMethodExposed(classMemberAccessPolicy, readMethod)) { introspData.put(pd.getName(), new FastPropertyDescriptor(readMethod)); } } private void addGenericGetToClassIntrospectionData(Map<Object, Object> introspData, - Map<MethodSignature, List<Method>> accessibleMethods) { + Map<MethodSignature, List<Method>> accessibleMethods, ClassMemberAccessPolicy classMemberAccessPolicy) { Method genericGet = getFirstAccessibleMethod( MethodSignature.GET_STRING_SIGNATURE, accessibleMethods); if (genericGet == null) { genericGet = getFirstAccessibleMethod( MethodSignature.GET_OBJECT_SIGNATURE, accessibleMethods); } - if (genericGet != null) { + if (genericGet != null && isMethodExposed(classMemberAccessPolicy, genericGet)) { introspData.put(GENERIC_GET_KEY, genericGet); } } private void addConstructorsToClassIntrospectionData(final Map<Object, Object> introspData, - Class<?> clazz) { + Class<?> clazz, ClassMemberAccessPolicy classMemberAccessPolicy) { try { - Constructor<?>[] ctors = clazz.getConstructors(); - if (ctors.length == 1) { - Constructor<?> ctor = ctors[0]; - introspData.put(CONSTRUCTORS_KEY, new SimpleMethod(ctor, ctor.getParameterTypes())); - } else if (ctors.length > 1) { - OverloadedMethods overloadedCtors = new OverloadedMethods(); - for (Constructor<?> ctor : ctors) { - overloadedCtors.addConstructor(ctor); + Constructor<?>[] ctorsUnfiltered = clazz.getConstructors(); + List<Constructor<?>> ctors = new ArrayList<Constructor<?>>(ctorsUnfiltered.length); + for (Constructor<?> ctor : ctorsUnfiltered) { + if (classMemberAccessPolicy == null || classMemberAccessPolicy.isConstructorExposed(ctor)) { + ctors.add(ctor); + } + } + + if (!ctors.isEmpty()) { + final Object ctorsIntrospData; + if (ctors.size() == 1) { + Constructor<?> ctor = ctors.get(0); + ctorsIntrospData = new SimpleMethod(ctor, ctor.getParameterTypes()); + } else { + OverloadedMethods overloadedCtors = new OverloadedMethods(); + for (Constructor<?> ctor : ctors) { + overloadedCtors.addConstructor(ctor); + } + ctorsIntrospData = overloadedCtors; } - introspData.put(CONSTRUCTORS_KEY, overloadedCtors); + introspData.put(CONSTRUCTORS_KEY, ctorsIntrospData); } } catch (SecurityException e) { LOG.warn("Can't discover constructors for class {}", clazz.getName(), e); @@ -759,8 +780,24 @@ class ClassIntrospector { } } - boolean isAllowedToExpose(Method method) { - return exposureLevel < DefaultObjectWrapper.EXPOSE_SAFE || !UnsafeMethods.isUnsafeMethod(method); + /** + * 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) + */ + 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); } private static Map<Method, Class<?>[]> getArgTypesByMethod(Map<Object, Object> classInfo) { @@ -980,6 +1017,10 @@ class ClassIntrospector { return exposeFields; } + MemberAccessPolicy getMemberAccessPolicy() { + return memberAccessPolicy; + } + MethodAppearanceFineTuner getMethodAppearanceFineTuner() { return methodAppearanceFineTuner; } @@ -1028,6 +1069,8 @@ class ClassIntrospector { private static final Map<Builder, Reference<ClassIntrospector>> INSTANCE_CACHE = new HashMap<>(); private static final ReferenceQueue<ClassIntrospector> INSTANCE_CACHE_REF_QUEUE = new ReferenceQueue<>(); + private final Version incompatibleImprovements; + // Properties and their *defaults*: private boolean sharingDisallowed; private boolean shardingDisallowedSet; @@ -1035,6 +1078,8 @@ class ClassIntrospector { private boolean exposureLevelSet; private boolean exposeFields; private boolean exposeFieldsSet; + private MemberAccessPolicy memberAccessPolicy; + private boolean memberAccessPolicySet; private MethodAppearanceFineTuner methodAppearanceFineTuner; private boolean methodAppearanceFineTunerSet; private MethodSorter methodSorter; @@ -1048,10 +1093,17 @@ class ClassIntrospector { Builder(Version incompatibleImprovements) { // Warning: incompatibleImprovements must not affect this object at versions increments where there's no - // change in the DefaultObjectWrapper.normalizeIncompatibleImprovements results. That is, this class may don't react - // to some version changes that affects DefaultObjectWrapper, but not the other way around. - _NullArgumentException.check(incompatibleImprovements); + // change in the DefaultObjectWrapper.normalizeIncompatibleImprovements results. That is, this class may + // don't react to some version changes that affects DefaultObjectWrapper, but not the other way around. + this.incompatibleImprovements = normalizeIncompatibleImprovementsVersion(incompatibleImprovements); // Currently nothing depends on incompatibleImprovements + memberAccessPolicy = DefaultMemberAccessPolicy.getInstance(this.incompatibleImprovements); + } + + private static Version normalizeIncompatibleImprovementsVersion(Version incompatibleImprovements) { + _CoreAPI.checkVersionNotNullAndSupported(incompatibleImprovements); + // All breakpoints here must occur in DefaultObjectWrapper.normalizeIncompatibleImprovements! + return Configuration.VERSION_3_0_0; } @Override @@ -1067,9 +1119,11 @@ class ClassIntrospector { public int hashCode() { final int prime = 31; int result = 1; + result = prime * result + incompatibleImprovements.hashCode(); result = prime * result + (sharingDisallowed ? 1231 : 1237); result = prime * result + (exposeFields ? 1231 : 1237); result = prime * result + exposureLevel; + result = prime * result + memberAccessPolicy.hashCode(); result = prime * result + System.identityHashCode(methodAppearanceFineTuner); result = prime * result + System.identityHashCode(methodSorter); return result; @@ -1082,9 +1136,11 @@ class ClassIntrospector { if (getClass() != obj.getClass()) return false; Builder other = (Builder) obj; + if (!incompatibleImprovements.equals(other.incompatibleImprovements)) return false; if (sharingDisallowed != other.sharingDisallowed) return false; if (exposeFields != other.exposeFields) return false; if (exposureLevel != other.exposureLevel) return false; + if (!memberAccessPolicy.equals(other.memberAccessPolicy)) return false; if (methodAppearanceFineTuner != other.methodAppearanceFineTuner) return false; return methodSorter == other.methodSorter; } @@ -1150,6 +1206,23 @@ class ClassIntrospector { return exposeFieldsSet; } + public MemberAccessPolicy getMemberAccessPolicy() { + return memberAccessPolicy; + } + + public void setMemberAccessPolicy(MemberAccessPolicy memberAccessPolicy) { + _NullArgumentException.check(memberAccessPolicy); + this.memberAccessPolicy = memberAccessPolicy; + memberAccessPolicySet = true; + } + + /** + * Tells if the property was explicitly set, as opposed to just holding its default value. + */ + public boolean isMemberAccessPolicySet() { + return memberAccessPolicySet; + } + public MethodAppearanceFineTuner getMethodAppearanceFineTuner() { return methodAppearanceFineTuner; } @@ -1174,6 +1247,13 @@ class ClassIntrospector { this.methodSorter = methodSorter; } + /** + * Returns the normalized incompatible improvements. + */ + public Version getIncompatibleImprovements() { + return incompatibleImprovements; + } + private static void removeClearedReferencesFromInstanceCache() { Reference<? extends ClassIntrospector> clearedRef; while ((clearedRef = INSTANCE_CACHE_REF_QUEUE.poll()) != null) { diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassMemberAccessPolicy.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassMemberAccessPolicy.java new file mode 100644 index 0000000..c57a711 --- /dev/null +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/ClassMemberAccessPolicy.java @@ -0,0 +1,36 @@ +/* + * 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; + +/** + * Returned by {@link MemberAccessPolicy#forClass(Class)}. The idea is that {@link MemberAccessPolicy#forClass(Class)} + * is called once per class, and then the methods of the resulting {@link ClassMemberAccessPolicy} object will be + * called for each member of the class. This can speed up the process as the class-specific lookups will be done only + * once per class, not once per member. + */ +public interface ClassMemberAccessPolicy { + boolean isMethodExposed(Method method); + boolean isConstructorExposed(Constructor<?> constructor); + boolean isFieldExposed(Field field); +} diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/UnsafeMethods.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java similarity index 62% rename from freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/UnsafeMethods.java rename to freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java index 54771cd..6899efe 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/UnsafeMethods.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultMemberAccessPolicy.java @@ -19,6 +19,8 @@ package org.apache.freemarker.core.model.impl; +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.util.HashMap; import java.util.HashSet; @@ -27,22 +29,21 @@ import java.util.Properties; import java.util.Set; import java.util.StringTokenizer; +import org.apache.freemarker.core.Version; +import org.apache.freemarker.core._CoreAPI; import org.apache.freemarker.core.util._ClassUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class UnsafeMethods { - - private static final Logger LOG = LoggerFactory.getLogger(UnsafeMethods.class); +/** + * Legacy black list based member access policy, used only to keep old behavior, as it can't provide meaningful safety. + * Do not use it if you allow untrusted users to edit templates! + */ +public final class DefaultMemberAccessPolicy implements MemberAccessPolicy { + private static final Logger LOG = LoggerFactory.getLogger(DefaultMemberAccessPolicy.class); private static final String UNSAFE_METHODS_PROPERTIES = "unsafeMethods.properties"; private static final Set<Method> UNSAFE_METHODS = createUnsafeMethodsSet(); - - private UnsafeMethods() { } - - static boolean isUnsafeMethod(Method method) { - return UNSAFE_METHODS.contains(method); - } - + private static Set<Method> createUnsafeMethodsSet() { try { Properties props = _ClassUtils.loadProperties(DefaultObjectWrapper.class, UNSAFE_METHODS_PROPERTIES); @@ -62,16 +63,16 @@ class UnsafeMethods { } private static Method parseMethodSpec(String methodSpec, Map<String, Class<?>> primClasses) - throws ClassNotFoundException, - NoSuchMethodException { + throws ClassNotFoundException, + NoSuchMethodException { int brace = methodSpec.indexOf('('); int dot = methodSpec.lastIndexOf('.', brace); - Class clazz = _ClassUtils.forName(methodSpec.substring(0, dot)); + Class<?> clazz = _ClassUtils.forName(methodSpec.substring(0, dot)); String methodName = methodSpec.substring(dot + 1, brace); String argSpec = methodSpec.substring(brace + 1, methodSpec.length() - 1); StringTokenizer tok = new StringTokenizer(argSpec, ","); int argcount = tok.countTokens(); - Class[] argTypes = new Class[argcount]; + Class<?>[] argTypes = new Class[argcount]; for (int i = 0; i < argcount; i++) { String argClassName = tok.nextToken(); argTypes[i] = primClasses.get(argClassName); @@ -95,4 +96,39 @@ class UnsafeMethods { return map; } + private DefaultMemberAccessPolicy() { + } + + private static final DefaultMemberAccessPolicy INSTANCE = new DefaultMemberAccessPolicy(); + + /** + * Returns the singleton that's compatible with the given incompatible improvements version. + */ + public static DefaultMemberAccessPolicy getInstance(Version incompatibleImprovements) { + _CoreAPI.checkVersionNotNullAndSupported(incompatibleImprovements); + // All breakpoints here must occur in ClassIntrospectorBuilder.normalizeIncompatibleImprovementsVersion! + // Though currently we don't have any. + return INSTANCE; + } + + public ClassMemberAccessPolicy forClass(Class<?> containingClass) { + return CLASS_MEMBER_ACCESS_POLICY_INSTANCE; + } + + private static final BacklistClassMemberAccessPolicy CLASS_MEMBER_ACCESS_POLICY_INSTANCE + = new BacklistClassMemberAccessPolicy(); + private static class BacklistClassMemberAccessPolicy implements ClassMemberAccessPolicy { + + public boolean isMethodExposed(Method method) { + return !UNSAFE_METHODS.contains(method); + } + + public boolean isConstructorExposed(Constructor<?> constructor) { + return true; + } + + public boolean isFieldExposed(Field field) { + return true; + } + } } diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java index e3474b1..7a9fe04 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/DefaultObjectWrapper.java @@ -93,7 +93,8 @@ public class DefaultObjectWrapper implements RichObjectWrapper { /** * At this level of exposure, all methods and properties of the - * wrapped objects are exposed to the template. + * wrapped objects are exposed to the template, and the {@link MemberAccessPolicy} + * will be ignored. */ public static final int EXPOSE_ALL = 0; @@ -1187,7 +1188,6 @@ public class DefaultObjectWrapper implements RichObjectWrapper { return Configuration.VERSION_3_0_0; } - /** * Returns the name-value pairs that describe the configuration of this {@link DefaultObjectWrapper}; called from * {@link #toString()}. The expected format is like {@code "foo=bar, baaz=wombat"}. When overriding this, you should @@ -1803,6 +1803,21 @@ public class DefaultObjectWrapper implements RichObjectWrapper { return classIntrospectorBuilder.isExposeFieldsSet(); } + public MemberAccessPolicy getMemberAccessPolicy() { + return classIntrospectorBuilder.getMemberAccessPolicy(); + } + + public void setMemberAccessPolicy(MemberAccessPolicy memberAccessPolicy) { + classIntrospectorBuilder.setMemberAccessPolicy(memberAccessPolicy); + } + + /** + * Tells if the property was explicitly set, as opposed to just holding its default value. + */ + public boolean isMemberAccessPolicy() { + return classIntrospectorBuilder.isMemberAccessPolicySet(); + } + /** * Getter pair of {@link #setMethodAppearanceFineTuner(MethodAppearanceFineTuner)} */ diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberAccessPolicy.java b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberAccessPolicy.java new file mode 100644 index 0000000..27be4f0 --- /dev/null +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/impl/MemberAccessPolicy.java @@ -0,0 +1,34 @@ +/* + * 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; + +/** + * Implement this to specify what class members are accessible from templates. Implementations must be thread + * safe, and instances should be generally singletons on JVM level. The last is because FreeMarker tries to cache + * class introspectors in a global (static, JVM-scope) cache for reuse, and that's only possible if the + * {@link MemberAccessPolicy} instances used at different places in the JVM are equal according to + * {@link #equals(Object) (and the singleton object of course {@link #equals(Object)} with itself). + */ +public interface MemberAccessPolicy { + /** + * Returns the {@link ClassMemberAccessPolicy} that encapsulates the member access policy for a given class. + */ + ClassMemberAccessPolicy forClass(Class<?> containingClass); +} \ No newline at end of file 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 21c525c..83df66a 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 @@ -138,11 +138,13 @@ 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) - && wrapper.getClassIntrospector().isAllowedToExpose(method)) { + && ClassIntrospector.isMethodExposed(classMemberAccessPolicy, method)) { String name = method.getName(); Object obj = map.get(name); if (obj instanceof Method) {
