From 20077a1a327b31c00771566390209d3c28e60867 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 02:23:50 +0900 Subject: [PATCH 1/6] Include bridge method in collectMethods() The test passes on Java 8, but yields warning on 9+ and exception on 16+. It's essentially the same as #104 . OGNL should call `StringBuilder#codePointAt()` (which is a bridge method) rather than the method defined in its non-public parent class `AbstractStringBuilder`. --- src/main/java/ognl/OgnlRuntime.java | 3 +-- src/test/java/ognl/OgnlRuntimeTest.java | 7 +++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 2b504882..57b682ec 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -2260,8 +2260,7 @@ private static void collectMethods(Class c, Map result, boolean staticMethods) { continue; } - // skip over synthetic methods - if (!isMethodCallable(ma[i])) + if (!isMethodCallable_BridgeOrNonSynthetic(ma[i])) continue; if (Modifier.isStatic(ma[i].getModifiers()) == staticMethods) diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index 4c12dcf4..6bd09107 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -548,4 +548,11 @@ public Integer getIndex() { } } + @Test + public void shouldInvokeSyntheticBridgeMethod() throws Exception { + StringBuilder root = new StringBuilder("abc"); + Assert.assertEquals((int) 'b', + Ognl.getValue("codePointAt(1)", defaultContext, root)); + } + } From 7998ebd9b1ea0d215f9f8a9b9255622537383598 Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 03:09:34 +0900 Subject: [PATCH 2/6] Instead of 'the most specific', use the public class' method if there is The test passes on Java 8, but yields warning on 9+ and exception on 16+. --- src/main/java/ognl/OgnlRuntime.java | 6 +++--- src/test/java/ognl/OgnlRuntimeTest.java | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 57b682ec..2b81e8d6 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -1782,12 +1782,12 @@ private static MatchingMethod findBestMethod(List methods, Class typeClass, Stri failure = null; } else if (mm.score == score) { // it happens that we see the same method signature multiple times - for the current class or interfaces ... - // TODO why are all of them on the list and not only the most specific one? // check for same signature if (Arrays.equals(mm.mMethod.getParameterTypes(), m.getParameterTypes()) && mm.mMethod.getName().equals(m.getName())) { boolean retsAreEqual = mm.mMethod.getReturnType().equals(m.getReturnType()); - // it is the same method. we use the most specific one... - if (mm.mMethod.getDeclaringClass().isAssignableFrom(m.getDeclaringClass())) { + // it is the same method. we use the public one... + if (!Modifier.isPublic(mm.mMethod.getDeclaringClass().getModifiers()) + && Modifier.isPublic(m.getDeclaringClass().getModifiers())) { if (!retsAreEqual && !mm.mMethod.getReturnType().isAssignableFrom(m.getReturnType())) System.err.println("Two methods with same method signature but return types conflict? \""+mm.mMethod+"\" and \""+m+"\" please report!"); diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index 6bd09107..78291577 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -6,6 +6,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; @@ -555,4 +556,10 @@ public void shouldInvokeSyntheticBridgeMethod() throws Exception { Ognl.getValue("codePointAt(1)", defaultContext, root)); } + @Test + public void shouldInvokeSuperclassMethod() throws Exception { + Map root = Collections.singletonMap(3L, 33L); + Assert.assertTrue((Boolean) Ognl.getValue("containsKey(3L)", + defaultContext, root)); + } } From 8ca995e20f9ef2ce71debedf0f25819780b0fe7d Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 03:24:13 +0900 Subject: [PATCH 3/6] Include interface methods in collectMethods() The test passes on Java 8, but yields warning on 9+ and exception on 16+. --- src/main/java/ognl/OgnlRuntime.java | 7 ------- src/test/java/ognl/OgnlRuntimeTest.java | 6 ++++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 2b81e8d6..3855d5ec 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -2253,13 +2253,6 @@ private static void collectMethods(Class c, Map result, boolean staticMethods) { } for (int i = 0, icount = ma.length; i < icount; i++) { - if (c.isInterface()) - { - if (isDefaultMethod(ma[i])) - addMethodToResult(result, ma[i]); - continue; - } - if (!isMethodCallable_BridgeOrNonSynthetic(ma[i])) continue; diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index 78291577..a7e2ec9c 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -562,4 +562,10 @@ public void shouldInvokeSuperclassMethod() throws Exception { Assert.assertTrue((Boolean) Ognl.getValue("containsKey(3L)", defaultContext, root)); } + + @Test + public void shouldInvokeInterfaceMethod() throws Exception { + Assert.assertTrue((Boolean) Ognl.getValue("isEmpty()", defaultContext, + Collections.checkedCollection(new ArrayList<>(), String.class))); + } } From 0e1ffeb175ddefb0ffde654a6b9941584954a0be Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 04:04:08 +0900 Subject: [PATCH 4/6] Remove inappropriate message in System.err The test passes, but there is a message in System.err: Two methods with same method signature but not providing classes assignable? This was originally reported as #35 and temporarily resolved by #40 , but as you can see, this is not an error case in the first place. --- src/main/java/ognl/OgnlRuntime.java | 3 --- src/test/java/ognl/OgnlRuntimeTest.java | 20 ++++++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 3855d5ec..46d598da 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -1793,9 +1793,6 @@ private static MatchingMethod findBestMethod(List methods, Class typeClass, Stri mm = new MatchingMethod(m, score, report, mParameterTypes); failure = null; - } else if (!m.getDeclaringClass().isAssignableFrom(mm.mMethod.getDeclaringClass())) { - // this should't happen - System.err.println("Two methods with same method signature but not providing classes assignable? \""+mm.mMethod+"\" and \""+m+"\" please report!"); } else if (!retsAreEqual && !m.getReturnType().isAssignableFrom(mm.mMethod.getReturnType())) System.err.println("Two methods with same method signature but return types conflict? \""+mm.mMethod+"\" and \""+m+"\" please report!"); } else { diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index a7e2ec9c..daa037d1 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -568,4 +568,24 @@ public void shouldInvokeInterfaceMethod() throws Exception { Assert.assertTrue((Boolean) Ognl.getValue("isEmpty()", defaultContext, Collections.checkedCollection(new ArrayList<>(), String.class))); } + + public interface I1 { + Integer getId(); + } + + public interface I2 { + Integer getId(); + } + + @Test + public void shouldMultipleInterfaceWithTheSameMethodBeFine() + throws Exception { + class C1 implements I1, I2 { + public Integer getId() { + return 100; + } + } + Assert.assertEquals(100, + Ognl.getValue("getId()", defaultContext, new C1())); + } } From 402b2166017925b064dc983e01566fbfef6b1f1c Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 04:24:38 +0900 Subject: [PATCH 5/6] Remove inappropriate message in System.err The test passes, but there is a message in System.err. It might have been a legitimate check in non-generic era, but not anymore. --- src/main/java/ognl/OgnlRuntime.java | 3 --- src/test/java/ognl/OgnlRuntimeTest.java | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 46d598da..7eee8988 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -1788,9 +1788,6 @@ private static MatchingMethod findBestMethod(List methods, Class typeClass, Stri // it is the same method. we use the public one... if (!Modifier.isPublic(mm.mMethod.getDeclaringClass().getModifiers()) && Modifier.isPublic(m.getDeclaringClass().getModifiers())) { - if (!retsAreEqual && !mm.mMethod.getReturnType().isAssignableFrom(m.getReturnType())) - System.err.println("Two methods with same method signature but return types conflict? \""+mm.mMethod+"\" and \""+m+"\" please report!"); - mm = new MatchingMethod(m, score, report, mParameterTypes); failure = null; } else if (!retsAreEqual && !m.getReturnType().isAssignableFrom(mm.mMethod.getReturnType())) diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index daa037d1..de6c2a51 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -588,4 +588,21 @@ public Integer getId() { Assert.assertEquals(100, Ognl.getValue("getId()", defaultContext, new C1())); } + + public interface I3 { + T get(); + } + + @Test + public void shouldTwoMethodsWithDifferentReturnTypeBeFine() + throws Exception { + class C1 implements I3 { + @Override + public Long get() { + return 3L; + } + } + Assert.assertEquals(3L, + Ognl.getValue("get()", defaultContext, new C1())); + } } From db64c9328bb168f4e693f7934bea17e9f799a41d Mon Sep 17 00:00:00 2001 From: Iwao AVE! Date: Sun, 9 Jan 2022 04:27:36 +0900 Subject: [PATCH 6/6] Remove inappropriate message in System.err The test passes, but there is a message in System.err. Slightly different version of the previous commit. --- src/main/java/ognl/OgnlRuntime.java | 3 +-- src/test/java/ognl/OgnlRuntimeTest.java | 6 +++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index 7eee8988..20cc35ed 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -1790,8 +1790,7 @@ private static MatchingMethod findBestMethod(List methods, Class typeClass, Stri && Modifier.isPublic(m.getDeclaringClass().getModifiers())) { mm = new MatchingMethod(m, score, report, mParameterTypes); failure = null; - } else if (!retsAreEqual && !m.getReturnType().isAssignableFrom(mm.mMethod.getReturnType())) - System.err.println("Two methods with same method signature but return types conflict? \""+mm.mMethod+"\" and \""+m+"\" please report!"); + } } else { // two methods with same score - direct compare to find the better one... // legacy wins over varargs diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index de6c2a51..b447e76a 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -593,10 +593,14 @@ public interface I3 { T get(); } + public interface I4 { + Long get(); + } + @Test public void shouldTwoMethodsWithDifferentReturnTypeBeFine() throws Exception { - class C1 implements I3 { + class C1 implements I3, I4 { @Override public Long get() { return 3L;