You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2020/06/15 00:16:10 UTC
[freemarker] 01/03: [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.)
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 <dd...@apache.org>
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