This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new f5bf27ca JEXL-420: improved arithmetic coherence; - added tests;
f5bf27ca is described below
commit f5bf27caaf7c577ed039232e5db81bcb835bc228
Author: Henri Biestro <hbies...@cloudera.com>
AuthorDate: Fri Feb 9 10:03:13 2024 +0100
JEXL-420: improved arithmetic coherence;
- added tests;
---
RELEASE-NOTES.txt | 1 +
src/changes/changes.xml | 3 +
.../org/apache/commons/jexl3/JexlArithmetic.java | 141 ++++++++++++---------
.../org/apache/commons/jexl3/ArithmeticTest.java | 27 +++-
4 files changed, 109 insertions(+), 63 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index a059780d..d8bac03e 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -38,6 +38,7 @@ New Features in 3.3.1:
Bugs Fixed in 3.3.1:
===================
+* JEXL-420: Error while comparing float and string value
* JEXL-417: JexlArithmetic looses precision during arithmetic operator
execution
* JEXL-416: Null-valued pragma throws NPE in 3.3
* JEXL-415: Incorrect template eval result
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index b614ba05..b5666812 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -46,6 +46,9 @@
</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">Add Maven
property project.build.outputTimestamp for build reproducibility.</action>
<!-- FIX -->
+ <action dev="henrib" type="fix" issue="JEXL-420" due-to="Xu
Pengcheng">
+ Error while comparing float and string value
+ </action>
<action dev="henrib" type="fix" issue="JEXL-417" due-to="Robert
Lucas" >
JexlArithmetic looses precision during arithmetic operator
execution
</action>
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 006bc426..044c3791 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -18,6 +18,7 @@
package org.apache.commons.jexl3;
import static java.lang.StrictMath.floor;
+import static org.apache.commons.jexl3.JexlOperator.EQ;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
@@ -58,12 +59,24 @@ import org.apache.commons.jexl3.introspection.JexlMethod;
* @since 2.0
*/
public class JexlArithmetic {
-
/** Marker class for null operand exceptions. */
public static class NullOperand extends ArithmeticException {
private static final long serialVersionUID = 4720876194840764770L;
}
+ /** Marker class for coercion operand exceptions. */
+ public static class CoercionException extends ArithmeticException {
+ private static final long serialVersionUID = 202402081150L;
+
+ /**
+ * Simple ctor.
+ * @param msg the exception message
+ */
+ public CoercionException(String msg) {
+ super(msg);
+ }
+ }
+
/** Double.MAX_VALUE as BigDecimal. */
protected static final BigDecimal BIGD_DOUBLE_MAX_VALUE =
BigDecimal.valueOf(Double.MAX_VALUE);
@@ -525,7 +538,7 @@ public class JexlArithmetic {
if (val == null) {
return controlNullOperand(strict, 0);
}
- throw new ArithmeticException("Integer coercion: "
+ throw new CoercionException("Integer coercion: "
+ val.getClass().getName() + ":(" + val + ")");
}
@@ -576,7 +589,7 @@ public class JexlArithmetic {
if (val == null) {
return controlNullOperand(strict, 0L);
}
- throw new ArithmeticException("Long coercion: "
+ throw new CoercionException("Long coercion: "
+ val.getClass().getName() + ":(" + val + ")");
}
@@ -637,7 +650,7 @@ public class JexlArithmetic {
if (val == null) {
return controlNullOperand(strict, BigInteger.ZERO);
}
- throw new ArithmeticException("BigInteger coercion: "
+ throw new CoercionException("BigInteger coercion: "
+ val.getClass().getName() + ":(" + val + ")");
}
@@ -676,7 +689,7 @@ public class JexlArithmetic {
return roundBigDecimal(new BigDecimal(val.toString(),
getMathContext()));
}
if (val instanceof Number) {
- return roundBigDecimal(new BigDecimal(val.toString(),
getMathContext()));
+ return roundBigDecimal(parseBigDecimal(val.toString()));
}
if (val instanceof Boolean) {
return BigDecimal.valueOf((Boolean) val ? 1. : 0.);
@@ -685,20 +698,15 @@ public class JexlArithmetic {
return BigDecimal.valueOf(((AtomicBoolean) val).get() ? 1L : 0L);
}
if (val instanceof String) {
- final String string = (String) val;
- if (string.isEmpty()) {
- return BigDecimal.ZERO;
- }
- return roundBigDecimal(new BigDecimal(string, getMathContext()));
+ return roundBigDecimal(parseBigDecimal((String) val));
}
if (val instanceof Character) {
- final int i = (Character) val;
- return new BigDecimal(i);
+ return new BigDecimal((Character) val);
}
if (val == null) {
return controlNullOperand(strict, BigDecimal.ZERO);
}
- throw new ArithmeticException("BigDecimal coercion: "
+ throw new CoercionException("BigDecimal coercion: "
+ val.getClass().getName() + ":(" + val + ")");
}
@@ -731,9 +739,7 @@ public class JexlArithmetic {
return (Double) val;
}
if (val instanceof Number) {
- //The below construct is used rather than
((Number)val).doubleValue() to ensure
- //equality between comparing new Double( 6.4 / 3 ) and the jexl
expression of 6.4 / 3
- return Double.parseDouble(String.valueOf(val));
+ return ((Number) val).doubleValue();
}
if (val instanceof Boolean) {
return (Boolean) val ? 1. : 0.;
@@ -750,7 +756,7 @@ public class JexlArithmetic {
if (val == null) {
return controlNullOperand(strict, 0.d);
}
- throw new ArithmeticException("Double coercion: "
+ throw new CoercionException("Double coercion: "
+ val.getClass().getName() + ":(" + val + ")");
}
@@ -1937,7 +1943,7 @@ public class JexlArithmetic {
operator = JexlOperator.valueOf(symbol);
} catch (final IllegalArgumentException xill) {
// ignore
- operator = JexlOperator.EQ;
+ operator = EQ;
}
return doCompare(left, right, operator);
}
@@ -1982,54 +1988,54 @@ public class JexlArithmetic {
private int doCompare(final Object left, final Object right, final
JexlOperator operator) {
final boolean strictCast = isStrict(operator);
if (left != null && right != null) {
- if (left instanceof BigDecimal || right instanceof BigDecimal) {
- final BigDecimal l = toBigDecimal(strictCast, left);
- final BigDecimal r = toBigDecimal(strictCast, right);
- return l.compareTo(r);
- }
- if (left instanceof BigInteger || right instanceof BigInteger) {
- try {
+ try {
+ if (left instanceof BigDecimal || right instanceof BigDecimal)
{
+ final BigDecimal l = toBigDecimal(strictCast, left);
+ final BigDecimal r = toBigDecimal(strictCast, right);
+ return l.compareTo(r);
+ }
+ if (left instanceof BigInteger || right instanceof BigInteger)
{
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
return l.compareTo(r);
- } catch (final ArithmeticException xconvert) {
- // ignore it, continue in sequence
}
- }
- if (isFloatingPoint(left) || isFloatingPoint(right)) {
- final double lhs = toDouble(strictCast, left);
- final double rhs = toDouble(strictCast, right);
- if (Double.isNaN(lhs)) {
+ if (isFloatingPoint(left) || isFloatingPoint(right)) {
+ final double lhs = toDouble(strictCast, left);
+ final double rhs = toDouble(strictCast, right);
+ if (Double.isNaN(lhs)) {
+ if (Double.isNaN(rhs)) {
+ return 0;
+ }
+ return -1;
+ }
if (Double.isNaN(rhs)) {
- return 0;
+ // lhs is not NaN
+ return +1;
}
- return -1;
- }
- if (Double.isNaN(rhs)) {
- // lhs is not NaN
- return +1;
+ return Double.compare(lhs, rhs);
}
- return Double.compare(lhs, rhs);
- }
- if (isNumberable(left) || isNumberable(right)) {
- try {
+ if (isNumberable(left) || isNumberable(right)) {
final long lhs = toLong(strictCast, left);
final long rhs = toLong(strictCast, right);
return Long.compare(lhs, rhs);
- } catch (final ArithmeticException xconvert) {
- // ignore it, continue in sequence
}
+ if (left instanceof String || right instanceof String) {
+ return toString(left).compareTo(toString(right));
+ }
+ } catch (final CoercionException ignore) {
+ // ignore it, continue in sequence
}
- if (left instanceof String || right instanceof String) {
- return toString(left).compareTo(toString(right));
- }
- if (JexlOperator.EQ == operator) {
+ if (EQ == operator) {
return left.equals(right) ? 0 : -1;
}
if (left instanceof Comparable<?>) {
@SuppressWarnings("unchecked") // OK because of instanceof
check above
final Comparable<Object> comparable = (Comparable<Object>)
left;
- return comparable.compareTo(right);
+ try {
+ return comparable.compareTo(right);
+ } catch(ClassCastException castException) {
+ // ignore it, continue in sequence
+ }
}
}
throw new ArithmeticException("Object comparison:(" + left + " " +
operator + " " + right + ")");
@@ -2049,11 +2055,11 @@ public class JexlArithmetic {
if (left == null || right == null) {
return false;
}
- final boolean strictCast = isStrict(JexlOperator.EQ);
+ final boolean strictCast = isStrict(EQ);
if (left instanceof Boolean || right instanceof Boolean) {
return toBoolean(left) == toBoolean(strictCast, right);
}
- return compare(left, right, JexlOperator.EQ) == 0;
+ return compare(left, right, EQ) == 0;
}
/**
@@ -2130,7 +2136,7 @@ public class JexlArithmetic {
try {
return arg.isEmpty()? Double.NaN : Double.parseDouble(arg);
} catch (final NumberFormatException e) {
- final ArithmeticException arithmeticException = new
ArithmeticException("Double coercion: ("+ arg +")");
+ final ArithmeticException arithmeticException = new
CoercionException("Double coercion: ("+ arg +")");
arithmeticException.initCause(e);
throw arithmeticException;
}
@@ -2152,7 +2158,7 @@ public class JexlArithmetic {
if (d == f) {
return (long) d;
}
- throw new ArithmeticException("Long coercion: ("+ arg +")");
+ throw new CoercionException("Long coercion: ("+ arg +")");
}
/**
@@ -2168,7 +2174,7 @@ public class JexlArithmetic {
if (i == l) {
return i;
}
- throw new ArithmeticException("Int coercion: ("+ arg +")");
+ throw new CoercionException("Int coercion: ("+ arg +")");
}
/**
@@ -2179,15 +2185,30 @@ public class JexlArithmetic {
* @throws ArithmeticException if the string can not be coerced into a big
integer
*/
private BigInteger parseBigInteger(final String arg) throws
ArithmeticException {
- if (arg.isEmpty()) {
- return BigInteger.ZERO;
+ try {
+ return arg.isEmpty()? BigInteger.ZERO : new BigInteger(arg);
+ } catch (final NumberFormatException e) {
+ final ArithmeticException arithmeticException = new
CoercionException("BigDecimal coercion: ("+ arg +")");
+ arithmeticException.initCause(e);
+ throw arithmeticException;
}
+ }
+
+ /**
+ * Convert a string to a BigDecimal.
+ * <>Empty string is considered as 0.</>
+ * @param arg the arg
+ * @return a BigDecimal
+ * @throws CoercionException if the string can not be coerced into a
BigDecimal
+ */
+ private BigDecimal parseBigDecimal(final String arg) throws
ArithmeticException {
try {
- return new BigInteger(arg);
- } catch (final NumberFormatException xformat) {
- // ignore, try harder
+ return arg.isEmpty()? BigDecimal.ZERO : new BigDecimal(arg,
getMathContext());
+ } catch (final NumberFormatException e) {
+ final ArithmeticException arithmeticException = new
CoercionException("BigDecimal coercion: ("+ arg +")");
+ arithmeticException.initCause(e);
+ throw arithmeticException;
}
- return BigInteger.valueOf(parseLong(arg));
}
/**
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
index fa019b6c..23ecabf6 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
@@ -1089,6 +1089,12 @@ public class ArithmeticTest extends JexlTestCase {
"'1.00000001' == 1h", false,
"1.0 >= '1'", true,
"1.0 > '1'", false,
+ "1.0 == 'a'", false,
+ "10 == 'a'", false,
+ "10 > 'a'", ArithmeticException.class,
+ "10.0 > 'a'", ArithmeticException.class,
+ "'a' <= 10b", ArithmeticException.class,
+ "'a' >= 1h", ArithmeticException.class
};
final JexlEngine jexl = new JexlBuilder().create();
final JexlContext jc = new EmptyTestContext();
@@ -1098,8 +1104,15 @@ public class ArithmeticTest extends JexlTestCase {
final String stext = (String) EXPRESSIONS[e];
final Object expected = EXPRESSIONS[e + 1];
expression = jexl.createExpression(stext);
- final Object result = expression.evaluate(jc);
- Assert.assertEquals("failed on " + stext, expected, result);
+ try {
+ final Object result = expression.evaluate(jc);
+ Assert.assertEquals("failed on " + stext, expected, result);
+ } catch(JexlException jexlException) {
+ Throwable cause = jexlException.getCause();
+ if (cause == null || !cause.getClass().equals(expected)) {
+ Assert.fail(stext);
+ }
+ }
}
}
@@ -1769,7 +1782,7 @@ public class ArithmeticTest extends JexlTestCase {
}
@Test
- public void testRealCoercionEdges() {
+ public void testRealCoercionEdges() throws Exception {
assertNullOperand(() -> jexla.toDouble(null));
Assert.assertEquals(0.0d, jexlb.toDouble(null), EPSILON);
Assert.assertEquals(32.0d, jexlb.toDouble((char) 32), EPSILON);
@@ -1789,6 +1802,14 @@ public class ArithmeticTest extends JexlTestCase {
Assert.assertEquals(BigDecimal.ZERO, jexla.toBigDecimal(Double.NaN));
Assert.assertEquals(BigDecimal.ZERO, jexla.toBigDecimal(""));
Assert.assertEquals(BigDecimal.ZERO, jexla.toBigDecimal((char) 0));
+
+ Double d64d3 = new Double( 6.4 / 3 );
+ Assert.assertEquals(d64d3, ((Number) JEXL.createExpression("6.4 /
3").evaluate(null)).doubleValue(), EPSILON);
+ asserter.assertExpression("6.4 / 3", d64d3);
+ Assert.assertEquals(d64d3, ((Number) JEXL.createExpression("6.4 /
3d").evaluate(null)).doubleValue(), EPSILON);
+ asserter.assertExpression("6.4 / 3d", d64d3);
+ Assert.assertEquals(64d / 3, ((Number) JEXL.createExpression("64d /
3").evaluate(null)).doubleValue(), EPSILON);
+ asserter.assertExpression("64 / 3d", 64 / 3d);
}
@Test
