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
commit d921f5dd833d190adf8d1c67fed8d26114c4816f Author: ddekany <[email protected]> AuthorDate: Mon Jun 15 00:14:35 2020 +0200 [FREEMARKER-145] Fixed bug where methods with "overloaded" return type may become inaccessible on Java 9+, if some overriding subclasses are not public. (This is because java.beans.Introspector behavior has changed with Java 9.) --- .../freemarker/ext/beans/ClassIntrospector.java | 27 ++-- .../java/freemarker/ext/beans/_MethodUtil.java | 82 +++++++++++ src/manual/en_US/book.xml | 24 ++- .../ext/beans/Java9InstrospectorBugWorkaround.java | 38 +++++ .../java/freemarker/ext/beans/MethodUtilTest2.java | 164 +++++++++++++++++++++ 5 files changed, 323 insertions(+), 12 deletions(-) diff --git a/src/main/java/freemarker/ext/beans/ClassIntrospector.java b/src/main/java/freemarker/ext/beans/ClassIntrospector.java index e0c5135..32053f2 100644 --- a/src/main/java/freemarker/ext/beans/ClassIntrospector.java +++ b/src/main/java/freemarker/ext/beans/ClassIntrospector.java @@ -814,21 +814,26 @@ class ClassIntrospector { } } + // This is needed as java.bean.Introspector sometimes gives back a method that's actually not accessible, + // as it's an override of an accessible method in a non-public subclass. While that's still a public method, calling + // it directly via reflection will throw java.lang.IllegalAccessException, and we are supposed to call the overidden + // accessible method instead. Like, we migth get two PropertyDescriptor-s for the same property name, and only one + // will have a reader method that we can actually call. So we have to find that method here. + // Furthermore, the return type of the inaccisable method is possibly different (more specific) than the return type + // of the overidden accessible method. Also Introspector behavior changed with Java 9, as earlier in such case the + // Introspector returned all variants of the method (so the accessible one was amongst them at least), while in + // Java 9 it apparently always returns one variant only, but that's sometimes (not sure if it's predictable) the + // inaccessbile one. private static Method getMatchingAccessibleMethod(Method m, Map<ExecutableMemberSignature, List<Method>> accessibles) { if (m == null) { return null; } - ExecutableMemberSignature sig = new ExecutableMemberSignature(m); - List<Method> ams = accessibles.get(sig); - if (ams == null) { - return null; - } - for (Method am : ams) { - if (am.getReturnType() == m.getReturnType()) { - return am; - } - } - return null; + List<Method> ams = accessibles.get(new ExecutableMemberSignature(m)); + // Certainly we could return any of the accessbiles, as Java reflection will call the correct override of the + // method anyway. There's an ambiguity when the return type is "overloaded", but in practice it probably doesn't + // matter which variant we call. Though, technically, they could do totaly different things. So, to avoid any + // corner cases that cause problems after an upgrade, we make an effort to give same result as before 2.3.31. + return ams != null ? _MethodUtil.getMethodWithClosestNonSubReturnType(m.getReturnType(), ams) : null; } private static Method getFirstAccessibleMethod( diff --git a/src/main/java/freemarker/ext/beans/_MethodUtil.java b/src/main/java/freemarker/ext/beans/_MethodUtil.java index 1540853..99a733e 100644 --- a/src/main/java/freemarker/ext/beans/_MethodUtil.java +++ b/src/main/java/freemarker/ext/beans/_MethodUtil.java @@ -25,6 +25,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Member; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -458,4 +459,85 @@ public final class _MethodUtil { return getInheritableFieldAnnotation(superClass, fieldName, false, annotationClass); } + public static Method getMethodWithClosestNonSubReturnType( + Class<?> returnType, Collection<Method> methods) { + // Exact match: + for (Method method : methods) { + if (method.getReturnType() == returnType) { + return method; + } + } + + if (returnType == Object.class || returnType.isPrimitive()) { + // We can't go wider than these types, so we give up. Note that void is primitive. + return null; + } + + // Super-class match, which we prefer over Interface match, except if the match is just Object: + Class<?> superClass = returnType.getSuperclass(); + while (superClass != null && superClass != Object.class) { + for (Method method : methods) { + if (method.getReturnType() == superClass) { + return method; + } + } + superClass = superClass.getSuperclass(); + } + + // Interface match: + Method result = getMethodWithClosestNonSubInterfaceReturnType(returnType, methods); + if (result != null) { + return result; + } + + // As the returnType is non-primitive, Object return type will do, as a last resort: + for (Method method : methods) { + if (method.getReturnType() == Object.class) { + return method; + } + } + + return null; + } + + private static Method getMethodWithClosestNonSubInterfaceReturnType( + Class<?> returnType, Collection<Method> methods) { + HashSet<Class<?>> nullResultReturnTypeInterfaces = new HashSet<>(); + do { + Method result + = getMethodWithClosestNonSubInterfaceReturnType(returnType, methods, nullResultReturnTypeInterfaces); + if (result != null) { + return result; + } + // As Class.getInterfaces() doesn't return the inherited interfaces, we have to do this. + returnType = returnType.getSuperclass(); + } while (returnType != null); + return null; + } + + private static Method getMethodWithClosestNonSubInterfaceReturnType( + Class<?> returnType, Collection<Method> methods, Set<Class<?>> nullResultReturnTypeInterfaces) { + boolean returnTypeIsInterface = returnType.isInterface(); + if (returnTypeIsInterface) { + if (nullResultReturnTypeInterfaces.contains(returnType)) { + return null; + } + for (Method method : methods) { + if (method.getReturnType() == returnType) { + return method; + } + } + } + for (Class<?> subInterface : returnType.getInterfaces()) { + Method result = getMethodWithClosestNonSubInterfaceReturnType(subInterface, methods, nullResultReturnTypeInterfaces); + if (result != null) { + return result; + } + } + if (returnTypeIsInterface) { + nullResultReturnTypeInterfaces.add(returnType); + } + return null; + } + } \ No newline at end of file diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 1041ca7..c779e01 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -29,7 +29,7 @@ <titleabbrev>Manual</titleabbrev> - <productname>Freemarker 2.3.30</productname> + <productname>Freemarker 2.3.31</productname> </info> <preface role="index.html" xml:id="preface"> @@ -29311,6 +29311,28 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> <appendix xml:id="app_versions"> <title>Version history</title> + <section xml:id="versions_2_3_31"> + <title>2.3.31</title> + + <para>Release date: FIXME</para> + + <section> + <title>Changes on the Java side</title> + + <itemizedlist> + <listitem> + <para><link + xlink:href="https://issues.apache.org/jira/browse/FREEMARKER-145";>FREEMARKER-145</link>: + Fixed bug where methods with "overloaded" return type may become + inaccessible on Java 9+, if some overriding subclasses are not + public. (This is because + <literal>java.beans.Introspector</literal> behavior has changed + with Java 9.)</para> + </listitem> + </itemizedlist> + </section> + </section> + <section xml:id="versions_2_3_30"> <title>2.3.30</title> diff --git a/src/test/java/freemarker/ext/beans/Java9InstrospectorBugWorkaround.java b/src/test/java/freemarker/ext/beans/Java9InstrospectorBugWorkaround.java new file mode 100644 index 0000000..05fa695 --- /dev/null +++ b/src/test/java/freemarker/ext/beans/Java9InstrospectorBugWorkaround.java @@ -0,0 +1,38 @@ +/* + * 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.io.IOException; +import java.nio.file.Paths; + +import org.junit.Test; + +import freemarker.template.TemplateException; +import freemarker.test.TemplateTest; + +public class Java9InstrospectorBugWorkaround extends TemplateTest { + + @Test + public void test() throws IOException, TemplateException { + addToDataModel("path", Paths.get("foo", "bar")); + assertOutput("<#assign _ = path.parent>", ""); + } + +} diff --git a/src/test/java/freemarker/ext/beans/MethodUtilTest2.java b/src/test/java/freemarker/ext/beans/MethodUtilTest2.java new file mode 100644 index 0000000..9723613 --- /dev/null +++ b/src/test/java/freemarker/ext/beans/MethodUtilTest2.java @@ -0,0 +1,164 @@ +/* + * 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 static freemarker.ext.beans._MethodUtil.*; +import static org.junit.Assert.*; + +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.Test; + +public class MethodUtilTest2 { + + @Test + public void testGetMethodWithClosestNonSubReturnType1() { + List<Method> methods = getMethods(ObjectM.class, ListM.class, CollectionM.class); + assertEquals(getMethod(ObjectM.class), getMethodWithClosestNonSubReturnType(Object.class, methods)); + assertEquals(getMethod(CollectionM.class), getMethodWithClosestNonSubReturnType(Collection.class, methods)); + assertEquals(getMethod(ListM.class), getMethodWithClosestNonSubReturnType(List.class, methods)); + assertEquals(getMethod(ListM.class), getMethodWithClosestNonSubReturnType(ArrayList.class, methods)); + assertEquals(getMethod(ObjectM.class), getMethodWithClosestNonSubReturnType(String.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(int.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(void.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType2() { + List<Method> methods = getMethods(ListM.class, CollectionM.class); + assertNull(getMethodWithClosestNonSubReturnType(Object.class, methods)); + assertEquals(getMethod(CollectionM.class), getMethodWithClosestNonSubReturnType(Collection.class, methods)); + assertEquals(getMethod(ListM.class), getMethodWithClosestNonSubReturnType(List.class, methods)); + assertEquals(getMethod(ListM.class), getMethodWithClosestNonSubReturnType(ArrayList.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(String.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(int.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(void.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType3() { + List<Method> methods = getMethods(ObjectM.class, SerializableM.class); + assertEquals(getMethod(SerializableM.class), getMethodWithClosestNonSubReturnType(String.class, methods)); + assertEquals(getMethod(SerializableM.class), getMethodWithClosestNonSubReturnType(Serializable.class, methods)); + assertEquals(getMethod(ObjectM.class), getMethodWithClosestNonSubReturnType(List.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(int.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(void.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType4() { + List<Method> methods = getMethods(ReturnType1M.class, ReturnType2M.class, ObjectM.class); + assertEquals(getMethod(ReturnType2M.class), getMethodWithClosestNonSubReturnType(ReturnType3.class, methods)); + assertEquals(getMethod(ReturnType2M.class), getMethodWithClosestNonSubReturnType(ReturnType2.class, methods)); + assertEquals(getMethod(ReturnType1M.class), getMethodWithClosestNonSubReturnType(ReturnType1.class, methods)); + assertEquals(getMethod(ObjectM.class), getMethodWithClosestNonSubReturnType(Serializable.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType5() { + List<Method> methods = getMethods(SerializableM.class, ReturnType1M.class); + assertEquals(getMethod(ReturnType1M.class), getMethodWithClosestNonSubReturnType(ReturnType3.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType6() { + List<Method> methods = getMethods(SerializableM.class); + assertEquals(getMethod(SerializableM.class), getMethodWithClosestNonSubReturnType(ReturnType3.class, methods)); + } + + @Test + public void testGetMethodWithClosestNonSubReturnType7() { + List<Method> methods = getMethods(IntM.class, VoidM.class, ObjectM.class, CollectionM.class); + assertEquals(getMethod(IntM.class), getMethodWithClosestNonSubReturnType(int.class, methods)); + assertEquals(getMethod(VoidM.class), getMethodWithClosestNonSubReturnType(void.class, methods)); + assertNull(getMethodWithClosestNonSubReturnType(long.class, methods)); + assertEquals(getMethod(ObjectM.class), getMethodWithClosestNonSubReturnType(Long.class, methods)); + assertEquals(getMethod(CollectionM.class), getMethodWithClosestNonSubReturnType(List.class, methods)); + } + + private static List<Method> getMethods(Class<?>... methodHolders) { + List<Method> result = new ArrayList<>(); + for (Class<?> methodHolder : methodHolders) { + result.add(getMethod(methodHolder)); + } + return result; + } + + private static Method getMethod(Class<?> methodHolder) { + try { + return methodHolder.getMethod("m"); + } catch (NoSuchMethodException e) { + throw new RuntimeException(e); + } + } + + public static class ObjectM { + public Object m() { return null; } + } + + public static class CollectionM { + public Collection<?> m() { return null; } + } + + public static class ListM { + public List<?> m() { return null; } + } + + public static class SerializableM { + public Serializable m() { return null; } + } + + public static class StringM { + public String m() { return null; } + } + + public static class ReturnType1M { + public ReturnType1 m() { return null; } + } + + public static class ReturnType2M { + public ReturnType2 m() { return null; } + } + + public static class ReturnType3M { + public ReturnType3 m() { return null; } + } + + public static class IntM { + public int m() { return 0; } + } + + public static class LongM { + public long m() { return 0L; } + } + + public static class VoidM { + public void m() { } + } + + public static class ReturnType1 { } + public static class ReturnType2 extends ReturnType1 implements Serializable { } + public static class ReturnType3 extends ReturnType2 { } + +} \ No newline at end of file
