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 46d9a80e5 Fix spurious overflow in Fraction.add/subtract for coprime
denominators (#1709)
46d9a80e5 is described below
commit 46d9a80e548f792c5ee1a9e63f82a84256bf18a4
Author: alhuda <[email protected]>
AuthorDate: Thu Jun 18 00:22:58 2026 +0530
Fix spurious overflow in Fraction.add/subtract for coprime denominators
(#1709)
* fix spurious overflow in Fraction.add/subtract for coprime denominators
* Use Math.addExact/subtractExact/toIntExact in Fraction.addSub
---
.../org/apache/commons/lang3/math/Fraction.java | 44 +++++-----------------
.../apache/commons/lang3/math/FractionTest.java | 20 ++++++++++
2 files changed, 29 insertions(+), 35 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 a1dd4a1ae..a2a45ac83 100644
--- a/src/main/java/org/apache/commons/lang3/math/Fraction.java
+++ b/src/main/java/org/apache/commons/lang3/math/Fraction.java
@@ -105,22 +105,11 @@ public final class Fraction extends Number implements
Comparable<Fraction> {
public static final Fraction FOUR_FIFTHS = new Fraction(4, 5);
/**
- * Adds two integers, checking for overflow.
+ * Checks that a denominator is not zero.
*
- * @param x an addend
- * @param y an addend
- * @return the sum {@code x+y}
- * @throws ArithmeticException if the result cannot be represented as
- * an int
+ * @param denominator the denominator to check
+ * @throws ArithmeticException if the denominator is zero
*/
- private static int addAndCheck(final int x, final int y) {
- final long s = (long) x + (long) y;
- if (s < Integer.MIN_VALUE || s > Integer.MAX_VALUE) {
- throw new ArithmeticException("overflow: add");
- }
- return (int) s;
- }
-
private static void checkDenominator(final int denominator) {
if (denominator == 0) {
throw new ArithmeticException("The denominator must not be zero");
@@ -444,23 +433,6 @@ private static int mulPosAndCheck(final int x, final int
y) {
return (int) m;
}
- /**
- * Subtracts two integers, checking for overflow.
- *
- * @param x the minuend
- * @param y the subtrahend
- * @return the difference {@code x-y}
- * @throws ArithmeticException if the result cannot be represented as
- * an int
- */
- private static int subAndCheck(final int x, final int y) {
- final long s = (long) x - (long) y;
- if (s < Integer.MIN_VALUE || s > Integer.MAX_VALUE) {
- throw new ArithmeticException("overflow: add");
- }
- return (int) s;
- }
-
/**
* The numerator number part of the fraction (the three in three sevenths).
*/
@@ -556,10 +528,12 @@ private Fraction addSub(final Fraction fraction, final
boolean isAdd) {
final int d1 = greatestCommonDivisor(denominator,
fraction.denominator);
if (d1 == 1) {
// result is ((u*v' +/- u'v) / u'v')
- final int uvp = mulAndCheck(numerator, fraction.denominator);
- final int upv = mulAndCheck(fraction.numerator, denominator);
- return new Fraction(isAdd ? addAndCheck(uvp, upv) :
subAndCheck(uvp, upv), mulPosAndCheck(denominator,
- fraction.denominator));
+ // the int cross products u*v' and u'*v can overflow even when the
reduced result
+ // fits an int, so widen to long and let Math narrow the final
numerator back.
+ final long uvp = (long) numerator * fraction.denominator;
+ final long upv = (long) fraction.numerator * denominator;
+ final long t = isAdd ? Math.addExact(uvp, upv) :
Math.subtractExact(uvp, upv);
+ return new Fraction(Math.toIntExact(t),
mulPosAndCheck(denominator, fraction.denominator));
}
// the quantity 't' requires 65 bits of precision; see knuth 4.5.1
// exercise 7. we're going to use a BigInteger.
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 aa2985c10..862e383b1 100644
--- a/src/test/java/org/apache/commons/lang3/math/FractionTest.java
+++ b/src/test/java/org/apache/commons/lang3/math/FractionTest.java
@@ -162,6 +162,19 @@ void testAdd() {
final Fraction f3 = Fraction.getFraction(3, 327680);
final Fraction f4 = Fraction.getFraction(2, 59049);
assertThrows(ArithmeticException.class, () -> f3.add(f4)); // should
overflow
+
+ // the cross products u*v' and u'*v overflow an int, but the reduced
result fits.
+ f1 = Fraction.getFraction(Integer.MAX_VALUE, 2);
+ f2 = Fraction.getFraction(-Integer.MAX_VALUE, 1);
+ f = f1.add(f2);
+ assertEquals(-Integer.MAX_VALUE, f.getNumerator());
+ assertEquals(2, f.getDenominator());
+
+ f1 = Fraction.getFraction(2, 1);
+ f2 = Fraction.getFraction(-Integer.MAX_VALUE, 2114962910);
+ f = f1.add(f2);
+ assertEquals(2082442173, f.getNumerator());
+ assertEquals(2114962910, f.getDenominator());
}
@Test
@@ -1065,6 +1078,13 @@ void testSubtract() {
// Should overflow
assertThrows(ArithmeticException.class, () -> Fraction.getFraction(3,
327680).subtract(Fraction.getFraction(2, 59049)));
+
+ // the cross products u*v' and u'*v overflow an int, but the reduced
result fits.
+ f1 = Fraction.getFraction(Integer.MAX_VALUE, 2);
+ f2 = Fraction.getFraction(Integer.MAX_VALUE, 1);
+ f = f1.subtract(f2);
+ assertEquals(-Integer.MAX_VALUE, f.getNumerator());
+ assertEquals(2, f.getDenominator());
}
@Test
