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/02/21 12:22:01 UTC

svn commit: r1570535 - in /tomcat/tc7.0.x/trunk: ./ java/javax/el/BeanELResolver.java java/javax/el/LocalStrings.properties java/javax/el/Util.java java/org/apache/el/parser/AstValue.java java/org/apache/el/util/ReflectionUtil.java

Author: markt
Date: Fri Feb 21 11:22:00 2014
New Revision: 1570535

URL: http://svn.apache.org/r1570535
Log:
Back-port the first part of the fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=55483
This fixes calling overloaded methods and aligns the method matching code in the API and the implementation. Unfortunately, this has required a fair amount of duplication. I don't see a way to avoid this, short of an el-util JAR that both depend on.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/javax/el/BeanELResolver.java
    tomcat/tc7.0.x/trunk/java/javax/el/LocalStrings.properties
    tomcat/tc7.0.x/trunk/java/javax/el/Util.java
    tomcat/tc7.0.x/trunk/java/org/apache/el/parser/AstValue.java
    tomcat/tc7.0.x/trunk/java/org/apache/el/util/ReflectionUtil.java

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1518328

Modified: tomcat/tc7.0.x/trunk/java/javax/el/BeanELResolver.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/javax/el/BeanELResolver.java?rev=1570535&r1=1570534&r2=1570535&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/javax/el/BeanELResolver.java (original)
+++ tomcat/tc7.0.x/trunk/java/javax/el/BeanELResolver.java Fri Feb 21 11:22:00 2014
@@ -175,7 +175,7 @@ public class BeanELResolver extends ELRe
 
         // Find the matching method
         Method matchingMethod =
