This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a2cfa7250a23ddadc8a6284fc56186ba809f83b7 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/LocalStrings.properties | 1 + java/org/apache/el/lang/ELSupport.java | 26 ++++++++++++++------------ test/org/apache/el/TestELEvaluation.java | 18 +++++++++++------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/java/org/apache/el/LocalStrings.properties b/java/org/apache/el/LocalStrings.properties index f11270f5e8..3915106e60 100644 --- a/java/org/apache/el/LocalStrings.properties +++ b/java/org/apache/el/LocalStrings.properties @@ -20,6 +20,7 @@ elSupport.coerce.nonAbstract=Unable to coerce a LambdaExpression to the function 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.fnMapper.method=Function [{0}] not found diff --git a/java/org/apache/el/lang/ELSupport.java b/java/org/apache/el/lang/ELSupport.java index 73df04c372..b92d290f79 100644 --- a/java/org/apache/el/lang/ELSupport.java +++ b/java/org/apache/el/lang/ELSupport.java @@ -28,6 +28,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 java.util.function.Supplier; @@ -66,21 +67,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 @@ -96,6 +95,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); @@ -122,12 +124,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 fb560b1569..89b76696eb 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]
