This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 10f442145 Fix spurious overflow in Fraction.multiplyBy for unreduced
operands (#1769)
10f442145 is described below
commit 10f4421456b891d5a426348511383f387c99f3e1
Author: alhuda <[email protected]>
AuthorDate: Mon Aug 10 16:47:30 2026 +0530
Fix spurious overflow in Fraction.multiplyBy for unreduced operands (#1769)
* Fix spurious overflow in Fraction.multiplyBy for unreduced operands
The Knuth 4.5.1 cross-gcd cancels only the cross terms and assumes both
operands are reduced, so an unreduced operand can overflow the intermediate int
product even when the reduced result fits. Reduce both operands before the
multiply; divideBy and pow route through multiplyBy.
* Reduce operands into locals in multiplyBy to avoid extra allocations
---
src/main/java/org/apache/commons/lang3/math/Fraction.java | 15 ++++++++++++---
.../java/org/apache/commons/lang3/math/FractionTest.java | 14 ++++++++++++++
2 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/src/main/java/org/apache/commons/lang3/math/Fraction.java
b/src/main/java/org/apache/commons/lang3/math/Fraction.java
index ac1973450..5fc936a28 100644
--- a/src/main/java/org/apache/commons/lang3/math/Fraction.java
+++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java
@@ -766,9 +766,18 @@ public Fraction multiplyBy(final Fraction fraction) {
}
// knuth 4.5.1
// make sure we don't overflow unless the result *must* overflow.
- final int d1 = greatestCommonDivisor(numerator, fraction.denominator);
- final int d2 = greatestCommonDivisor(fraction.numerator, denominator);
- return getReducedFraction(mulAndCheck(numerator / d1,
fraction.numerator / d2), mulPosAndCheck(denominator / d2, fraction.denominator
/ d1));
+ // Reduce both operands first: the cross-gcd below cancels the cross
terms only, so a
+ // factor shared inside an unreduced operand survives into the product
and can overflow
+ // an int even when the reduced result fits.
+ final int thisGcd = greatestCommonDivisor(numerator, denominator);
+ final int thatGcd = greatestCommonDivisor(fraction.numerator,
fraction.denominator);
+ final int thisNumerator = numerator / thisGcd;
+ final int thisDenominator = denominator / thisGcd;
+ final int thatNumerator = fraction.numerator / thatGcd;
+ final int thatDenominator = fraction.denominator / thatGcd;
+ final int d1 = greatestCommonDivisor(thisNumerator, thatDenominator);
+ final int d2 = greatestCommonDivisor(thatNumerator, thisDenominator);
+ return getReducedFraction(mulAndCheck(thisNumerator / d1,
thatNumerator / d2), mulPosAndCheck(thisDenominator / d2, thatDenominator /
d1));
}
/**
diff --git a/src/test/java/org/apache/commons/lang3/math/FractionTest.java
b/src/test/java/org/apache/commons/lang3/math/FractionTest.java
index f068402c7..86bb7b120 100644
--- a/src/test/java/org/apache/commons/lang3/math/FractionTest.java
+++ b/src/test/java/org/apache/commons/lang3/math/FractionTest.java
@@ -303,6 +303,11 @@ void testDivide() {
final Fraction negative = Fraction.getFraction(1, -Integer.MAX_VALUE);
assertThrows(ArithmeticException.class, () ->
negative.divideBy(negative.invert())); // Should overflow
+
+ // An unreduced divisor must not trigger a spurious overflow when the
reduced quotient fits an int.
+ f = Fraction.getFraction(-1,
46341).divideBy(Fraction.getFraction(1000000, 100));
+ assertEquals(-1, f.getNumerator());
+ assertEquals(463410000, f.getDenominator());
}
@Test
@@ -747,6 +752,15 @@ void testMultiply() {
final Fraction fr2 = Fraction.getFraction(1, -Integer.MAX_VALUE);
assertThrows(ArithmeticException.class, () -> fr2.multiplyBy(fr2));
+
+ // An unreduced operand must not trigger a spurious overflow when the
reduced product fits an int.
+ f = Fraction.getFraction(-1,
46341).multiplyBy(Fraction.getFraction(100, 1000000));
+ assertEquals(-1, f.getNumerator());
+ assertEquals(463410000, f.getDenominator());
+
+ f = Fraction.getFraction(1,
10000).multiplyBy(Fraction.getFraction(100, 1000000));
+ assertEquals(1, f.getNumerator());
+ assertEquals(100000000, f.getDenominator());
}
@Test
