You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/08/05 13:20:53 UTC

svn commit: r1615911 - in /tomcat/trunk: java/javax/el/ java/org/apache/el/util/ test/org/apache/el/ webapps/docs/

Author: markt
Date: Tue Aug  5 11:20:52 2014
New Revision: 1615911

URL: http://svn.apache.org/r1615911
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56797
When matching a method in an EL expression, do not treat bridge methods as duplicates of the method they bridge to. In this case always call the target of the bridge method.

Modified:
    tomcat/trunk/java/javax/el/Util.java
    tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java
    tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java
    tomcat/trunk/test/org/apache/el/TesterBeanA.java
    tomcat/trunk/test/org/apache/el/TesterBeanAA.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/javax/el/Util.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/Util.java?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/Util.java (original)
+++ tomcat/trunk/java/javax/el/Util.java Tue Aug  5 11:20:52 2014
@@ -310,12 +310,13 @@ class Util {
                 return w;
             }
 
-            candidates.put(w, new MatchResult(exactMatch, assignableMatch, coercibleMatch));
+            candidates.put(w, new MatchResult(
+                    exactMatch, assignableMatch, coercibleMatch, w.isBridge()));
         }
 
         // Look for the method that has the highest number of parameters where
         // the type matches exactly
-        MatchResult bestMatch = new MatchResult(0, 0, 0);
+        MatchResult bestMatch = new MatchResult(0, 0, 0, false);
         Wrapper match = null;
         boolean multiple = false;
         for (Map.Entry<Wrapper, MatchResult> entry : candidates.entrySet()) {
@@ -662,6 +663,7 @@ class Util {
         public abstract Object unWrap();
         public abstract Class<?>[] getParameterTypes();
         public abstract boolean isVarArgs();
+        public abstract boolean isBridge();
     }
 
 
@@ -686,6 +688,11 @@ class Util {
         public boolean isVarArgs() {
             return m.isVarArgs();
         }
+
+        @Override
+        public boolean isBridge() {
+            return m.isBridge();
+        }
     }
 
     private static class ConstructorWrapper extends Wrapper {
@@ -709,6 +716,11 @@ class Util {
         public boolean isVarArgs() {
             return c.isVarArgs();
         }
+
+        @Override
+        public boolean isBridge() {
+            return false;
+        }
     }
 
     /*
@@ -720,11 +732,13 @@ class Util {
         private final int exact;
         private final int assignable;
         private final int coercible;
+        private final boolean bridge;
 
-        public MatchResult(int exact, int assignable, int coercible) {
+        public MatchResult(int exact, int assignable, int coercible, boolean bridge) {
             this.exact = exact;
             this.assignable = assignable;
             this.coercible = coercible;
+            this.bridge = bridge;
         }
 
         public int getExact() {
@@ -739,6 +753,10 @@ class Util {
             return coercible;
         }
 
+        public boolean isBridge() {
+            return bridge;
+        }
+
         @Override
         public int compareTo(MatchResult o) {
             int cmp = Integer.compare(this.getExact(), o.getExact());
@@ -746,6 +764,13 @@ class Util {
                 cmp = Integer.compare(this.getAssignable(), o.getAssignable());
                 if (cmp == 0) {
                     cmp = Integer.compare(this.getCoercible(), o.getCoercible());
+                    if (cmp == 0) {
+                        // The nature of bridge methods is such that it actually
+                        // doesn't matter which one we pick as long as we pick
+                        // one. That said, pick the 'right' one (the non-bridge
+                        // one) anyway.
+                        cmp = Boolean.compare(o.isBridge(), this.isBridge());
+                    }
                 }
             }
             return cmp;

Modified: tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java (original)
+++ tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java Tue Aug  5 11:20:52 2014
@@ -218,12 +218,13 @@ public class ReflectionUtil {
                 return getMethod(base.getClass(), m);
             }
 
-            candidates.put(m, new MatchResult(exactMatch, assignableMatch, coercibleMatch));
+            candidates.put(m, new MatchResult(
+                    exactMatch, assignableMatch, coercibleMatch, m.isBridge()));
         }
 
         // Look for the method that has the highest number of parameters where
         // the type matches exactly
-        MatchResult bestMatch = new MatchResult(0, 0, 0);
+        MatchResult bestMatch = new MatchResult(0, 0, 0, false);
         Method match = null;
         boolean multiple = false;
         for (Map.Entry<Method, MatchResult> entry : candidates.entrySet()) {
@@ -450,11 +451,13 @@ public class ReflectionUtil {
         private final int exact;
         private final int assignable;
         private final int coercible;
+        private final boolean bridge;
 
-        public MatchResult(int exact, int assignable, int coercible) {
+        public MatchResult(int exact, int assignable, int coercible, boolean bridge) {
             this.exact = exact;
             this.assignable = assignable;
             this.coercible = coercible;
+            this.bridge = bridge;
         }
 
         public int getExact() {
@@ -469,6 +472,10 @@ public class ReflectionUtil {
             return coercible;
         }
 
+        public boolean isBridge() {
+            return bridge;
+        }
+
         @Override
         public int compareTo(MatchResult o) {
             int cmp = Integer.compare(this.getExact(), o.getExact());
@@ -476,6 +483,13 @@ public class ReflectionUtil {
                 cmp = Integer.compare(this.getAssignable(), o.getAssignable());
                 if (cmp == 0) {
                     cmp = Integer.compare(this.getCoercible(), o.getCoercible());
+                    if (cmp == 0) {
+                        // The nature of bridge methods is such that it actually
+                        // doesn't matter which one we pick as long as we pick
+                        // one. That said, pick the 'right' one (the non-bridge
+                        // one) anyway.
+                        cmp = Boolean.compare(o.isBridge(), this.isBridge());
+                    }
                 }
             }
             return cmp;

Modified: tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java (original)
+++ tomcat/trunk/test/org/apache/el/TestMethodExpressionImpl.java Tue Aug  5 11:20:52 2014
@@ -464,4 +464,22 @@ public class TestMethodExpressionImpl {
         Integer result = (Integer) me.invoke(context, null);
         assertEquals(beanB.sayHello().length(), result.intValue());
     }
+
+
+    @Test
+    public void testBug56797a() {
+        MethodExpression me = factory.createMethodExpression(context,
+                "${beanAA.echo1('Hello World!')}", null , null);
+        Object r = me.invoke(context, null);
+        assertEquals("AA1Hello World!", r.toString());
+    }
+
+
+    @Test
+    public void testBug56797b() {
+        MethodExpression me = factory.createMethodExpression(context,
+                "${beanAA.echo2('Hello World!')}", null , null);
+        Object r = me.invoke(context, null);
+        assertEquals("AA2Hello World!", r.toString());
+    }
 }

Modified: tomcat/trunk/test/org/apache/el/TesterBeanA.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/el/TesterBeanA.java?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/el/TesterBeanA.java (original)
+++ tomcat/trunk/test/org/apache/el/TesterBeanA.java Tue Aug  5 11:20:52 2014
@@ -56,4 +56,12 @@ public class TesterBeanA {
     public void setValList(List<?> valList) {
         this.valList = valList;
     }
+
+    public CharSequence echo1(CharSequence cs) {
+        return "A1" + cs;
+    }
+
+    public CharSequence echo2(String s) {
+        return "A2" + s;
+    }
 }

Modified: tomcat/trunk/test/org/apache/el/TesterBeanAA.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/el/TesterBeanAA.java?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/el/TesterBeanAA.java (original)
+++ tomcat/trunk/test/org/apache/el/TesterBeanAA.java Tue Aug  5 11:20:52 2014
@@ -18,6 +18,14 @@
 package org.apache.el;
 
 public class TesterBeanAA extends TesterBeanA {
-    // No additional implementation - just need a class that extends A for
-    // testing EL methods calls
+
+    @Override
+    public String echo1(CharSequence cs) {
+        return "AA1" + cs.toString();
+    }
+
+    @Override
+    public String echo2(String s) {
+        return "AA2" + s;
+    }
 }

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1615911&r1=1615910&r2=1615911&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Aug  5 11:20:52 2014
@@ -139,6 +139,11 @@
         <bug>56709</bug>: Fix system property name in a log message. Submitted
         by Robert Kish. (remm)
       </fix>
+      <fix>
+        <bug>56797</bug>: When matching a method in an EL expression, do not
+        treat bridge methods as duplicates of the method they bridge to. In this
+        case always call the target of the bridge method. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="WebSocket">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org