You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by fa...@apache.org on 2026/08/26 23:08:59 UTC
(poi-xmlbeans) branch trunk updated: Fix MathUtil conversion regressions on non-validation paths (#97)
This is an automated email from the ASF dual-hosted git repository.
pjfanning pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/poi-xmlbeans.git
The following commit(s) were added to refs/heads/trunk by this push:
new 1ba55136 Fix MathUtil conversion regressions on non-validation paths (#97)
1ba55136 is described below
commit 1ba55136b24a4e7a93f344e91b943a739bb8a2ea
Author: PJ Fanning <pj...@users.noreply.github.com>
AuthorDate: Thu Aug 27 00:08:53 2026 +0100
Fix MathUtil conversion regressions on non-validation paths (#97)
* Fix MathUtil conversion regressions on non-validation paths
5.4.0 replaced several silently-truncating BigDecimal conversions with
MathUtil equivalents that throw. On the parse and validation paths that is
the intent, but four of the call sites are not validation paths:
- JavaDecimalHolder.value_hash_code() applied the max-number-chars limit,
so hashCode() threw IllegalArgumentException for any decimal over the
limit, poisoning every HashMap or HashSet holding the object. The limit
bounds parse-time work and cannot be applied here anyway: the equal
xsd:integer hashes without a limit, and the hashes have to agree. Guard
the one case that is genuinely expensive instead - expanding a large
negative scale, eg 1E+2000000000 - and hash such values from their
canonical form. The branch is taken on integer-digit count, which is the
same for every representation of a value, so equal values still agree.
- XmlObjectBase.getBigIntegerValue() and set_BigDecimal() in the int, long
and integer holders let MathUtil's plain IllegalArgumentException escape,
where the lexical path for the same value reports it as
XmlValueOutOfRangeException. Translate it in one place so both paths and
both magnitude regimes agree.
- GDurationBuilder.normalize() converted the whole-second carry with
toInt() while GDateBuilder does the same computation with toLong() into
the same long variable, so a fraction between int and long range threw
in one and not the other.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
* Bound the integral setters by their own range, not the parse limit
set_BigDecimal in the int and long holders applied
maxNumberOfCharsForNumbers, which bounds numbers read out of a document
and has no business gating a value handed to a setter directly: under
maxChars(8), setBigDecimalValue(123456789012) was rejected while
setBigIntegerValue and setLongValue accepted the same value.
The real constraint is the type's own range, which set_BigInteger already
enforces, so pass the width of that range - 19 digits for long, 10 for
int, at either end, as precision() ignores the sign. MathUtil still does
the conversion, so both expansion traps stay guarded: a large negative
scale is rejected before it is expanded, and a value below 1 truncates to
zero without computing the divisor. xs:integer is unbounded and keeps the
configured limit, which is its only bound.
Also drop stripTrailingZeros() from the oversized-value branch of
value_hash_code(): it is quadratic in the number of trailing zeros, so
hashing a 200000-digit value took 39 seconds. The digit count and sign
are just as representation-invariant and are cheap to read.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../java/org/apache/xmlbeans/GDurationBuilder.java | 2 +-
.../xmlbeans/impl/values/JavaDecimalHolder.java | 24 +++-
.../apache/xmlbeans/impl/values/JavaIntHolder.java | 6 +-
.../xmlbeans/impl/values/JavaIntegerHolder.java | 2 +-
.../xmlbeans/impl/values/JavaLongHolder.java | 6 +-
.../apache/xmlbeans/impl/values/XmlObjectBase.java | 28 ++++-
.../java/misc/checkin/MaxNumberOfCharsTest.java | 121 +++++++++++++++++++++
.../xmlobject/schematypes/checkin/GDateTests.java | 15 +++
8 files changed, 196 insertions(+), 8 deletions(-)
diff --git a/src/main/java/org/apache/xmlbeans/GDurationBuilder.java b/src/main/java/org/apache/xmlbeans/GDurationBuilder.java
index 6bf32d47..fcc78e93 100644
--- a/src/main/java/org/apache/xmlbeans/GDurationBuilder.java
+++ b/src/main/java/org/apache/xmlbeans/GDurationBuilder.java
@@ -350,7 +350,7 @@ public class GDurationBuilder implements GDurationSpecification, java.io.Seriali
if (_fs != null && (_fs.signum() < 0 || _fs.compareTo(GDate._one) >= 0)) {
BigDecimal bdcarry = _fs.setScale(0, RoundingMode.FLOOR);
_fs = _fs.subtract(bdcarry);
- carry = MathUtil.toInt(bdcarry);
+ carry = MathUtil.toLong(bdcarry);
}
if (carry != 0 || _s < 0 || _s > 59 || _m < 0 || _m > 50 || _h < 0 || _h > 23) {
diff --git a/src/main/java/org/apache/xmlbeans/impl/values/JavaDecimalHolder.java b/src/main/java/org/apache/xmlbeans/impl/values/JavaDecimalHolder.java
index c2bdfbc2..fad36d97 100644
--- a/src/main/java/org/apache/xmlbeans/impl/values/JavaDecimalHolder.java
+++ b/src/main/java/org/apache/xmlbeans/impl/values/JavaDecimalHolder.java
@@ -152,6 +152,15 @@ public class JavaDecimalHolder extends XmlObjectBase {
private static final BigInteger _maxlong = BigInteger.valueOf(Long.MAX_VALUE);
private static final BigInteger _minlong = BigInteger.valueOf(Long.MIN_VALUE);
+ /**
+ * The number of integer digits we are prepared to materialise when hashing.
+ * A value with a large negative scale (eg 1E+2000000000) expands to billions of
+ * digits, so it is hashed from its canonical form instead. hashCode() must not
+ * throw, so this is a fallback rather than the max-number-chars limit applied
+ * elsewhere.
+ */
+ private static final long MAX_HASH_INTEGER_DIGITS = 100000;
+
/**
* Note, this is carefully aligned with hash codes for all xsd:decimal
* primitives.
@@ -163,7 +172,20 @@ public class JavaDecimalHolder extends XmlObjectBase {
}
}
- BigInteger intval = MathUtil.toBigInteger(_value, get_max_number_chars());
+ // precision() - scale() is the number of integer digits, and is the same for
+ // every representation of a given value, so this branches consistently for
+ // values that compare equal
+ long integerDigits = (long) _value.precision() - _value.scale();
+ if (integerDigits > MAX_HASH_INTEGER_DIGITS) {
+ // hash on the digit count and sign: both are cheap and, like the branch
+ // above, the same for every representation of the value. Values this wide
+ // collide with each other, which is allowed - expanding them is not.
+ return (int) integerDigits * 31 + _value.signum();
+ }
+
+ // deliberately BigDecimal.toBigInteger() and not MathUtil.toBigInteger():
+ // hashCode() must not throw, and the expansion is bounded by the check above
+ BigInteger intval = _value.toBigInteger();
if (intval.compareTo(_maxlong) > 0 ||
intval.compareTo(_minlong) < 0) {
diff --git a/src/main/java/org/apache/xmlbeans/impl/values/JavaIntHolder.java b/src/main/java/org/apache/xmlbeans/impl/values/JavaIntHolder.java
index 72d765ef..53fb40c5 100644
--- a/src/main/java/org/apache/xmlbeans/impl/values/JavaIntHolder.java
+++ b/src/main/java/org/apache/xmlbeans/impl/values/JavaIntHolder.java
@@ -20,7 +20,6 @@ import org.apache.xmlbeans.SimpleValue;
import org.apache.xmlbeans.XmlErrorCodes;
import org.apache.xmlbeans.XmlObject;
import org.apache.xmlbeans.impl.schema.BuiltinSchemaTypeSystem;
-import org.apache.xmlbeans.impl.util.MathUtil;
import org.apache.xmlbeans.impl.util.XsTypeConverter;
import java.math.BigDecimal;
@@ -76,9 +75,12 @@ public abstract class JavaIntHolder extends XmlObjectBase {
static final BigInteger _max = BigInteger.valueOf(Integer.MAX_VALUE);
static final BigInteger _min = BigInteger.valueOf(Integer.MIN_VALUE);
+ /** both ends of the int range are 10 digits, so anything wider is out of range */
+ private static final int MAX_INT_DIGITS = 10;
+
// setters
protected void set_BigDecimal(BigDecimal v) {
- set_BigInteger(MathUtil.toBigInteger(v, get_max_number_chars()));
+ set_BigInteger(to_BigInteger(v, MAX_INT_DIGITS));
}
protected void set_BigInteger(BigInteger v) {
diff --git a/src/main/java/org/apache/xmlbeans/impl/values/JavaIntegerHolder.java b/src/main/java/org/apache/xmlbeans/impl/values/JavaIntegerHolder.java
index 3c4c8f8c..9e6143e5 100644
--- a/src/main/java/org/apache/xmlbeans/impl/values/JavaIntegerHolder.java
+++ b/src/main/java/org/apache/xmlbeans/impl/values/JavaIntegerHolder.java
@@ -79,7 +79,7 @@ public abstract class JavaIntegerHolder extends XmlObjectBase {
// setters
protected void set_BigDecimal(BigDecimal v) {
- _value = MathUtil.toBigInteger(v, get_max_number_chars());
+ _value = to_BigInteger(v);
}
protected void set_BigInteger(BigInteger v) {
diff --git a/src/main/java/org/apache/xmlbeans/impl/values/JavaLongHolder.java b/src/main/java/org/apache/xmlbeans/impl/values/JavaLongHolder.java
index bc21efe9..83783a27 100644
--- a/src/main/java/org/apache/xmlbeans/impl/values/JavaLongHolder.java
+++ b/src/main/java/org/apache/xmlbeans/impl/values/JavaLongHolder.java
@@ -20,7 +20,6 @@ import org.apache.xmlbeans.SimpleValue;
import org.apache.xmlbeans.XmlErrorCodes;
import org.apache.xmlbeans.XmlObject;
import org.apache.xmlbeans.impl.schema.BuiltinSchemaTypeSystem;
-import org.apache.xmlbeans.impl.util.MathUtil;
import org.apache.xmlbeans.impl.util.XsTypeConverter;
import java.math.BigDecimal;
@@ -71,9 +70,12 @@ public abstract class JavaLongHolder extends XmlObjectBase {
private static final BigInteger _max = BigInteger.valueOf(Long.MAX_VALUE);
private static final BigInteger _min = BigInteger.valueOf(Long.MIN_VALUE);
+ /** both ends of the long range are 19 digits, so anything wider is out of range */
+ private static final int MAX_LONG_DIGITS = 19;
+
// setters
protected void set_BigDecimal(BigDecimal v) {
- set_BigInteger(MathUtil.toBigInteger(v, get_max_number_chars()));
+ set_BigInteger(to_BigInteger(v, MAX_LONG_DIGITS));
}
protected void set_BigInteger(BigInteger v) {
diff --git a/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java b/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java
index daf812a4..27bb6509 100644
--- a/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java
+++ b/src/main/java/org/apache/xmlbeans/impl/values/XmlObjectBase.java
@@ -1357,7 +1357,33 @@ public abstract class XmlObjectBase implements TypeStoreUser, Serializable, XmlO
// numerics: integral
public BigInteger getBigIntegerValue() {
BigDecimal bd = getBigDecimalValue();
- return bd == null ? null : MathUtil.toBigInteger(bd, get_max_number_chars());
+ return bd == null ? null : to_BigInteger(bd);
+ }
+
+ /**
+ * Converts to BigInteger, applying the maximum number of characters configured by
+ * XmlOptions.setMaxNumberOfCharsForNumbers.
+ */
+ protected final BigInteger to_BigInteger(BigDecimal v) {
+ return to_BigInteger(v, get_max_number_chars());
+ }
+
+ /**
+ * Converts to BigInteger, rejecting anything wider than maxIntegerDigits. Types with
+ * a bound of their own pass that bound rather than the configured maximum number of
+ * characters, which limits the size of numbers read out of a document and so has no
+ * business rejecting a value handed to a setter directly.
+ * <p>
+ * MathUtil reports an over-large magnitude as a plain IllegalArgumentException, but
+ * the XmlObject API reports out-of-range values as XmlValueOutOfRangeException, so
+ * translate it here rather than let it escape to callers.
+ */
+ protected final BigInteger to_BigInteger(BigDecimal v, int maxIntegerDigits) {
+ try {
+ return MathUtil.toBigInteger(v, maxIntegerDigits);
+ } catch (IllegalArgumentException e) {
+ throw new XmlValueOutOfRangeException(e.getMessage());
+ }
}
public byte getByteValue() {
diff --git a/src/test/java/misc/checkin/MaxNumberOfCharsTest.java b/src/test/java/misc/checkin/MaxNumberOfCharsTest.java
index ad7f3231..6e771436 100644
--- a/src/test/java/misc/checkin/MaxNumberOfCharsTest.java
+++ b/src/test/java/misc/checkin/MaxNumberOfCharsTest.java
@@ -17,13 +17,19 @@ package misc.checkin;
import org.apache.xmlbeans.SimpleValue;
import org.apache.xmlbeans.XmlDecimal;
import org.apache.xmlbeans.XmlException;
+import org.apache.xmlbeans.XmlInt;
import org.apache.xmlbeans.XmlInteger;
+import org.apache.xmlbeans.XmlLong;
import org.apache.xmlbeans.XmlOptions;
import org.apache.xmlbeans.impl.values.XmlValueOutOfRangeException;
import org.junit.jupiter.api.Test;
+import java.math.BigDecimal;
+import java.math.BigInteger;
+
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
/**
* XmlOptions.setMaxNumberOfCharsForNumbers has to apply to values materialised from the
@@ -95,4 +101,119 @@ public class MaxNumberOfCharsTest {
SimpleValue value = (SimpleValue) XmlDecimal.Factory.parse(frag(digits(2000)));
assertThrows(XmlValueOutOfRangeException.class, value::getBigIntegerValue);
}
+
+ @Test
+ public void testDecimalToBigIntegerReportsOutOfRangeWhenSetProgrammatically() {
+ // setBigDecimalValue() bypasses the lexical path, so the limit is only applied
+ // on the way out - and has to be reported the same way it is on the way in
+ XmlDecimal value = XmlDecimal.Factory.newInstance();
+ value.setBigDecimalValue(new BigDecimal(digits(2000)));
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> ((SimpleValue) value).getBigIntegerValue());
+ }
+
+ @Test
+ public void testIntegralSettersReportOutOfRange() {
+ BigDecimal oversized = new BigDecimal(digits(2000));
+
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlInt.Factory.newInstance().setBigDecimalValue(oversized));
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlLong.Factory.newInstance().setBigDecimalValue(oversized));
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlInteger.Factory.newInstance().setBigDecimalValue(oversized));
+
+ // a value that merely overflows the java type is unaffected
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlInt.Factory.newInstance().setBigDecimalValue(new BigDecimal("1E+20")));
+ }
+
+ @Test
+ public void testIntegralSettersUseTheirOwnBoundNotTheCharLimit() throws XmlException {
+ // maxNumberOfCharsForNumbers bounds numbers read out of a document; it must not
+ // reject a valid value handed to a setter directly, or setBigDecimalValue would
+ // disagree with the other setters for the same value
+ XmlLong value = (XmlLong) XmlLong.Factory.parse(frag("1"), maxChars(8));
+
+ value.setBigDecimalValue(new BigDecimal("123456789012"));
+ assertEquals(123456789012L, value.getLongValue());
+ value.setBigIntegerValue(new BigInteger("123456789012"));
+ assertEquals(123456789012L, value.getLongValue());
+ value.setLongValue(123456789012L);
+ assertEquals(123456789012L, value.getLongValue());
+
+ XmlInt intValue = (XmlInt) XmlInt.Factory.parse(frag("1"), maxChars(4));
+ intValue.setBigDecimalValue(new BigDecimal("123456789"));
+ assertEquals(123456789, intValue.getIntValue());
+ }
+
+ @Test
+ public void testIntegralSettersStillRejectOutOfRange() {
+ // the type's own bound still applies, and reaching it does not need the value
+ // to be expanded first
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlLong.Factory.newInstance().setBigDecimalValue(new BigDecimal("1E+2000000000")));
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlInt.Factory.newInstance().setBigDecimalValue(new BigDecimal("1E+2000000000")));
+
+ // 19 digits is the widest a long can be at either end - precision() counts the
+ // digits of the unscaled value, so the sign does not consume one - and the bound
+ // is on width, so a 19-digit value out of range is still caught by set_BigInteger
+ XmlLong value = XmlLong.Factory.newInstance();
+ value.setBigDecimalValue(new BigDecimal("9223372036854775807"));
+ assertEquals(Long.MAX_VALUE, value.getLongValue());
+ value.setBigDecimalValue(new BigDecimal("-9223372036854775808"));
+ assertEquals(Long.MIN_VALUE, value.getLongValue());
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlLong.Factory.newInstance().setBigDecimalValue(new BigDecimal("9999999999999999999")));
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlLong.Factory.newInstance().setBigDecimalValue(new BigDecimal("-9999999999999999999")));
+
+ XmlInt intValue = XmlInt.Factory.newInstance();
+ intValue.setBigDecimalValue(new BigDecimal("2147483647"));
+ assertEquals(Integer.MAX_VALUE, intValue.getIntValue());
+ intValue.setBigDecimalValue(new BigDecimal("-2147483648"));
+ assertEquals(Integer.MIN_VALUE, intValue.getIntValue());
+ assertThrows(XmlValueOutOfRangeException.class,
+ () -> XmlInt.Factory.newInstance().setBigDecimalValue(new BigDecimal("-9999999999")));
+
+ // a value below 1 truncates to zero, as it always has
+ XmlLong truncated = XmlLong.Factory.newInstance();
+ truncated.setBigDecimalValue(new BigDecimal("1E-10000000"));
+ assertEquals(0L, truncated.getLongValue());
+ }
+
+ @Test
+ public void testDecimalHashCodeIgnoresLimit() {
+ // hashCode() must not throw, whatever the limit is, and must stay aligned with
+ // the hash of the same value held as an xsd:integer
+ XmlDecimal decimal = XmlDecimal.Factory.newInstance();
+ decimal.setBigDecimalValue(new BigDecimal(digits(2000)));
+ XmlInteger integer = XmlInteger.Factory.newInstance();
+ integer.setBigIntegerValue(new BigInteger(digits(2000)));
+
+ assertEquals(integer.valueHashCode(), decimal.valueHashCode());
+ }
+
+ @Test
+ public void testDecimalHashCodeIsIndependentOfScale() {
+ // 1E+200000 and the same value written out in full are equal, and are both past
+ // the threshold at which hashing stops expanding the value - so the two have to
+ // reach the same hash without either being expanded
+ XmlDecimal exponent = XmlDecimal.Factory.newInstance();
+ exponent.setBigDecimalValue(new BigDecimal("1E+200000"));
+ XmlDecimal expanded = XmlDecimal.Factory.newInstance();
+ expanded.setBigDecimalValue(new BigDecimal(digits(200001)));
+
+ assertEquals(true, exponent.valueEquals(expanded));
+ assertEquals(expanded.valueHashCode(), exponent.valueHashCode());
+ }
+
+ @Test
+ public void testDecimalHashCodeDoesNotExpandHugeExponent() {
+ // expanding 1E+2000000000 would need gigabytes; hashing must not attempt it
+ XmlDecimal value = XmlDecimal.Factory.newInstance();
+ value.setBigDecimalValue(new BigDecimal("1E+2000000000"));
+ assertTimeoutPreemptively(java.time.Duration.ofSeconds(10), value::valueHashCode);
+ }
}
diff --git a/src/test/java/xmlobject/schematypes/checkin/GDateTests.java b/src/test/java/xmlobject/schematypes/checkin/GDateTests.java
index 06409bc5..5e5fdbdb 100755
--- a/src/test/java/xmlobject/schematypes/checkin/GDateTests.java
+++ b/src/test/java/xmlobject/schematypes/checkin/GDateTests.java
@@ -690,6 +690,21 @@ public class GDateTests {
assertThrows(IllegalArgumentException.class, () -> new GDuration("PT3000000000S"));
}
+ @Test
+ void testDurationFractionCarryBeyondIntRange() {
+ // setFraction() is unvalidated, so the whole-second carry can exceed int range.
+ // GDateBuilder handles the same quantity as a long, so GDurationBuilder must too.
+ GDurationBuilder gdb = new GDurationBuilder();
+ gdb.setFraction(new BigDecimal("1E+10"));
+ gdb.normalize();
+ assertEquals("P115740DT17H46M40S", gdb.toString());
+
+ GDurationBuilder small = new GDurationBuilder();
+ small.setFraction(new BigDecimal("1.5"));
+ small.normalize();
+ assertEquals("PT1.5S", small.toString());
+ }
+
// Assert-style check that prints PASS/FAIL against the expected validity.
static void check(SchemaTypeLoader loader, String durationLiteral,
boolean expectedValid, String note) throws Exception {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org