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 5ab46d99 JEXL-417: fix precision loss in JexlArithmetic; - added tests;
5ab46d99 is described below
commit 5ab46d9935c992eab7ab7dce4f1fde196976c789
Author: Henri Biestro <hbies...@cloudera.com>
AuthorDate: Tue Nov 28 10:48:30 2023 +0100
JEXL-417: fix precision loss in JexlArithmetic;
- added tests;
---
RELEASE-NOTES.txt | 1 +
src/changes/changes.xml | 3 +
.../org/apache/commons/jexl3/JexlArithmetic.java | 125 ++++++++++++---------
.../org/apache/commons/jexl3/ArithmeticTest.java | 51 ++++++---
4 files changed, 114 insertions(+), 66 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 5de131f8..b53f7d24 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -37,6 +37,7 @@ New Features in 3.3.1:
Bugs Fixed in 3.3.1:
===================
+* 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
* JEXL-414: SoftCache may suffer from race conditions
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d058c067..e626ca33 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -42,6 +42,9 @@
Allow 'trailing commas' or ellipsis while defining array, map
and set literals
</action>
<!-- FIX -->
+ <action dev="henrib" type="fix" issue="JEXL-417" due-to="Robert
Lucas" >
+ JexlArithmetic looses precision during arithmetic operator
execution
+ </action>
<action dev="henrib" type="fix" issue="JEXL-416" due-to="William
Price" >
Null-valued pragma throws NPE in 3.3
</action>
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index c51d11a3..701422ba 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -29,6 +29,7 @@ import java.math.MathContext;
import java.util.Collection;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -1062,14 +1063,14 @@ public class JexlArithmetic {
protected Number narrowBigInteger(final Object lhs, final Object rhs,
final BigInteger bigi) {
//coerce to long if possible
if ((isNumberable(lhs) || isNumberable(rhs))
- && bigi.compareTo(BIGI_LONG_MAX_VALUE) <= 0
- && bigi.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
+ && bigi.compareTo(BIGI_LONG_MAX_VALUE) <= 0
+ && bigi.compareTo(BIGI_LONG_MIN_VALUE) >= 0) {
// coerce to int if possible
final long l = bigi.longValue();
// coerce to int when possible (int being so often used in method
parms)
if (!(lhs instanceof Long || rhs instanceof Long)
- && l <= Integer.MAX_VALUE
- && l >= Integer.MIN_VALUE) {
+ && l <= Integer.MAX_VALUE
+ && l >= Integer.MIN_VALUE) {
return (int) l;
}
return l;
@@ -1079,7 +1080,7 @@ public class JexlArithmetic {
/**
* Given a BigDecimal, attempt to narrow it to an Integer or Long if it
fits and
- * one of the arguments is a numberable.
+ * one of the arguments is numberable.
*
* @param lhs the left-hand side operand that lead to the bigd result
* @param rhs the right-hand side operand that lead to the bigd result
@@ -1144,17 +1145,42 @@ public class JexlArithmetic {
/**
* Checks if value class is a number that can be represented exactly in a
long.
+ * <p>For convenience, booleans are converted as 1/0 (true/false).</p>
*
* @param value argument
* @return true if argument can be represented by a long
*/
protected Number asLongNumber(final Object value) {
- return value instanceof Long
- || value instanceof Integer
- || value instanceof Short
- || value instanceof Byte
- ? (Number) value
- : null;
+ return asLongNumber(strict, value);
+ }
+
+ /**
+ * Checks if value class is a number that can be represented exactly in a
long.
+ * <p>For convenience, booleans are converted as 1/0 (true/false).</p>
+ *
+ * @param strict whether null argument is converted as 0 or remains null
+ * @param value argument
+ * @return a non-null value if argument can be represented by a long
+ */
+ protected Number asLongNumber(final boolean strict, final Object value) {
+ if (value instanceof Long
+ || value instanceof Integer
+ || value instanceof Short
+ || value instanceof Byte) {
+ return (Number) value;
+ }
+ if (value instanceof Boolean) {
+ Boolean b = (Boolean) value;
+ return b ? 1L : 0L;
+ }
+ if (value instanceof AtomicBoolean) {
+ AtomicBoolean b = (AtomicBoolean) value;
+ return b.get() ? 1L : 0L;
+ }
+ if (value == null && !strict) {
+ return 0L;
+ }
+ return null;
}
/**
@@ -1234,9 +1260,10 @@ public class JexlArithmetic {
: left instanceof String && right instanceof
String;
if (!strconcat) {
try {
+ final boolean strictCast = isStrict(JexlOperator.ADD);
// if both (non-null) args fit as long
- final Number ln = asLongNumber(left);
- final Number rn = asLongNumber(right);
+ final Number ln = asLongNumber(strictCast, left);
+ final Number rn = asLongNumber(strictCast, right);
if (ln != null && rn != null) {
final long x = ln.longValue();
final long y = rn.longValue();
@@ -1247,21 +1274,19 @@ public class JexlArithmetic {
}
return narrowLong(left, right, result);
}
- final boolean strictCast = isStrict(JexlOperator.ADD);
- // if either are bigdecimal use that type
+ // if either are BigDecimal, use that type
if (left instanceof BigDecimal || right instanceof BigDecimal)
{
final BigDecimal l = toBigDecimal(strictCast, left);
final BigDecimal r = toBigDecimal(strictCast, right);
- final BigDecimal result = l.add(r, getMathContext());
- return narrowBigDecimal(left, right, result);
+ return l.add(r, getMathContext());
}
- // if either are floating point (double or float) use double
+ // if either are floating point (double or float), use double
if (isFloatingPointNumber(left) ||
isFloatingPointNumber(right)) {
final double l = toDouble(strictCast, left);
final double r = toDouble(strictCast, right);
return l + r;
}
- // otherwise treat as (big) integers
+ // otherwise treat as BigInteger
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
final BigInteger result = l.add(r);
@@ -1285,9 +1310,10 @@ public class JexlArithmetic {
if (left == null && right == null) {
return controlNullNullOperands(JexlOperator.DIVIDE);
}
+ final boolean strictCast = isStrict(JexlOperator.DIVIDE);
// if both (non-null) args fit as long
- final Number ln = asLongNumber(left);
- final Number rn = asLongNumber(right);
+ final Number ln = asLongNumber(strictCast, left);
+ final Number rn = asLongNumber(strictCast, right);
if (ln != null && rn != null) {
final long x = ln.longValue();
final long y = rn.longValue();
@@ -1297,18 +1323,16 @@ public class JexlArithmetic {
final long result = x / y;
return narrowLong(left, right, result);
}
- final boolean strictCast = isStrict(JexlOperator.DIVIDE);
- // if either are bigdecimal use that type
+ // if either are BigDecimal, use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
final BigDecimal l = toBigDecimal(strictCast, left);
final BigDecimal r = toBigDecimal(strictCast, right);
if (BigDecimal.ZERO.equals(r)) {
throw new ArithmeticException("/");
}
- final BigDecimal result = l.divide(r, getMathContext());
- return narrowBigDecimal(left, right, result);
+ return l.divide(r, getMathContext());
}
- // if either are floating point (double or float) use double
+ // if either are floating point (double or float), use double
if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
final double l = toDouble(strictCast, left);
final double r = toDouble(strictCast, right);
@@ -1317,7 +1341,7 @@ public class JexlArithmetic {
}
return l / r;
}
- // otherwise treat as integers
+ // otherwise treat as BigInteger
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
if (BigInteger.ZERO.equals(r)) {
@@ -1339,9 +1363,10 @@ public class JexlArithmetic {
if (left == null && right == null) {
return controlNullNullOperands(JexlOperator.MOD);
}
+ final boolean strictCast = isStrict(JexlOperator.MOD);
// if both (non-null) args fit as long
- final Number ln = asLongNumber(left);
- final Number rn = asLongNumber(right);
+ final Number ln = asLongNumber(strictCast, left);
+ final Number rn = asLongNumber(strictCast, right);
if (ln != null && rn != null) {
final long x = ln.longValue();
final long y = rn.longValue();
@@ -1351,18 +1376,16 @@ public class JexlArithmetic {
final long result = x % y;
return narrowLong(left, right, result);
}
- final boolean strictCast = isStrict(JexlOperator.MOD);
- // if either are bigdecimal use that type
+ // if either are BigDecimal, use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
final BigDecimal l = toBigDecimal(strictCast, left);
final BigDecimal r = toBigDecimal(strictCast, right);
if (BigDecimal.ZERO.equals(r)) {
throw new ArithmeticException("%");
}
- final BigDecimal remainder = l.remainder(r, getMathContext());
- return narrowBigDecimal(left, right, remainder);
+ return l.remainder(r, getMathContext());
}
- // if either are floating point (double or float) use double
+ // if either are floating point (double or float), use double
if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
final double l = toDouble(strictCast, left);
final double r = toDouble(strictCast, right);
@@ -1371,7 +1394,7 @@ public class JexlArithmetic {
}
return l % r;
}
- // otherwise treat as integers
+ // otherwise treat as BigInteger
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
if (BigInteger.ZERO.equals(r)) {
@@ -1412,9 +1435,10 @@ public class JexlArithmetic {
if (left == null && right == null) {
return controlNullNullOperands(JexlOperator.MULTIPLY);
}
- // if both (non-null) args fit as int
- final Number ln = asLongNumber(left);
- final Number rn = asLongNumber(right);
+ final boolean strictCast = isStrict(JexlOperator.MULTIPLY);
+ // if both (non-null) args fit as long
+ final Number ln = asLongNumber(strictCast, left);
+ final Number rn = asLongNumber(strictCast, right);
if (ln != null && rn != null) {
final long x = ln.longValue();
final long y = rn.longValue();
@@ -1425,21 +1449,19 @@ public class JexlArithmetic {
}
return narrowLong(left, right, result);
}
- final boolean strictCast = isStrict(JexlOperator.MULTIPLY);
- // if either are bigdecimal use that type
+ // if either are BigDecimal, use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
final BigDecimal l = toBigDecimal(strictCast, left);
final BigDecimal r = toBigDecimal(strictCast, right);
- final BigDecimal result = l.multiply(r, getMathContext());
- return narrowBigDecimal(left, right, result);
+ return l.multiply(r, getMathContext());
}
- // if either are floating point (double or float) use double
+ // if either are floating point (double or float), use double
if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
final double l = toDouble(strictCast, left);
final double r = toDouble(strictCast, right);
return l * r;
}
- // otherwise treat as integers
+ // otherwise treat as BigInteger
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
final BigInteger result = l.multiply(r);
@@ -1457,9 +1479,10 @@ public class JexlArithmetic {
if (left == null && right == null) {
return controlNullNullOperands(JexlOperator.SUBTRACT);
}
+ final boolean strictCast = isStrict(JexlOperator.SUBTRACT);
// if both (non-null) args fit as long
- final Number ln = asLongNumber(left);
- final Number rn = asLongNumber(right);
+ final Number ln = asLongNumber(strictCast, left);
+ final Number rn = asLongNumber(strictCast, right);
if (ln != null && rn != null) {
final long x = ln.longValue();
final long y = rn.longValue();
@@ -1470,21 +1493,19 @@ public class JexlArithmetic {
}
return narrowLong(left, right, result);
}
- final boolean strictCast = isStrict(JexlOperator.SUBTRACT);
- // if either are bigdecimal use that type
+ // if either are BigDecimal, use that type
if (left instanceof BigDecimal || right instanceof BigDecimal) {
final BigDecimal l = toBigDecimal(strictCast, left);
final BigDecimal r = toBigDecimal(strictCast, right);
- final BigDecimal result = l.subtract(r, getMathContext());
- return narrowBigDecimal(left, right, result);
+ return l.subtract(r, getMathContext());
}
- // if either are floating point (double or float) use double
+ // if either are floating point (double or float), use double
if (isFloatingPointNumber(left) || isFloatingPointNumber(right)) {
final double l = toDouble(strictCast, left);
final double r = toDouble(strictCast, right);
return l - r;
}
- // otherwise treat as integers
+ // otherwise treat as BigInteger
final BigInteger l = toBigInteger(strictCast, left);
final BigInteger r = toBigInteger(strictCast, right);
final BigInteger result = l.subtract(r);
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
index a2cc3618..51f0f69a 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
@@ -917,6 +917,17 @@ public class ArithmeticTest extends JexlTestCase {
Assert.assertEquals(BigDecimal.valueOf(10), ja.toBigDecimal("10"));
Assert.assertEquals(BigDecimal.valueOf(1.), ja.toBigDecimal(true));
Assert.assertEquals(BigDecimal.valueOf(0.), ja.toBigDecimal(false));
+ // BigDecimal precision is kept when used as argument
+ BigDecimal a42 = BigDecimal.valueOf(42);
+ BigDecimal a49 = BigDecimal.valueOf(49);
+ JexlScript bde = JEXL.createScript("a * 6 / 7", "a");
+ Assert.assertEquals(a42, bde.execute(null, a49));
+ bde = JEXL.createScript("(a - 12) / 12", "a");
+ MathContext mc = ja.getMathContext();
+ BigDecimal b56 = BigDecimal.valueOf(56);
+ BigDecimal b12 = BigDecimal.valueOf(12);
+ BigDecimal b3dot666 = b56.subtract(b12, mc).divide(b12, mc);
+ Assert.assertEquals(b3dot666, bde.execute(null, b56));
}
@Test
@@ -1564,26 +1575,38 @@ public class ArithmeticTest extends JexlTestCase {
}
@Test
- public void testNarrowBig() throws Exception {
+ public void testNarrowBigInteger() throws Exception {
final List<String> ls = Arrays.asList("zero", "one", "two");
asserter.setVariable("list",ls);
- asserter.setVariable("aBigDecimal", new BigDecimal("1"));
- asserter.setVariable("aBigInteger", new BigDecimal("1"));
- asserter.assertExpression("list.get(aBigDecimal)", "one");
- asserter.assertExpression("list.get(aBigInteger)", "one");
+ asserter.assertExpression("a -> list.get(a)", "zero", BigInteger.ZERO);
+ asserter.assertExpression("a -> list.get(a)", "one", BigInteger.ONE);
+ asserter.assertExpression("a -> list.get(2H)", "two");
+ BigInteger b42 = BigInteger.valueOf(42);
+ asserter.setVariable("bi10", BigInteger.valueOf(10));
+ asserter.setVariable("bi420", BigInteger.valueOf(420));
+ asserter.assertExpression("420 / bi10", b42);
+ asserter.assertExpression("420l / bi10", b42);
+ asserter.assertExpression("bi420 / 420", BigInteger.ONE);
+ asserter.assertExpression("bi420 / 420l", BigInteger.ONE);
+ asserter.assertExpression("bi420 / 420H", BigInteger.ONE);
}
@Test
public void testNarrowBigDecimal() throws Exception {
- asserter.setVariable("bi420", BigInteger.valueOf(420));
- asserter.setVariable("bi10", BigInteger.valueOf(10));
- asserter.setVariable("bd420", new BigDecimal("420"));
- asserter.setVariable("bd10", new BigDecimal("10"));
- asserter.assertExpression("420 / bi10", 42);
- asserter.assertExpression("420l / bi10", 42L);
- asserter.assertExpression("bi420 / 420", 1);
- asserter.assertExpression("bi420 / 420l", 1L);
- asserter.assertExpression("bd420 / 10", new BigDecimal("42"));
+ final List<String> ls = Arrays.asList("zero", "one", "two");
+ asserter.setVariable("list", ls);
+ asserter.assertExpression("a -> list.get(a.intValue())", "zero",
BigDecimal.ZERO);
+ asserter.assertExpression("a -> list.get(a.intValue())", "one",
BigDecimal.ONE);
+ asserter.assertExpression("a -> list.get(2B.intValue())", "two");
+ BigDecimal bd42 = BigDecimal.valueOf(42);
+ asserter.setVariable("bd10", BigDecimal.valueOf(10d));
+ asserter.setVariable("bd420",BigDecimal.valueOf(420d));
+ asserter.assertExpression("420 / bd10", bd42);
+ asserter.assertExpression("420l / bd10", bd42);
+ asserter.assertExpression("420H / bd10", bd42);
+ asserter.assertExpression("bd420 / 10", bd42);
+ asserter.assertExpression("bd420 / 10H", bd42);
+ asserter.assertExpression("bd420 / 10B", bd42);
}
@Test
