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/04/25 21:49:46 UTC
svn commit: r1590120 - in /tomcat/trunk: java/javax/el/Util.java
java/org/apache/el/util/ReflectionUtil.java test/javax/el/TestUtil.java
webapps/docs/changelog.xml
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1590120 - in /tomcat/trunk: java/javax/el/Util.java java/org/apache/el/util/ReflectionUtil.java test/javax/el/TestUtil.java webapps/docs/changelog.xml
Posted by Konstantin Preißer <kp...@apache.org>.
Hi Mark,
> -----Original Message-----
> From: Mark Thomas [mailto:markt@apache.org]
> Sent: Friday, April 25, 2014 10:43 PM
<snip>
> OK for Tomcat 8 then but not Tomcat 7.
>
> You have commit privs. It is all yours.
OK, done.
Regards,
Konstantin Preißer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1590120 - in /tomcat/trunk: java/javax/el/Util.java
java/org/apache/el/util/ReflectionUtil.java test/javax/el/TestUtil.java webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 25/04/2014 21:41, Konstantin Preißer wrote:
> Hi Mark,
<snip/>
> What about using Integer.compare(int, int), e.g.:
>
> @Override
> public int compareTo(MatchResult o) {
> int cmp = Integer.compare(this.getExact(), o.getExact());
> if (cmp == 0) {
> cmp = Integer.compare(this.getAssignable(), o.getAssignable());
> if (cmp == 0) {
> cmp = Integer.compare(this.getCoercible(), o.getCoercible());
> }
> }
> return cmp;
> }
>
> (but according to Java API documentation, Integer.compare(int, int) is only available since Java 1.7)
OK for Tomcat 8 then but not Tomcat 7.
You have commit privs. It is all yours.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
RE: svn commit: r1590120 - in /tomcat/trunk: java/javax/el/Util.java java/org/apache/el/util/ReflectionUtil.java test/javax/el/TestUtil.java webapps/docs/changelog.xml
Posted by Konstantin Preißer <kp...@apache.org>.
Hi Mark,
> -----Original Message-----
> From: markt@apache.org [mailto:markt@apache.org]
> Sent: Friday, April 25, 2014 9:50 PM
<snip>
> + @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;
> + }
> + }
> + }
> + }
What about using Integer.compare(int, int), e.g.:
@Override
public int compareTo(MatchResult o) {
int cmp = Integer.compare(this.getExact(), o.getExact());
if (cmp == 0) {
cmp = Integer.compare(this.getAssignable(), o.getAssignable());
if (cmp == 0) {
cmp = Integer.compare(this.getCoercible(), o.getCoercible());
}
}
return cmp;
}
(but according to Java API documentation, Integer.compare(int, int) is only available since Java 1.7)
Regards,
Konstantin Preißer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org