[PR] fix boundary and performance issues in MathUtil conversio ns [poi-xmlbeans]
pjfanning (via GitHub) <[email protected]>
| Newsgroups | gmane.comp.jakarta.poi.devel |
|---|---|
| Message-ID | <[email protected]> |
pjfanning opened a new pull request, #94: URL: https://github.com/apache/poi-xmlbeans/pull/94 Three issues in `MathUtil`, each covered by a new test. **`safeFloatToInt` accepted `2147483648f`.** Widening `Integer.MAX_VALUE` to `float` rounds it up to `2^31`, so `f > Integer.MAX_VALUE` was false for exactly `2^31` and the `(int)` cast silently saturated to `2147483647` — the wrong answer from a method whose job is to reject out-of-range input. Comparing as `double` fixes it. The lower bound was already correct, since `-2^31` is exactly representable, and `safeDoubleToInt` was never affected. **`toBigInteger` guarded a large negative scale but not a large positive one.** A value like `1E-10000000` has `integerDigits == -9999999` and `scale == +10000000`, so neither existing condition tripped and `BigDecimal.toBigInteger()` was reached. That calls `setScale(0)`, which computes `10^scale`: measured at just over 4 seconds to return `0` from a 12-character input, and at larger exponents it throws `ArithmeticException: BigInteger would overflow supported range` instead of the documented `IllegalArgumentException`. The string passes `parseAsBigDecimal`'s length limit easily, and the path is reachable from parsed content via `JavaDecimalHolder` / `XmlObjectBase.getBigIntegerValue()`. Any such value truncates to zero, so return `BigInteger.ZERO` directly — which also makes every s ub-1 conversion cheaper. **`toBigInteger` had no explicit null check**, unlike every other method here. It already threw `NullPointerException` from `stripTrailingZeros()`, so this is consistency rather than a behaviour change; `toLong` and `toInt` inherit it. Also added a comment on the existing `scale() < -DEFAULT_MAX_NUMBER_CHARS` condition, which reads as redundant but is not: for `1E+2147483647` the `precision() - scale()` subtraction overflows to `Integer.MIN_VALUE` and only the scale check catches it. And corrected a few copy-pasted javadoc `@return` types (`parseAsDouble` documented `float`). Ran the `impl.util` and `impl.values` tests locally — all pass. Leaving the full suite to CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]