freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [freemarker] branch 2.3-gae updated: 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.
Date Tue, 14 Jan 2020 07:43:46 GMT
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 <ddekany@apache.org>
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;
+                }
+            };
+        }
+    }
 }


Mime
View raw message