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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org