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 2013/08/28 21:02:40 UTC
svn commit: r1518328 - in /tomcat/trunk:
java/javax/el/LocalStrings.properties java/javax/el/Util.java
java/org/apache/el/parser/AstValue.java
java/org/apache/el/util/ReflectionUtil.java test/javax/el/TestUtil.java
Author: markt
Date: Wed Aug 28 19:02:39 2013
New Revision: 1518328
URL: http://svn.apache.org/r1518328
Log:
Partial 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.
Added:
tomcat/trunk/test/javax/el/TestUtil.java (with props)
Modified:
tomcat/trunk/java/javax/el/LocalStrings.properties
tomcat/trunk/java/javax/el/Util.java
tomcat/trunk/java/org/apache/el/parser/AstValue.java
tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java
Modified: tomcat/trunk/java/javax/el/LocalStrings.properties
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/LocalStrings.properties?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/LocalStrings.properties (original)
+++ tomcat/trunk/java/javax/el/LocalStrings.properties Wed Aug 28 19:02:39 2013
@@ -45,4 +45,7 @@ lambdaExpression.tooFewArgs=Only [{0}] a
staticFieldELResolver.methodNotFound=No matching public static method named [{0}] found on class [{1}]
staticFieldELResolver.notFound=No public static field named [{0}] was found on class [{1}]
-staticFieldELResolver.notWriteable=Writing to static fields (in this case field [{0}] on class [{1}]) is not permitted
\ No newline at end of file
+staticFieldELResolver.notWriteable=Writing to static fields (in this case field [{0}] on class [{1}]) is not permitted
+
+util.method.notfound=Method not found: {0}.{1}({2})
+util.method.ambiguous=Unable to find unambiguous method: {0}.{1}({2})
Modified: tomcat/trunk/java/javax/el/Util.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/Util.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/Util.java (original)
+++ tomcat/trunk/java/javax/el/Util.java Wed Aug 28 19:02:39 2013
@@ -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,48 +188,311 @@ class Util {
}
+ /*
+ * This class 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[] params) {
+ Class<?>[] paramTypes, Object[] paramValues) {
- Method matchingMethod = null;
+ if (clazz == null || methodName == null) {
+ throw new MethodNotFoundException(
+ message(null, "util.method.notfound", clazz, methodName,
+ paramString(paramTypes)));
+ }
- 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<>();
+
+ 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);
+
+ 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
+ }
+ } else if (!isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+ if (paramValues == null) {
+ noMatch = true;
break;
+ } else {
+ if (!isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+ noMatch = true;
+ break;
+ }
}
- if (m.isVarArgs()
- && paramCount > m.getParameterTypes().length - 2) {
- matchingMethod = getMethod(clazz, m);
+ }
+ }
+ 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)));
+ }
+ }
+
+ // 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);
+ }
+
+
+ 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(", ");
+ }
+ }
+ if (sb.length() > 2) {
+ sb.setLength(sb.length() - 2);
+ }
+ return sb.toString();
+ }
+ 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;
}
}
}
- if (matchingMethod == null) {
- throw new MethodNotFoundException(
- "Unable to find method [" + methodName + "] with ["
- + paramCount + "] parameters");
+ }
+
+ return match;
+ }
+
+
+ /*
+ * This class duplicates code in org.apache.el.util.ReflectionUtil. 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) {
+ return true;
+ }
+
+ Class<?> targetClass;
+ if (target.isPrimitive()) {
+ if (target == Boolean.TYPE) {
+ targetClass = Boolean.class;
+ } else if (target == Character.TYPE) {
+ targetClass = Character.class;
+ } else if (target == Byte.TYPE) {
+ targetClass = Byte.class;
+ } else if (target == Short.TYPE) {
+ targetClass = Short.class;
+ } else if (target == Integer.TYPE) {
+ targetClass = Integer.class;
+ } else if (target == Long.TYPE) {
+ targetClass = Long.class;
+ } else if (target == Float.TYPE) {
+ targetClass = Float.class;
+ } else {
+ targetClass = Double.class;
}
+ } else {
+ targetClass = target;
+ }
+ return targetClass.isAssignableFrom(src);
+ }
+
+
+ /*
+ * 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;
}
- return matchingMethod;
+ 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;
Modified: tomcat/trunk/java/org/apache/el/parser/AstValue.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/parser/AstValue.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/parser/AstValue.java (original)
+++ tomcat/trunk/java/org/apache/el/parser/AstValue.java Wed Aug 28 19:02:39 2013
@@ -154,8 +154,9 @@ public final class AstValue extends Simp
}
}
// 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/trunk/java/org/apache/el/util/ReflectionUtil.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java?rev=1518328&r1=1518327&r2=1518328&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java (original)
+++ tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java Wed Aug 28 19:02:39 2013
@@ -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;
@@ -105,7 +106,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
@@ -113,6 +114,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)
@@ -201,7 +206,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));
@@ -247,9 +252,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
@@ -281,23 +290,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) {
@@ -329,6 +360,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.
@@ -340,7 +376,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++) {
Added: tomcat/trunk/test/javax/el/TestUtil.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestUtil.java?rev=1518328&view=auto
==============================================================================
--- tomcat/trunk/test/javax/el/TestUtil.java (added)
+++ tomcat/trunk/test/javax/el/TestUtil.java Wed Aug 28 19:02:39 2013
@@ -0,0 +1,41 @@
+/*
+ * 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 javax.el;
+
+import java.util.Date;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestUtil {
+
+ @Test
+ public void test01() {
+ ELProcessor processor = new ELProcessor();
+ processor.defineBean("sb", new StringBuilder());
+ Assert.assertEquals("a", processor.eval("sb.append('a'); sb.toString()"));
+ }
+
+
+ // Commented out as this is not yet fixed @Test
+ public void test02() {
+ ELProcessor processor = new ELProcessor();
+ processor.getELManager().importClass("java.util.Date");
+ Date result = (Date) processor.eval("Date(86400)");
+ Assert.assertEquals(86400, result.getTime());
+ }
+}
Propchange: tomcat/trunk/test/javax/el/TestUtil.java
------------------------------------------------------------------------------
svn:eol-style = native
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org