From 9fec47b79bbb40a78eb79e5a66716697c91ae0be Mon Sep 17 00:00:00 2001 From: Luc Maisonobe Date: Fri, 1 Feb 2008 12:02:12 +0000 Subject: [PATCH] added checks for integer overflows during double to Fraction conversion JIRA: MATH-182 git-svn-id: https://svn.apache.org/repos/asf/commons/proper/math/trunk@617482 13f79535-47bb-0310-9956-ffa450edef68 --- .../commons/math/MessagesResources_fr.java | 2 + .../commons/math/fraction/Fraction.java | 37 +++++++++++-------- .../fraction/FractionConversionException.java | 21 ++++++++--- .../commons/math/fraction/FractionTest.java | 16 ++++++++ xdocs/changes.xml | 3 ++ 5 files changed, 59 insertions(+), 20 deletions(-) diff --git a/src/java/org/apache/commons/math/MessagesResources_fr.java b/src/java/org/apache/commons/math/MessagesResources_fr.java index 5fc9700c9..7bff4908b 100644 --- a/src/java/org/apache/commons/math/MessagesResources_fr.java +++ b/src/java/org/apache/commons/math/MessagesResources_fr.java @@ -71,6 +71,8 @@ public class MessagesResources_fr // org.apache.commons.math.fraction.FractionConversionException { "Unable to convert {0} to fraction after {1} iterations", "Impossible de convertir {0} en fraction apr\u00e8s {1} it\u00e9rations" }, + { "Overflow trying to convert {0} to fraction ({1}/{2})", + "D\u00e9passement de capacit\u00e9 lors de la conversion de {0} en fraction ({1}/{2})" }, // org.apache.commons.math.analysis.UnivariateRealSolverUtils { "Number of iterations={0}, maximum iterations={1}, initial={2}, lower bound={3}, upper bound={4}," + diff --git a/src/java/org/apache/commons/math/fraction/Fraction.java b/src/java/org/apache/commons/math/fraction/Fraction.java index dfccced07..9fe075d6b 100644 --- a/src/java/org/apache/commons/math/fraction/Fraction.java +++ b/src/java/org/apache/commons/math/fraction/Fraction.java @@ -34,7 +34,7 @@ public class Fraction extends Number implements Comparable { public static final Fraction ZERO = new Fraction(0, 1); /** Serializable version identifier */ - private static final long serialVersionUID = 5463066929751300926L; + private static final long serialVersionUID = -8958519416450949235L; /** The denominator. */ private int denominator; @@ -128,33 +128,40 @@ public class Fraction extends Number implements Comparable { private Fraction(double value, double epsilon, int maxDenominator, int maxIterations) throws FractionConversionException { + long overflow = Integer.MAX_VALUE; double r0 = value; - int a0 = (int)Math.floor(r0); + long a0 = (long)Math.floor(r0); + if (a0 > overflow) { + throw new FractionConversionException(value, a0, 1l); + } // check for (almost) integer arguments, which should not go // to iterations. if (Math.abs(a0 - value) < epsilon) { - this.numerator = a0; + this.numerator = (int) a0; this.denominator = 1; return; } - - int p0 = 1; - int q0 = 0; - int p1 = a0; - int q1 = 1; - int p2 = 0; - int q2 = 1; + long p0 = 1; + long q0 = 0; + long p1 = a0; + long q1 = 1; + + long p2 = 0; + long q2 = 1; int n = 0; boolean stop = false; do { ++n; double r1 = 1.0 / (r0 - a0); - int a1 = (int)Math.floor(r1); + long a1 = (long)Math.floor(r1); p2 = (a1 * p1) + p0; q2 = (a1 * q1) + q0; + if ((p2 > overflow) || (q2 > overflow)) { + throw new FractionConversionException(value, p2, q2); + } double convergent = (double)p2 / (double)q2; if (n < maxIterations && Math.abs(convergent - value) > epsilon && q2 < maxDenominator) { @@ -174,11 +181,11 @@ public class Fraction extends Number implements Comparable { } if (q2 < maxDenominator) { - this.numerator = p2; - this.denominator = q2; + this.numerator = (int) p2; + this.denominator = (int) q2; } else { - this.numerator = p1; - this.denominator = q1; + this.numerator = (int) p1; + this.denominator = (int) q1; } } diff --git a/src/java/org/apache/commons/math/fraction/FractionConversionException.java b/src/java/org/apache/commons/math/fraction/FractionConversionException.java index 25487def1..b9a34adb0 100644 --- a/src/java/org/apache/commons/math/fraction/FractionConversionException.java +++ b/src/java/org/apache/commons/math/fraction/FractionConversionException.java @@ -17,7 +17,7 @@ package org.apache.commons.math.fraction; -import org.apache.commons.math.MaxIterationsExceededException; +import org.apache.commons.math.ConvergenceException; /** * Error thrown when a double value cannot be converted to a fraction @@ -25,10 +25,10 @@ import org.apache.commons.math.MaxIterationsExceededException; * * @version $Revision$ $Date$ */ -public class FractionConversionException extends MaxIterationsExceededException { +public class FractionConversionException extends ConvergenceException { /** Serializable version identifier. */ - private static final long serialVersionUID = 4588659344016668813L; + private static final long serialVersionUID = -4661812640132576263L; /** * Constructs an exception with specified formatted detail message. @@ -37,9 +37,20 @@ public class FractionConversionException extends MaxIterationsExceededException * @param maxIterations maximal number of iterations allowed */ public FractionConversionException(double value, int maxIterations) { - super(maxIterations, - "Unable to convert {0} to fraction after {1} iterations", + super("Unable to convert {0} to fraction after {1} iterations", new Object[] { new Double(value), new Integer(maxIterations) }); } + /** + * Constructs an exception with specified formatted detail message. + * Message formatting is delegated to {@link java.text.MessageFormat}. + * @param value double value to convert + * @param p current numerator + * @param q current denominator + */ + public FractionConversionException(double value, long p, long q) { + super("Overflow trying to convert {0} to fraction ({1}/{2})", + new Object[] { new Double(value), new Long(p), new Long(q) }); + } + } diff --git a/src/test/org/apache/commons/math/fraction/FractionTest.java b/src/test/org/apache/commons/math/fraction/FractionTest.java index 986ffc235..6844fd9b9 100644 --- a/src/test/org/apache/commons/math/fraction/FractionTest.java +++ b/src/test/org/apache/commons/math/fraction/FractionTest.java @@ -133,6 +133,22 @@ public class FractionTest extends TestCase { assertFraction(769, 1250, new Fraction(0.6152, 9999)); } + public void testIntegerOverflow() { + checkIntegerOverflow(0.75000000001455192); + checkIntegerOverflow(1.0e10); + } + + private void checkIntegerOverflow(double a) { + try { + new Fraction(a, 1.0e-12, 1000); + fail("an exception should have been thrown"); + } catch (ConvergenceException ce) { + // expected behavior + } catch (Exception e) { + fail("wrong exception caught"); + } + } + public void testEpsilonLimitConstructor() throws ConvergenceException { assertFraction(2, 5, new Fraction(0.4, 1.0e-5, 100)); diff --git a/xdocs/changes.xml b/xdocs/changes.xml index 645e0318c..8b247e810 100644 --- a/xdocs/changes.xml +++ b/xdocs/changes.xml @@ -136,6 +136,9 @@ Commons Math Release Notes Add Fraction constructor using max denominator value. + + Add integer overflow checks in Fraction constructor using double parameter. +