Author: markt
Date: Fri Apr 25 19:49:46 2014
New Revision: 1590120
URL: http://svn.apache.org/r1590120
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56425
Improve method matching for EL expressions. When looking for matching methods,
an exact match between parameter types is preferred followed by an assignable
match followed by a coercible match.
Modified:
tomcat/trunk/java/javax/el/Util.java
tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java
tomcat/trunk/test/javax/el/TestUtil.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=1590120&r1=1590119&r2=1590120&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/Util.java (original)
+++ tomcat/trunk/java/javax/el/Util.java Fri Apr 25 19:49:46 2014
@@ -228,7 +228,7 @@ class Util {
private static Wrapper findWrapper(Class<?> clazz, List<Wrapper> wrappers,
String name, Class<?>[] paramTypes, Object[] paramValues) {
- Map<Wrapper,Integer> candidates = new HashMap<>();
+ Map<Wrapper,MatchResult> candidates = new HashMap<>();
int paramCount;
if (paramTypes == null) {
@@ -255,6 +255,8 @@ class Util {
// Check the parameters match
int exactMatch = 0;
+ int assignableMatch = 0;
+ int coercibleMatch = 0;
boolean noMatch = false;
for (int i = 0; i < mParamCount; i++) {
// Can't be null
@@ -263,12 +265,16 @@ class Util {
} else if (i == (mParamCount - 1) && w.isVarArgs()) {
Class<?> varType = mParamTypes[i].getComponentType();
for (int j = i; j < paramCount; j++) {
- if (!isAssignableFrom(paramTypes[j], varType)) {
+ if (isAssignableFrom(paramTypes[j], varType)) {
+ assignableMatch++;
+ } else {
if (paramValues == null) {
noMatch = true;
break;
} else {
- if (!isCoercibleFrom(paramValues[j], varType))
{
+ if (isCoercibleFrom(paramValues[j], varType)) {
+ coercibleMatch++;
+ } else {
noMatch = true;
break;
}
@@ -278,12 +284,16 @@ class Util {
// lead to a varArgs method matching when the result
// should be ambiguous
}
- } else if (!isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+ } else if (isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+ assignableMatch++;
+ } else {
if (paramValues == null) {
noMatch = true;
break;
} else {
- if (!isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+ if (isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+ coercibleMatch++;
+ } else {
noMatch = true;
break;
}
@@ -300,26 +310,26 @@ class Util {
return w;
}
- candidates.put(w, Integer.valueOf(exactMatch));
+ candidates.put(w, new MatchResult(exactMatch, assignableMatch,
coercibleMatch));
}
// Look for the method that has the highest number of parameters where
// the type matches exactly
- int bestMatch = 0;
+ MatchResult bestMatch = new MatchResult(0, 0, 0);
Wrapper match = null;
boolean multiple = false;
- for (Map.Entry<Wrapper, Integer> entry : candidates.entrySet()) {
- if (entry.getValue().intValue() > bestMatch ||
- match == null) {
- bestMatch = entry.getValue().intValue();
+ for (Map.Entry<Wrapper, MatchResult> entry : candidates.entrySet()) {
+ int cmp = entry.getValue().compareTo(bestMatch);
+ if (cmp > 0 || match == null) {
+ bestMatch = entry.getValue();
match = entry.getKey();
multiple = false;
- } else if (entry.getValue().intValue() == bestMatch) {
+ } else if (cmp == 0) {
multiple = true;
}
}
if (multiple) {
- if (bestMatch == paramCount - 1) {
+ if (bestMatch.getExact() == paramCount - 1) {
// Only one parameter is not an exact match - try using the
// super class
match = resolveAmbiguousWrapper(candidates.keySet(),
paramTypes);
@@ -700,4 +710,56 @@ class Util {
return c.isVarArgs();
}
}
+
+ /*
+ * This class duplicates code in org.apache.el.util.ReflectionUtil. When
+ * making changes keep the code in sync.
+ */
+ private static class MatchResult implements Comparable<MatchResult> {
+
+ private final int exact;
+ private final int assignable;
+ private final int coercible;
+
+ public MatchResult(int exact, int assignable, int coercible) {
+ this.exact = exact;
+ this.assignable = assignable;
+ this.coercible = coercible;
+ }
+
+ public int getExact() {
+ return exact;
+ }
+
+ public int getAssignable() {
+ return assignable;
+ }
+
+ public int getCoercible() {
+ return coercible;
+ }
+
+ @Override
+ public int compareTo(MatchResult o) {
+ if (this.getExact() < o.getExact()) {
+ return -1;
+ } else if (this.getExact() > o.getExact()) {
+ return 1;
+ } else {
+ if (this.getAssignable() < o.getAssignable()) {
+ return -1;
+ } else if (this.getAssignable() > o.getAssignable()) {
+ return 1;
+ } else {
+ if (this.getCoercible() < o.getCoercible()) {
+ return -1;
+ } else if (this.getCoercible() > o.getCoercible()) {
+ return 1;
+ } else {
+ return 0;
+ }
+ }
+ }
+ }
+ }
}
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=1590120&r1=1590119&r2=1590120&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java (original)
+++ tomcat/trunk/java/org/apache/el/util/ReflectionUtil.java Fri Apr 25
19:49:46 2014
@@ -138,7 +138,7 @@ public class ReflectionUtil {
}
Method[] methods = base.getClass().getMethods();
- Map<Method,Integer> candidates = new HashMap<>();
+ Map<Method,MatchResult> candidates = new HashMap<>();
for (Method m : methods) {
if (!m.getName().equals(methodName)) {
@@ -163,6 +163,8 @@ public class ReflectionUtil {
// Check the parameters match
int exactMatch = 0;
+ int assignableMatch = 0;
+ int coercibleMatch = 0;
boolean noMatch = false;
for (int i = 0; i < mParamCount; i++) {
// Can't be null
@@ -171,12 +173,16 @@ public class ReflectionUtil {
} 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 (isAssignableFrom(paramTypes[j], varType)) {
+ assignableMatch++;
+ } else {
if (paramValues == null) {
noMatch = true;
break;
} else {
- if (!isCoercibleFrom(paramValues[j], varType))
{
+ if (isCoercibleFrom(paramValues[j], varType)) {
+ coercibleMatch++;
+ } else {
noMatch = true;
break;
}
@@ -186,12 +192,16 @@ public class ReflectionUtil {
// lead to a varArgs method matching when the result
// should be ambiguous
}
- } else if (!isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+ } else if (isAssignableFrom(paramTypes[i], mParamTypes[i])) {
+ assignableMatch++;
+ } else {
if (paramValues == null) {
noMatch = true;
break;
} else {
- if (!isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+ if (isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+ coercibleMatch++;
+ } else {
noMatch = true;
break;
}
@@ -208,26 +218,26 @@ public class ReflectionUtil {
return getMethod(base.getClass(), m);
}
- candidates.put(m, Integer.valueOf(exactMatch));
+ candidates.put(m, new MatchResult(exactMatch, assignableMatch,
coercibleMatch));
}
// Look for the method that has the highest number of parameters where
// the type matches exactly
- int bestMatch = 0;
+ MatchResult bestMatch = new MatchResult(0, 0, 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();
+ for (Map.Entry<Method, MatchResult> entry : candidates.entrySet()) {
+ int cmp = entry.getValue().compareTo(bestMatch);
+ if (cmp > 0 || match == null) {
+ bestMatch = entry.getValue();
match = entry.getKey();
multiple = false;
- } else if (entry.getValue().intValue() == bestMatch) {
+ } else if (cmp == 0) {
multiple = true;
}
}
if (multiple) {
- if (bestMatch == paramCount - 1) {
+ if (bestMatch.getExact() == paramCount - 1) {
// Only one parameter is not an exact match - try using the
// super class
match = resolveAmbiguousMethod(candidates.keySet(),
paramTypes);
@@ -430,4 +440,57 @@ public class ReflectionUtil {
}
return null;
}
+
+ /*
+ * This class duplicates code in javax.el.Util. When making changes keep
+ * the code in sync.
+ */
+ private static class MatchResult implements Comparable<MatchResult> {
+
+ private final int exact;
+ private final int assignable;
+ private final int coercible;
+
+ public MatchResult(int exact, int assignable, int coercible) {
+ this.exact = exact;
+ this.assignable = assignable;
+ this.coercible = coercible;
+ }
+
+ public int getExact() {
+ return exact;
+ }
+
+ public int getAssignable() {
+ return assignable;
+ }
+
+ public int getCoercible() {
+ return coercible;
+ }
+
+ @Override
+ public int compareTo(MatchResult o) {
+ if (this.getExact() < o.getExact()) {
+ return -1;
+ } else if (this.getExact() > o.getExact()) {
+ return 1;
+ } else {
+ if (this.getAssignable() < o.getAssignable()) {
+ return -1;
+ } else if (this.getAssignable() > o.getAssignable()) {
+ return 1;
+ } else {
+ if (this.getCoercible() < o.getCoercible()) {
+ return -1;
+ } else if (this.getCoercible() > o.getCoercible()) {
+ return 1;
+ } else {
+ return 0;
+ }
+ }
+ }
+ }
+ }
+
}
Modified: tomcat/trunk/test/javax/el/TestUtil.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestUtil.java?rev=1590120&r1=1590119&r2=1590120&view=diff
==============================================================================
--- tomcat/trunk/test/javax/el/TestUtil.java (original)
+++ tomcat/trunk/test/javax/el/TestUtil.java Fri Apr 25 19:49:46 2014
@@ -38,4 +38,12 @@ public class TestUtil {
Date result = (Date) processor.eval("Date(86400)");
Assert.assertEquals(86400, result.getTime());
}
+
+
+ @Test
+ public void testBug56425() {
+ ELProcessor processor = new ELProcessor();
+ processor.defineBean("string", "a-b-c-d");
+ Assert.assertEquals("a_b_c_d",
processor.eval("string.replace(\"-\",\"_\")"));
+ }
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1590120&r1=1590119&r2=1590120&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Apr 25 19:49:46 2014
@@ -185,6 +185,12 @@
<bug>56334</bug>: Fix a regression in the handling of back-slash
escaping introduced by the fix for <bug>55735</bug>. (markt)
</fix>
+ <fix>
+ <bug>56425</bug>: Improve method matching for EL expressions. When
+ looking for matching methods, an exact match between parameter types is
+ preferred followed by an assignable match followed by a coercible
match.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
