From bbf22d3c92ed5fe83349241cffc07bc15d301667 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Tue, 25 Aug 2026 13:47:08 +0100 Subject: [PATCH] fix boundary and performance issues in MathUtil conversions safeFloatToInt accepted 2147483648f: widening Integer.MAX_VALUE to float rounds it up to 2^31, so the upper-bound check never tripped and the cast silently saturated to 2147483647. Compare as double instead. toBigInteger guarded a large negative scale but not a large positive one, so a tiny value such as 1E-10000000 reached BigDecimal.toBigInteger(), which computes 10^scale - several seconds of CPU to produce zero, and an undocumented ArithmeticException at larger exponents. Such values truncate to zero, so return that directly. Also add an explicit null check to toBigInteger, matching the other methods, and correct a few copy-pasted javadoc return types. Co-Authored-By: Claude Opus 5 (1M context) --- .../apache/xmlbeans/impl/util/MathUtil.java | 19 ++++-- .../xmlbeans/impl/util/TestMathUtil.java | 58 +++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java b/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java index f232df73e..feb50d355 100644 --- a/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java +++ b/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java @@ -34,7 +34,8 @@ public static int safeFloatToInt(float f) { if (Float.isInfinite(f)) { throw new IllegalArgumentException("Cannot convert infinity to int"); } - if (f > Integer.MAX_VALUE || f < Integer.MIN_VALUE) { + // compare as double: widening Integer.MAX_VALUE to float rounds it up to 2^31 + if ((double) f > Integer.MAX_VALUE || (double) f < Integer.MIN_VALUE) { throw new IllegalArgumentException("Value out of range: " + f); } return (int) f; @@ -101,7 +102,7 @@ public static BigInteger parseAsBigInteger(String s) { /** * @param s string to parse - * @return valid Float + * @return valid float * @throws NumberFormatException if parse fails * @throws IllegalArgumentException if string is too long * @throws NullPointerException if string is null @@ -130,7 +131,7 @@ public static float parseAsFloat(String s, int maxNumberOfChars) { /** * @param s string to parse - * @return valid float + * @return valid double * @throws NumberFormatException if parse fails * @throws IllegalArgumentException if string is too long * @throws NullPointerException if string is null @@ -142,7 +143,7 @@ public static double parseAsDouble(String s) { /** * @param s string to parse * @param maxNumberOfChars maximum number of characters allowed in the string - * @return valid float + * @return valid double * @throws NumberFormatException if parse fails * @throws IllegalArgumentException if string is too long * @throws NullPointerException if string is null @@ -198,13 +199,23 @@ public static int parseAsInt(String s) { * @throws NullPointerException if value is null */ public static BigInteger toBigInteger(BigDecimal value) { + if (value == null) { + throw new NullPointerException("Cannot convert null to BigInteger"); + } BigDecimal normalized = value.stripTrailingZeros(); int integerDigits = normalized.precision() - normalized.scale(); + // the scale check is not redundant: for a very negative scale (eg 1E+2147483647) the + // subtraction above overflows and integerDigits comes out negative if (integerDigits > DEFAULT_MAX_NUMBER_CHARS || normalized.scale() < -DEFAULT_MAX_NUMBER_CHARS) { throw new IllegalArgumentException( "BigDecimal magnitude too large to convert safely: approx " + integerDigits + " integer digits (limit " + DEFAULT_MAX_NUMBER_CHARS + ")"); } + if (integerDigits <= 0) { + // abs(value) is less than 1, so it truncates to zero - avoid BigDecimal.toBigInteger() + // computing 10^scale, which is very expensive for a large scale (eg 1E-10000000) + return BigInteger.ZERO; + } return normalized.toBigInteger(); } diff --git a/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java b/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java index b27fb3bf9..5e304b115 100644 --- a/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java +++ b/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java @@ -20,9 +20,11 @@ Licensed to the Apache Software Foundation (ASF) under one or more import java.math.BigDecimal; import java.math.BigInteger; +import java.time.Duration; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; public class TestMathUtil { @Test @@ -52,4 +54,60 @@ public void testToLongWithValueOutOfRange() { BigDecimal expected2 = BigDecimal.valueOf(Long.MIN_VALUE).subtract(BigDecimal.ONE); assertThrows(IllegalArgumentException.class, () -> MathUtil.toLong(expected2)); } + + @Test + public void testToBigIntegerNull() { + assertThrows(NullPointerException.class, () -> MathUtil.toBigInteger(null)); + assertThrows(NullPointerException.class, () -> MathUtil.toLong(null)); + assertThrows(NullPointerException.class, () -> MathUtil.toInt(null)); + } + + @Test + public void testToBigIntegerSmallValuesTruncateToZero() { + assertEquals(BigInteger.ZERO, MathUtil.toBigInteger(new BigDecimal("0.5"))); + assertEquals(BigInteger.ZERO, MathUtil.toBigInteger(new BigDecimal("-0.999"))); + assertEquals(BigInteger.ZERO, MathUtil.toBigInteger(BigDecimal.ZERO)); + assertEquals(BigInteger.ONE, MathUtil.toBigInteger(new BigDecimal("1.5"))); + } + + @Test + public void testToBigIntegerSmallExponent() { + // BigDecimal.toBigInteger() would compute 10^10000000 here, taking seconds + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> + assertEquals(BigInteger.ZERO, MathUtil.toBigInteger(new BigDecimal("1E-10000000")))); + } + + @Test + public void testToBigIntegerMaxNegativeScale() { + BigDecimal value = new BigDecimal("1E+2147483647"); + assertThrows(IllegalArgumentException.class, () -> MathUtil.toBigInteger(value)); + } + + @Test + public void testSafeFloatToInt() { + assertEquals(1, MathUtil.safeFloatToInt(1.75f)); + assertEquals(-1, MathUtil.safeFloatToInt(-1.75f)); + assertEquals(Integer.MIN_VALUE, MathUtil.safeFloatToInt(Integer.MIN_VALUE)); + } + + @Test + public void testSafeFloatToIntWithValueOutOfRange() { + // 2^31 is the nearest float above Integer.MAX_VALUE and must not be accepted + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(2147483648f)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(-2147483904f)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(Float.MAX_VALUE)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(Float.NaN)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(Float.POSITIVE_INFINITY)); + } + + @Test + public void testSafeDoubleToInt() { + assertEquals(1, MathUtil.safeDoubleToInt(1.75)); + assertEquals(Integer.MAX_VALUE, MathUtil.safeDoubleToInt(Integer.MAX_VALUE)); + assertEquals(Integer.MIN_VALUE, MathUtil.safeDoubleToInt(Integer.MIN_VALUE)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeDoubleToInt(2147483648d)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeDoubleToInt(-2147483649d)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeDoubleToInt(Double.NaN)); + assertThrows(IllegalArgumentException.class, () -> MathUtil.safeDoubleToInt(Double.NEGATIVE_INFINITY)); + } }