This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 145924387aae9b914398b00c49814608d1db92c3 Author: Mark Thomas <[email protected]> AuthorDate: Fri Jul 4 18:21:38 2025 +0100 Fix broken tests. If obj0 != obj1 and either is null, the algorithm for relational operators terminates before reaching the Comparable section. So these objects should not not null if processing gets as far as the Comparable section. --- java/org/apache/el/Messages.properties | 3 +++ java/org/apache/el/lang/ELSupport.java | 26 ++++++++++++++------------ test/org/apache/el/TestELEvaluation.java | 18 +++++++++++------- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/java/org/apache/el/Messages.properties b/java/org/apache/el/Messages.properties index c093280899..b71f6ab4e3 100644 --- a/java/org/apache/el/Messages.properties +++ b/java/org/apache/el/Messages.properties @@ -15,6 +15,9 @@ # General Errors error.cannotSetVariables=Cannot set variables on factory +error.compare=Cannot compare [{0}] to [{1}] +error.compare.null=Unable to compare null with non-null object +error.context.null=ELContext was null error.convert=Cannot convert [{0}] of type [{1}] to [{2}] error.compare=Cannot compare [{0}] to [{1}] error.function=Problems calling function [{0}] diff --git a/java/org/apache/el/lang/ELSupport.java b/java/org/apache/el/lang/ELSupport.java index c5ee9a2a42..a2c292dfca 100644 --- a/java/org/apache/el/lang/ELSupport.java +++ b/java/org/apache/el/lang/ELSupport.java @@ -25,6 +25,7 @@ import java.security.AccessController; import java.security.PrivilegedAction; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Set; import javax.el.ELContext; @@ -61,21 +62,19 @@ public class ELSupport { * If the objects are identical, or they are equal according to {@link #equals(ELContext, Object, Object)} then * return 0. * <p> + * If either object is null, error + * <p> * If either object is a BigDecimal, then coerce both to BigDecimal first. Similarly for Double(Float), BigInteger, * and Long(Integer, Char, Short, Byte). * <p> - * Otherwise, check that the first object is an instance of Comparable, and compare against the second object. If - * that is null, return 1, otherwise return the result of comparing against the second object. + * If either object is TemporalAccessor, Clock, java.util.Date or java.sql.Timestamp, coerce both to Instant and + * then compare. + * <p> + * If the first object is an instance of Comparable, return the result of comparing against the second object. * <p> - * Similarly, if the second object is Comparable, if the first is null, return -1, else return the result of - * comparing against the first object. + * If the second object is an instance of Comparable, return -1 * the result of comparing against the first object. * <p> - * A null object is considered as: - * <ul> - * <li>ZERO when compared with Numbers</li> - * <li>the empty string for String compares</li> - * <li>Otherwise null is considered to be lower than anything else.</li> - * </ul> + * Otherwise, error. * * @param ctx the context in which this comparison is taking place * @param obj0 first object @@ -91,6 +90,9 @@ public class ELSupport { if (obj0 == obj1 || equals(ctx, obj0, obj1)) { return 0; } + Objects.requireNonNull(obj0, MessageFactory.get("error.compare.null")); + Objects.requireNonNull(obj1, MessageFactory.get("error.compare.null")); + if (isBigDecimalOp(obj0, obj1)) { BigDecimal bd0 = (BigDecimal) coerceToNumber(ctx, obj0, BigDecimal.class); BigDecimal bd1 = (BigDecimal) coerceToNumber(ctx, obj1, BigDecimal.class); @@ -117,12 +119,12 @@ public class ELSupport { if (obj0 instanceof Comparable<?>) { @SuppressWarnings("unchecked") // checked above final Comparable<Object> comparable = (Comparable<Object>) obj0; - return (obj1 != null) ? comparable.compareTo(obj1) : 1; + return comparable.compareTo(obj1); } if (obj1 instanceof Comparable<?>) { @SuppressWarnings("unchecked") // checked above final Comparable<Object> comparable = (Comparable<Object>) obj1; - return (obj0 != null) ? -comparable.compareTo(obj0) : -1; + return -comparable.compareTo(obj0); } throw new ELException(MessageFactory.get("error.compare", obj0, obj1)); } diff --git a/test/org/apache/el/TestELEvaluation.java b/test/org/apache/el/TestELEvaluation.java index 6646c3a9b8..323e09d22f 100644 --- a/test/org/apache/el/TestELEvaluation.java +++ b/test/org/apache/el/TestELEvaluation.java @@ -160,24 +160,28 @@ public class TestELEvaluation { private void compareBoth(String msg, int expected, Object o1, Object o2) { int i1 = ELSupport.compare(null, o1, o2); int i2 = ELSupport.compare(null, o2, o1); - Assert.assertEquals(msg, expected, i1); - Assert.assertEquals(msg, expected, -i2); + if (expected == -1) { + Assert.assertTrue(msg, i1 < 0); + Assert.assertTrue(msg, i2 > 0); + } else if (expected == 0) { + Assert.assertTrue(msg, i1 == 0); + Assert.assertTrue(msg, i2 == 0); + } else { + Assert.assertTrue(msg, i1 > 0); + Assert.assertTrue(msg, i2 < 0); + } } @Test public void testElSupportCompare() { compareBoth("Nulls should compare equal", 0, null, null); - compareBoth("Null should compare equal to \"\"", 0, "", null); - compareBoth("Null should be less than File()", -1, null, new File("")); - compareBoth("Null should be less than Date()", -1, null, new Date()); compareBoth("Date(0) should be less than Date(1)", -1, new Date(0), new Date(1)); try { compareBoth("Should not compare", 0, new Date(), new File("")); Assert.fail("Expecting ClassCastException"); - } catch (ClassCastException expected) { + } catch (ELException expected) { // Expected } - Assert.assertTrue(null == null); } /** --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
