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 2021/07/16 14:10:12 UTC
[tomcat] 02/02: Additional fix for BZ 65358
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e3c1df49c7b99f20d3f2357fec2824e0b75e4de7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Jul 16 15:00:18 2021 +0100
Additional fix for BZ 65358
https://bz.apache.org/bugzilla/show_bug.cgi?id=65358
A method that does not declare varargs should always be preferred to a
method that does
---
java/jakarta/el/Util.java | 40 ++++++++++++++++--------
java/org/apache/el/util/ReflectionUtil.java | 40 ++++++++++++++++--------
test/org/apache/el/TestMethodExpressionImpl.java | 12 +++++++
test/org/apache/el/TesterBeanI.java | 29 +++++++++++++++++
4 files changed, 95 insertions(+), 26 deletions(-)
diff --git a/java/jakarta/el/Util.java b/java/jakarta/el/Util.java
index 080793a..80a6ee0 100644
--- a/java/jakarta/el/Util.java
+++ b/java/jakarta/el/Util.java
@@ -333,12 +333,13 @@ class Util {
return w;
}
- candidates.put(w, new MatchResult(exactMatch, assignableMatch, coercibleMatch, varArgsMatch, w.isBridge()));
+ candidates.put(w, new MatchResult(
+ w.isVarArgs(), exactMatch, assignableMatch, coercibleMatch, varArgsMatch, w.isBridge()));
}
// Look for the method that has the highest number of parameters where
// the type matches exactly
- MatchResult bestMatch = new MatchResult(0, 0, 0, 0, false);
+ MatchResult bestMatch = new MatchResult(true, 0, 0, 0, 0, true);
Wrapper<T> match = null;
boolean multiple = false;
for (Map.Entry<Wrapper<T>, MatchResult> entry : candidates.entrySet()) {
@@ -753,13 +754,16 @@ class Util {
*/
private static class MatchResult implements Comparable<MatchResult> {
+ private final boolean varArgs;
private final int exactCount;
private final int assignableCount;
private final int coercibleCount;
private final int varArgsCount;
private final boolean bridge;
- public MatchResult(int exactCount, int assignableCount, int coercibleCount, int varArgsCount, boolean bridge) {
+ public MatchResult(boolean varArgs, int exactCount, int assignableCount, int coercibleCount, int varArgsCount,
+ boolean bridge) {
+ this.varArgs = varArgs;
this.exactCount = exactCount;
this.assignableCount = assignableCount;
this.coercibleCount = coercibleCount;
@@ -767,6 +771,10 @@ class Util {
this.bridge = bridge;
}
+ public boolean isVarArgs() {
+ return varArgs;
+ }
+
public int getExactCount() {
return exactCount;
}
@@ -789,20 +797,24 @@ class Util {
@Override
public int compareTo(MatchResult o) {
- int cmp = Integer.compare(this.getExactCount(), o.getExactCount());
+ // Non-varArgs always beats varArgs
+ int cmp = Boolean.compare(o.isVarArgs(), this.isVarArgs());
if (cmp == 0) {
- cmp = Integer.compare(this.getAssignableCount(), o.getAssignableCount());
+ cmp = Integer.compare(this.getExactCount(), o.getExactCount());
if (cmp == 0) {
- cmp = Integer.compare(this.getCoercibleCount(), o.getCoercibleCount());
+ cmp = Integer.compare(this.getAssignableCount(), o.getAssignableCount());
if (cmp == 0) {
- // Fewer var args matches are better
- cmp = Integer.compare(o.getVarArgsCount(), this.getVarArgsCount());
+ cmp = Integer.compare(this.getCoercibleCount(), o.getCoercibleCount());
if (cmp == 0) {
- // The nature of bridge methods is such that it actually
- // doesn't matter which one we pick as long as we pick
- // one. That said, pick the 'right' one (the non-bridge
- // one) anyway.
- cmp = Boolean.compare(o.isBridge(), this.isBridge());
+ // Fewer var args matches are better
+ cmp = Integer.compare(o.getVarArgsCount(), this.getVarArgsCount());
+ if (cmp == 0) {
+ // The nature of bridge methods is such that it actually
+ // doesn't matter which one we pick as long as we pick
+ // one. That said, pick the 'right' one (the non-bridge
+ // one) anyway.
+ cmp = Boolean.compare(o.isBridge(), this.isBridge());
+ }
}
}
}
@@ -818,6 +830,7 @@ class Util {
((MatchResult)o).getAssignableCount() == this.getAssignableCount() &&
((MatchResult)o).getCoercibleCount() == this.getCoercibleCount() &&
((MatchResult)o).getVarArgsCount() == this.getVarArgsCount() &&
+ ((MatchResult)o).isVarArgs() == this.isVarArgs() &&
((MatchResult)o).isBridge() == this.isBridge());
}
@@ -829,6 +842,7 @@ class Util {
result = prime * result + (bridge ? 1231 : 1237);
result = prime * result + coercibleCount;
result = prime * result + exactCount;
+ result = prime * result + (varArgs ? 1231 : 1237);
result = prime * result + varArgsCount;
return result;
}
diff --git a/java/org/apache/el/util/ReflectionUtil.java b/java/org/apache/el/util/ReflectionUtil.java
index 80151d8..da32349 100644
--- a/java/org/apache/el/util/ReflectionUtil.java
+++ b/java/org/apache/el/util/ReflectionUtil.java
@@ -255,12 +255,13 @@ public class ReflectionUtil {
return getMethod(base.getClass(), base, m);
}
- candidates.put(m, new MatchResult(exactMatch, assignableMatch, coercibleMatch, varArgsMatch, m.isBridge()));
+ candidates.put(m, new MatchResult(
+ m.isVarArgs(), exactMatch, assignableMatch, coercibleMatch, varArgsMatch, m.isBridge()));
}
// Look for the method that has the highest number of parameters where
// the type matches exactly
- MatchResult bestMatch = new MatchResult(0, 0, 0, 0, false);
+ MatchResult bestMatch = new MatchResult(true, 0, 0, 0, 0, true);
Method match = null;
boolean multiple = false;
for (Map.Entry<Method, MatchResult> entry : candidates.entrySet()) {
@@ -511,13 +512,16 @@ public class ReflectionUtil {
*/
private static class MatchResult implements Comparable<MatchResult> {
+ private final boolean varArgs;
private final int exactCount;
private final int assignableCount;
private final int coercibleCount;
private final int varArgsCount;
private final boolean bridge;
- public MatchResult(int exactCount, int assignableCount, int coercibleCount, int varArgsCount, boolean bridge) {
+ public MatchResult(boolean varArgs, int exactCount, int assignableCount, int coercibleCount, int varArgsCount,
+ boolean bridge) {
+ this.varArgs = varArgs;
this.exactCount = exactCount;
this.assignableCount = assignableCount;
this.coercibleCount = coercibleCount;
@@ -525,6 +529,10 @@ public class ReflectionUtil {
this.bridge = bridge;
}
+ public boolean isVarArgs() {
+ return varArgs;
+ }
+
public int getExactCount() {
return exactCount;
}
@@ -547,20 +555,24 @@ public class ReflectionUtil {
@Override
public int compareTo(MatchResult o) {
- int cmp = Integer.compare(this.getExactCount(), o.getExactCount());
+ // Non-varArgs always beats varArgs
+ int cmp = Boolean.compare(o.isVarArgs(), this.isVarArgs());
if (cmp == 0) {
- cmp = Integer.compare(this.getAssignableCount(), o.getAssignableCount());
+ cmp = Integer.compare(this.getExactCount(), o.getExactCount());
if (cmp == 0) {
- cmp = Integer.compare(this.getCoercible(), o.getCoercible());
+ cmp = Integer.compare(this.getAssignableCount(), o.getAssignableCount());
if (cmp == 0) {
- // Fewer var args matches are better
- cmp = Integer.compare(o.getVarArgsCount(), this.getVarArgsCount());
+ cmp = Integer.compare(this.getCoercible(), o.getCoercible());
if (cmp == 0) {
- // The nature of bridge methods is such that it actually
- // doesn't matter which one we pick as long as we pick
- // one. That said, pick the 'right' one (the non-bridge
- // one) anyway.
- cmp = Boolean.compare(o.isBridge(), this.isBridge());
+ // Fewer var args matches are better
+ cmp = Integer.compare(o.getVarArgsCount(), this.getVarArgsCount());
+ if (cmp == 0) {
+ // The nature of bridge methods is such that it actually
+ // doesn't matter which one we pick as long as we pick
+ // one. That said, pick the 'right' one (the non-bridge
+ // one) anyway.
+ cmp = Boolean.compare(o.isBridge(), this.isBridge());
+ }
}
}
}
@@ -576,6 +588,7 @@ public class ReflectionUtil {
((MatchResult)o).getAssignableCount() == this.getAssignableCount() &&
((MatchResult)o).getCoercible() == this.getCoercible() &&
((MatchResult)o).getVarArgsCount() == this.getVarArgsCount() &&
+ ((MatchResult)o).isVarArgs() == this.isVarArgs() &&
((MatchResult)o).isBridge() == this.isBridge());
}
@@ -587,6 +600,7 @@ public class ReflectionUtil {
result = prime * result + (bridge ? 1231 : 1237);
result = prime * result + coercibleCount;
result = prime * result + exactCount;
+ result = prime * result + (varArgs ? 1231 : 1237);
result = prime * result + varArgsCount;
return result;
}
diff --git a/test/org/apache/el/TestMethodExpressionImpl.java b/test/org/apache/el/TestMethodExpressionImpl.java
index 3339d01..085a39f 100644
--- a/test/org/apache/el/TestMethodExpressionImpl.java
+++ b/test/org/apache/el/TestMethodExpressionImpl.java
@@ -749,4 +749,16 @@ public class TestMethodExpressionImpl {
String javaResult = func.apply(new TesterBeanH());
Assert.assertEquals(javaResult, elResult);
}
+
+
+ @Test
+ public void testPreferNoVarArgs() {
+ ELProcessor elp = new ELProcessor();
+ TesterBeanAAA bean = new TesterBeanAAA();
+ bean.setName("xyz");
+ elp.defineBean("bean2", bean);
+ elp.defineBean("bean1", new TesterBeanI());
+ String elResult = elp.eval("bean1.echo(bean2)");
+ Assert.assertEquals("No varargs: xyz", elResult);
+ }
}
diff --git a/test/org/apache/el/TesterBeanI.java b/test/org/apache/el/TesterBeanI.java
new file mode 100644
index 0000000..9372fc1
--- /dev/null
+++ b/test/org/apache/el/TesterBeanI.java
@@ -0,0 +1,29 @@
+/*
+ * 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 org.apache.el;
+
+public class TesterBeanI {
+
+ public String echo(TesterBeanA beanA) {
+ return "No varargs: " + beanA.getName();
+ }
+
+
+ public String echo(TesterBeanAAA beanAAA, @SuppressWarnings("unused") String... extras) {
+ return "With varargs: " + beanAAA.getName();
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org