-                Util.findMethod(base, methodName, paramTypes, params);
+                Util.findMethod(base.getClass(), methodName, paramTypes, params);
 
         Object[] parameters = Util.buildParameters(
                 matchingMethod.getParameterTypes(), matchingMethod.isVarArgs(),

Modified: tomcat/tc7.0.x/trunk/java/javax/el/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/javax/el/LocalStrings.properties?rev=1570535&r1=1570534&r2=1570535&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/javax/el/LocalStrings.properties (original)
+++ tomcat/tc7.0.x/trunk/java/javax/el/LocalStrings.properties Fri Feb 21 11:22:00 2014
@@ -22,3 +22,6 @@ propertyNotWritable=Property ''{1}'' not
 propertyReadError=Error reading ''{1}'' on type {0}
 propertyWriteError=Error writing ''{1}'' on type {0}
 objectNotAssignable=Unable to add an object of type [{0}] to an array of objects of type [{1}]
+
+util.method.notfound=Method not found: {0}.{1}({2})
+util.method.ambiguous=Unable to find unambiguous method: {0}.{1}({2})
\ No newline at end of file

Modified: tomcat/tc7.0.x/trunk/java/javax/el/Util.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/javax/el/Util.java?rev=1570535&r1=1570534&r2=1570535&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/javax/el/Util.java (original)
+++ tomcat/tc7.0.x/trunk/java/javax/el/Util.java Fri Feb 21 11:22:00 2014
@@ -22,9 +22,12 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.text.MessageFormat;
+import java.util.HashMap;
 import java.util.Locale;
+import java.util.Map;
 import java.util.MissingResourceException;
 import java.util.ResourceBundle;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.locks.Lock;
@@ -185,84 +188,238 @@ class Util {
     }
 
 
-    static Method findMethod(Object base, String methodName,
-            Class<?>[] paramTypes, Object[] params) {
+    /*
+     * This method duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    @SuppressWarnings("null")
+    static Method findMethod(Class<?> clazz, String methodName,
+            Class<?>[] paramTypes, Object[] paramValues) {
 
-        Method matchingMethod = null;
+        if (clazz == null || methodName == null) {
+            throw new MethodNotFoundException(
+                    message(null, "util.method.notfound", clazz, methodName,
+                    paramString(paramTypes)));
+        }
 
-        Class<?> clazz = base.getClass();
-        if (paramTypes != null) {
-            try {
-                matchingMethod =
-                    getMethod(clazz, clazz.getMethod(methodName, paramTypes));
-            } catch (NoSuchMethodException e) {
-                throw new MethodNotFoundException(e);
-            }
+        int paramCount;
+        if (paramTypes == null) {
+            paramTypes = getTypesFromValues(paramValues);
+        }
+
+        if (paramTypes == null) {
+            paramCount = 0;
         } else {
-            int paramCount = 0;
-            if (params != null) {
-                paramCount = params.length;
+            paramCount = paramTypes.length;
+        }
+
+        Method[] methods = clazz.getMethods();
+        Map<Method,Integer> candidates = new HashMap<Method,Integer>();
+
+        for (Method m : methods) {
+            if (!m.getName().equals(methodName)) {
+                // Method name doesn't match
+                continue;
             }
-            Method[] methods = clazz.getMethods();
-            for (Method m : methods) {
-                if (methodName.equals(m.getName())) {
-                    if (m.getParameterTypes().length == paramCount) {
-                        // Same number of parameters - use the first match
-                        matchingMethod = getMethod(clazz, m);
-                        break;
+
+            Class<?>[] mParamTypes = m.getParameterTypes();
+            int mParamCount;
+            if (mParamTypes == null) {
+                mParamCount = 0;
+            } else {
+                mParamCount = mParamTypes.length;
+            }
+
+            // Check the number of parameters
+            if (!(paramCount == mParamCount ||
+                    (m.isVarArgs() && paramCount >= mParamCount))) {
+                // Method has wrong number of parameters
+                continue;
+            }
+
+            // Check the parameters match
+            int exactMatch = 0;
+            boolean noMatch = false;
+            for (int i = 0; i < mParamCount; i++) {
+                // Can't be null
+                if (mParamTypes[i].equals(paramTypes[i])) {
+                    exactMatch++;
+                } else if (i == (mParamCount - 1) && m.isVarArgs()) {
+                    Class<?> varType = mParamTypes[i].getComponentType();
+                    for (int j = i; j < paramCount; j++) {
+                        if (!isAssignableFrom(paramTypes[j], varType)) {
+                            if (paramValues == null) {
+                                noMatch = true;
+                                break;
+                            } else {
+                                if (!isCoercibleFrom(paramValues[j], varType)) {
+                                    noMatch = true;
+                                    break;
+                                }
+                            }
+                        }
+                        // Don't treat a varArgs match as an exact match, it can
+                        // lead to a varArgs method matching when the result
+                        // should be ambiguous
                     }
-                    if (m.isVarArgs()
-                            && paramCount > m.getParameterTypes().length - 2) {
-                        matchingMethod = getMethod(clazz, m);
+                } else if (!isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+                    if (paramValues == null) {
+                        noMatch = true;
+                        break;
+                    } else {
+                        if (!isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+                            noMatch = true;
+                            break;
+                        }
                     }
                 }
             }
-            if (matchingMethod == null) {
-                throw new MethodNotFoundException(
-                        "Unable to find method [" + methodName + "] with ["
-                        + paramCount + "] parameters");
+            if (noMatch) {
+                continue;
+            }
+
+            // If a method is found where every parameter matches exactly,
+            // return it
+            if (exactMatch == paramCount) {
+                return getMethod(clazz, m);
+            }
+
+            candidates.put(m, Integer.valueOf(exactMatch));
+        }
+
+        // Look for the method that has the highest number of parameters where
+        // the type matches exactly
+        int bestMatch = 0;
+        Method match = null;
+        boolean multiple = false;
+        for (Map.Entry<Method, Integer> entry : candidates.entrySet()) {
+            if (entry.getValue().intValue() > bestMatch ||
+                    match == null) {
+                bestMatch = entry.getValue().intValue();
+                match = entry.getKey();
+                multiple = false;
+            } else if (entry.getValue().intValue() == bestMatch) {
+                multiple = true;
+            }
+        }
+        if (multiple) {
+            if (bestMatch == paramCount - 1) {
+                // Only one parameter is not an exact match - try using the
+                // super class
+                match = resolveAmbiguousMethod(candidates.keySet(), paramTypes);
+            } else {
+                match = null;
             }
+
+            if (match == null) {
+                // If multiple methods have the same matching number of parameters
+                // the match is ambiguous so throw an exception
+                throw new MethodNotFoundException(message(
+                        null, "util.method.ambiguous", clazz, methodName,
+                        paramString(paramTypes)));
+                }
         }
 
-        return matchingMethod;
+        // Handle case where no match at all was found
+        if (match == null) {
+            throw new MethodNotFoundException(message(
+                        null, "util.method.notfound", clazz, methodName,
+                        paramString(paramTypes)));
+        }
+
+        return getMethod(clazz, match);
     }
 
 
-    static Method getMethod(Class<?> type, Method m) {
-        if (m == null || Modifier.isPublic(type.getModifiers())) {
-            return m;
-        }
-        Class<?>[] inf = type.getInterfaces();
-        Method mp = null;
-        for (int i = 0; i < inf.length; i++) {
-            try {
-                mp = inf[i].getMethod(m.getName(), m.getParameterTypes());
-                mp = getMethod(mp.getDeclaringClass(), mp);
-                if (mp != null) {
-                    return mp;
+    private static final String paramString(Class<?>[] types) {
+        if (types != null) {
+            StringBuilder sb = new StringBuilder();
+            for (int i = 0; i < types.length; i++) {
+                if (types[i] == null) {
+                    sb.append("null, ");
+                } else {
+                    sb.append(types[i].getName()).append(", ");
                 }
-            } catch (NoSuchMethodException e) {
-                // Ignore
             }
+            if (sb.length() > 2) {
+                sb.setLength(sb.length() - 2);
+            }
+            return sb.toString();
         }
-        Class<?> sup = type.getSuperclass();
-        if (sup != null) {
-            try {
-                mp = sup.getMethod(m.getName(), m.getParameterTypes());
-                mp = getMethod(mp.getDeclaringClass(), mp);
-                if (mp != null) {
-                    return mp;
+        return null;
+    }
+
+
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    private static Method resolveAmbiguousMethod(Set<Method> candidates,
+            Class<?>[] paramTypes) {
+        // Identify which parameter isn't an exact match
+        Method m = candidates.iterator().next();
+
+        int nonMatchIndex = 0;
+        Class<?> nonMatchClass = null;
+
+        for (int i = 0; i < paramTypes.length; i++) {
+            if (m.getParameterTypes()[i] != paramTypes[i]) {
+                nonMatchIndex = i;
+                nonMatchClass = paramTypes[i];
+                break;
+            }
+        }
+
+        if (nonMatchClass == null) {
+            // Null will always be ambiguous
+            return null;
+        }
+
+        for (Method c : candidates) {
+           if (c.getParameterTypes()[nonMatchIndex] ==
+                   paramTypes[nonMatchIndex]) {
+               // Methods have different non-matching parameters
+               // Result is ambiguous
+               return null;
+           }
+        }
+
+        // Can't be null
+        Class<?> superClass = nonMatchClass.getSuperclass();
+        while (superClass != null) {
+            for (Method c : candidates) {
+                if (c.getParameterTypes()[nonMatchIndex].equals(superClass)) {
+                    // Found a match
+                    return c;
+                }
+            }
+            superClass = superClass.getSuperclass();
+        }
+
+        // Treat instances of Number as a special case
+        Method match = null;
+        if (Number.class.isAssignableFrom(nonMatchClass)) {
+            for (Method c : candidates) {
+                Class<?> candidateType = c.getParameterTypes()[nonMatchIndex];
+                if (Number.class.isAssignableFrom(candidateType) ||
+                        candidateType.isPrimitive()) {
+                    if (match == null) {
+                        match = c;
+                    } else {
+                        // Match still ambiguous
+                        match = null;
+                        break;
+                    }
                 }
-            } catch (NoSuchMethodException e) {
-                // Ignore
             }
         }
-        return null;
+
+        return match;
     }
-    
-    
+
+
     /*
-     * This method duplicates code in org.apache.el.util.ReflectionUtil. When
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
      * making changes keep the code in sync.
      */
     static boolean isAssignableFrom(Class<?> src, Class<?> target) {
@@ -299,6 +456,76 @@ class Util {
     }
 
 
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    private static boolean isCoercibleFrom(Object src, Class<?> target) {
+        // TODO: This isn't pretty but it works. Significant refactoring would
+        //       be required to avoid the exception.
+        try {
+            getExpressionFactory().coerceToType(src, target);
+        } catch (ELException e) {
+            return false;
+        }
+        return true;
+    }
+
+
+    private static Class<?>[] getTypesFromValues(Object[] values) {
+        if (values == null) {
+            return null;
+        }
+
+        Class<?> result[] = new Class<?>[values.length];
+        for (int i = 0; i < values.length; i++) {
+            if (values[i] == null) {
+                result[i] = null;
+            } else {
+                result[i] = values[i].getClass();
+            }
+        }
+        return result;
+    }
+
+
+    /*
+     * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    static Method getMethod(Class<?> type, Method m) {
+        if (m == null || Modifier.isPublic(type.getModifiers())) {
+            return m;
+        }
+        Class<?>[] inf = type.getInterfaces();
+        Method mp = null;
+        for (int i = 0; i < inf.length; i++) {
+            try {
+                mp = inf[i].getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        Class<?> sup = type.getSuperclass();
+        if (sup != null) {
+            try {
+                mp = sup.getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        return null;
+    }
+    
+    
     static Constructor<?> findConstructor(Object base, Class<?>[] paramTypes,
             Object[] params) {
 

Modified: tomcat/tc7.0.x/trunk/java/org/apache/el/parser/AstValue.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/el/parser/AstValue.java?rev=1570535&r1=1570534&r2=1570535&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/el/parser/AstValue.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/el/parser/AstValue.java Fri Feb 21 11:22:00 2014
@@ -169,8 +169,9 @@ public final class AstValue extends Simp
                 AstMethodParameters mps =
                     (AstMethodParameters) this.children[i+1];
                 // This is a method
-                base = resolver.invoke(ctx, base, suffix, null,
-                        mps.getParameters(ctx));
+                Object[] paramValues = mps.getParameters(ctx);
+                base = resolver.invoke(ctx, base, suffix,
+                        getTypesFromValues(paramValues), paramValues);
                 i+=2;
             } else {
                 // This is a property

Modified: tomcat/tc7.0.x/trunk/java/org/apache/el/util/ReflectionUtil.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/el/util/ReflectionUtil.java?rev=1570535&r1=1570534&r2=1570535&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/el/util/ReflectionUtil.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/el/util/ReflectionUtil.java Fri Feb 21 11:22:00 2014
@@ -18,6 +18,7 @@ package org.apache.el.util;
 
 import java.lang.reflect.Array;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -104,7 +105,7 @@ public class ReflectionUtil {
     }
 
     /**
-     * Returns a method based on the criteria
+     * Returns a method based on the criteria.
      * @param base the object that owns the method
      * @param property the name of the method
      * @param paramTypes the parameter types to use
@@ -112,6 +113,10 @@ public class ReflectionUtil {
      * @return the method specified
      * @throws MethodNotFoundException
      */
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     @SuppressWarnings("null")
     public static Method getMethod(Object base, Object property,
             Class<?>[] paramTypes, Object[] paramValues)
@@ -200,7 +205,7 @@ public class ReflectionUtil {
             // If a method is found where every parameter matches exactly,
             // return it
             if (exactMatch == paramCount) {
-                return m;
+                getMethod(base.getClass(), m);
             }
             
             candidates.put(m, Integer.valueOf(exactMatch));
@@ -246,9 +251,13 @@ public class ReflectionUtil {
                         paramString(paramTypes)));
         }
         
-        return match;
+        return getMethod(base.getClass(), match);
     }
 
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static Method resolveAmbiguousMethod(Set<Method> candidates,
             Class<?>[] paramTypes) {
         // Identify which parameter isn't an exact match
@@ -280,23 +289,45 @@ public class ReflectionUtil {
         }
         
         // Can't be null
-        nonMatchClass = nonMatchClass.getSuperclass();
-        while (nonMatchClass != null) {
+        Class<?> superClass = nonMatchClass.getSuperclass();
+        while (superClass != null) {
             for (Method c : candidates) {
-                if (c.getParameterTypes()[nonMatchIndex].equals(
-                        nonMatchClass)) {
+                if (c.getParameterTypes()[nonMatchIndex].equals(superClass)) {
                     // Found a match
                     return c;
                 }
             }
-            nonMatchClass = nonMatchClass.getSuperclass();
+            superClass = superClass.getSuperclass();
         }
         
-        return null;
+        // Treat instances of Number as a special case
+        Method match = null;
+        if (Number.class.isAssignableFrom(nonMatchClass)) {
+            for (Method c : candidates) {
+                Class<?> candidateType = c.getParameterTypes()[nonMatchIndex];
+                if (Number.class.isAssignableFrom(candidateType) ||
+                        candidateType.isPrimitive()) {
+                    if (match == null) {
+                        match = c;
+                    } else {
+                        // Match still ambiguous
+                        match = null;
+                        break;
+                    }
+                }
+            }
+        }
+
+        return match;
     }
 
-    // src will always be an object
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static boolean isAssignableFrom(Class<?> src, Class<?> target) {
+        // src will always be an object
         // Short-cut. null is always assignable to an object and in EL null
         // can always be coerced to a valid value for a primitive
         if (src == null) {
@@ -328,6 +359,11 @@ public class ReflectionUtil {
         return targetClass.isAssignableFrom(src);
     }
 
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
     private static boolean isCoercibleFrom(Object src, Class<?> target) {
         // TODO: This isn't pretty but it works. Significant refactoring would
         //       be required to avoid the exception.
@@ -339,7 +375,45 @@ public class ReflectionUtil {
         return true;
     }
 
-    protected static final String paramString(Class<?>[] types) {
+
+    /*
+     * This class duplicates code in javax.el.Util. When making changes keep
+     * the code in sync.
+     */
+    private static Method getMethod(Class<?> type, Method m) {
+        if (m == null || Modifier.isPublic(type.getModifiers())) {
+            return m;
+        }
+        Class<?>[] inf = type.getInterfaces();
+        Method mp = null;
+        for (int i = 0; i < inf.length; i++) {
+            try {
+                mp = inf[i].getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        Class<?> sup = type.getSuperclass();
+        if (sup != null) {
+            try {
+                mp = sup.getMethod(m.getName(), m.getParameterTypes());
+                mp = getMethod(mp.getDeclaringClass(), mp);
+                if (mp != null) {
+                    return mp;
+                }
+            } catch (NoSuchMethodException e) {
+                // Ignore
+            }
+        }
+        return null;
+    }
+
+
+    private static final String paramString(Class<?>[] types) {
         if (types != null) {
             StringBuilder sb = new StringBuilder();
             for (int i = 0; i < types.length; i++) {



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