This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 8a48f2973a79ed8c2dbc8435707cc680b99cb7ce 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/javax/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/javax/el/Util.java b/java/javax/el/Util.java index c3509b4..b659796 100644 --- a/java/javax/el/Util.java +++ b/java/javax/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 df1c750..c1162d1 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 78a9bff..0473174 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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org