From ddc6cab32f02196dfd908fe78852582a5b83600e Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 20 Jun 2026 08:07:03 -0400 Subject: [PATCH] Compare exact values in BigInteger and BigDecimal Number range checks - A reworked version of PR 402 with less code duplication - Javadoc - Reduce vertical whitespace --- .../routines/BigDecimalValidator.java | 72 +++++- .../routines/BigIntegerValidator.java | 207 ++++++++++-------- .../routines/BigDecimalValidatorTest.java | 18 ++ .../routines/BigIntegerValidatorTest.java | 19 ++ 4 files changed, 216 insertions(+), 100 deletions(-) diff --git a/src/main/java/org/apache/commons/validator/routines/BigDecimalValidator.java b/src/main/java/org/apache/commons/validator/routines/BigDecimalValidator.java index c339e6f5e..ea0bd679d 100644 --- a/src/main/java/org/apache/commons/validator/routines/BigDecimalValidator.java +++ b/src/main/java/org/apache/commons/validator/routines/BigDecimalValidator.java @@ -65,7 +65,6 @@ public class BigDecimalValidator extends AbstractNumberValidator { private static final long serialVersionUID = -670320911490506772L; - private static final BigDecimalValidator VALIDATOR = new BigDecimalValidator(); /** @@ -88,6 +87,23 @@ public static BigDecimalValidator getInstance() { return VALIDATOR; } + private static boolean isFinite(final Number value) { + if (value instanceof Double) { + return Double.isFinite((Double) value); + } + if (value instanceof Float) { + return Float.isFinite((Float) value); + } + return true; + } + + private static BigDecimal toBigDecimal(final Object value) { + if (value instanceof Long) { + return BigDecimal.valueOf(((Long) value).longValue()); + } + return value instanceof BigDecimal ? (BigDecimal) value : new BigDecimal(value.toString()); + } + /** * Constructs a strict instance. */ @@ -123,6 +139,17 @@ protected BigDecimalValidator(final boolean strict, final int formatType, final super(strict, formatType, allowFractions); } + /** + * Compares two values as BigDecimals. + * + * @param value1 {@code BigDecimal} to compare. + * @param value2 {@code BigDecimal} to which {@code value1} is to be compared. + * @return -1, 0, or 1 as this {@code BigDecimal} is numerically less than, equal to, or greater than {@code val}. + */ + private int compareTo(final Number value1, final Number value2) { + return toBigDecimal(value1).compareTo(toBigDecimal(value2)); + } + /** * Tests if the value is within a specified range. * @@ -146,6 +173,24 @@ public boolean maxValue(final BigDecimal value, final double max) { return Double.isFinite(max) ? compareTo(value, max) <= 0 : value.doubleValue() <= max; } + /** + * Tests if the value is less than or equal to a maximum, comparing the exact values. + * + *

+ * This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code double} before comparing and so loses + * precision for a {@code BigDecimal} that differs from the bound only beyond double precision. A non-finite {@link Double} or {@link Float} operand keeps + * the {@code doubleValue()} comparison so the documented infinity behaviour is unchanged. + *

+ * + * @param value The value validation is being performed on. + * @param max The maximum value. + * @return {@code true} if the value is less than or equal to the maximum. + */ + @Override + public boolean maxValue(final Number value, final Number max) { + return isFinite(value) && isFinite(max) ? compareTo(value, max) <= 0 : value.doubleValue() <= max.doubleValue(); + } + /** * Tests if the value is greater than or equal to a minimum. * @@ -157,6 +202,24 @@ public boolean minValue(final BigDecimal value, final double min) { return Double.isFinite(min) ? compareTo(value, min) >= 0 : value.doubleValue() >= min; } + /** + * Tests if the value is greater than or equal to a minimum, comparing the exact values. + * + *

+ * This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code double} before comparing and so loses + * precision for a {@code BigDecimal} that differs from the bound only beyond double precision. A non-finite {@link Double} or {@link Float} operand keeps + * the {@code doubleValue()} comparison so the documented infinity behaviour is unchanged. + *

+ * + * @param value The value validation is being performed on. + * @param min The minimum value. + * @return {@code true} if the value is greater than or equal to the minimum. + */ + @Override + public boolean minValue(final Number value, final Number min) { + return isFinite(value) && isFinite(min) ? compareTo(value, min) >= 0 : value.doubleValue() >= min.doubleValue(); + } + /** * Converts the parsed value to a {@code BigDecimal}. * @@ -166,12 +229,7 @@ public boolean minValue(final BigDecimal value, final double min) { */ @Override protected Object processParsedValue(final Object value, final Format formatter) { - BigDecimal decimal; - if (value instanceof Long) { - decimal = BigDecimal.valueOf(((Long) value).longValue()); - } else { - decimal = new BigDecimal(value.toString()); - } + BigDecimal decimal = toBigDecimal(value); final int scale = determineScale((NumberFormat) formatter); if (scale >= 0) { decimal = decimal.setScale(scale, BigDecimal.ROUND_DOWN); diff --git a/src/main/java/org/apache/commons/validator/routines/BigIntegerValidator.java b/src/main/java/org/apache/commons/validator/routines/BigIntegerValidator.java index 3535f4e95..af2acbb69 100644 --- a/src/main/java/org/apache/commons/validator/routines/BigIntegerValidator.java +++ b/src/main/java/org/apache/commons/validator/routines/BigIntegerValidator.java @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.apache.commons.validator.routines; import java.math.BigDecimal; @@ -25,49 +26,47 @@ /** * BigInteger Validation and Conversion routines ({@code java.math.BigInteger}). * - *

This validator provides a number of methods for - * validating/converting a {@link String} value to - * a {@code BigInteger} using {@link NumberFormat} - * to parse either:

- * + *

+ * This validator provides a number of methods for validating/converting a {@link String} value to a {@code BigInteger} using {@link NumberFormat} to parse + * either: + *

+ * * - *

Use one of the {@code isValid()} methods to just validate or - * one of the {@code validate()} methods to validate and receive a - * converted {@code BigInteger} value.

+ *

+ * Use one of the {@code isValid()} methods to just validate or one of the {@code validate()} methods to validate and receive a converted + * {@code BigInteger} value. + *

* - *

Once a value has been successfully converted the following - * methods can be used to perform minimum, maximum and range checks:

- * + *

+ * Once a value has been successfully converted the following methods can be used to perform minimum, maximum and range checks: + *

+ * * - *

So that the same mechanism used for parsing an input value - * for validation can be used to format output, corresponding - * {@code format()} methods are also provided. That is you can - * format either:

- * + *

+ * So that the same mechanism used for parsing an input value for validation can be used to format output, corresponding {@code format()} + * methods are also provided. That is you can format either: + *

+ * * * @since 1.3.0 */ public class BigIntegerValidator extends AbstractNumberValidator { private static final long serialVersionUID = 6713144356347139988L; - private static final BigIntegerValidator VALIDATOR = new BigIntegerValidator(); /** @@ -79,6 +78,17 @@ public static BigIntegerValidator getInstance() { return VALIDATOR; } + private static BigInteger toBigInteger(final Object value) { + if (value instanceof Long) { + return BigInteger.valueOf(((Long) value).longValue()); + } + if (value instanceof Double) { + // No need to roundtrip with a string. + return BigDecimal.valueOf(((Double) value).doubleValue()).toBigInteger(); + } + return value instanceof BigInteger ? (BigInteger) value : new BigDecimal(value.toString()).toBigInteger(); + } + /** * Constructs a strict instance. */ @@ -87,104 +97,118 @@ public BigIntegerValidator() { } /** - * Construct an instance with the specified strict setting - * and format type. + * Construct an instance with the specified strict setting and format type. * - *

The {@code formatType} specified what type of - * {@code NumberFormat} is created - valid types - * are:

- * + *

+ * The {@code formatType} specified what type of {@code NumberFormat} is created - valid types are: + *

+ * * - * @param strict {@code true} if strict - * {@code Format} parsing should be used. - * @param formatType The {@code NumberFormat} type to - * create for validation, default is STANDARD_FORMAT. + * @param strict {@code true} if strict {@code Format} parsing should be used. + * @param formatType The {@code NumberFormat} type to create for validation, default is STANDARD_FORMAT. */ public BigIntegerValidator(final boolean strict, final int formatType) { super(strict, formatType, false); } /** - * Check if the value is within a specified range. + * Tests if the value is within a specified range. * * @param value The {@code Number} value to check. - * @param min The minimum value of the range. - * @param max The maximum value of the range. - * @return {@code true} if the value is within the - * specified range. + * @param min The minimum value of the range. + * @param max The maximum value of the range. + * @return {@code true} if the value is within the specified range. */ public boolean isInRange(final BigInteger value, final long min, final long max) { - return value.compareTo(BigInteger.valueOf(min)) >= 0 && value.compareTo(BigInteger.valueOf(max)) <= 0; + return minValue(value, min) && maxValue(value, max); } /** - * Check if the value is less than or equal to a maximum. + * Tests if the value is less than or equal to a maximum. * * @param value The value validation is being performed on. - * @param max The maximum value. - * @return {@code true} if the value is less than - * or equal to the maximum. + * @param max The maximum value. + * @return {@code true} if the value is less than or equal to the maximum. */ public boolean maxValue(final BigInteger value, final long max) { return value.compareTo(BigInteger.valueOf(max)) <= 0; } /** - * Check if the value is greater than or equal to a minimum. + * Tests if the value is less than or equal to a maximum, comparing the exact values. + * + *

+ * This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code long} before comparing and so loses + * magnitude for a {@code BigInteger} outside the long range. + *

+ * + * @param value The value validation is being performed on. + * @param max The maximum value. + * @return {@code true} if the value is less than or equal to the maximum. + */ + @Override + public boolean maxValue(final Number value, final Number max) { + return toBigInteger(value).compareTo(toBigInteger(max)) <= 0; + } + + /** + * Tests if the value is greater than or equal to a minimum. * * @param value The value validation is being performed on. - * @param min The minimum value. - * @return {@code true} if the value is greater than - * or equal to the minimum. + * @param min The minimum value. + * @return {@code true} if the value is greater than or equal to the minimum. */ public boolean minValue(final BigInteger value, final long min) { return value.compareTo(BigInteger.valueOf(min)) >= 0; } /** - * Convert the parsed value to a {@code BigInteger}. + * Tests if the value is greater than or equal to a minimum, comparing the exact values. * - * @param value The parsed {@code Number} object created. + *

+ * This overrides the {@link Number} overload inherited from the superclass, which narrows the value to a {@code long} before comparing and so loses + * magnitude for a {@code BigInteger} outside the long range. + *

+ * + * @param value The value validation is being performed on. + * @param min The minimum value. + * @return {@code true} if the value is greater than or equal to the minimum. + */ + @Override + public boolean minValue(final Number value, final Number min) { + return toBigInteger(value).compareTo(toBigInteger(min)) >= 0; + } + + /** + * Converts the parsed value to a {@code BigInteger}. + * + * @param value The parsed {@code Number} object created. * @param formatter The Format used to parse the value with. - * @return The parsed {@code Number} converted to a - * {@code BigInteger}. + * @return The parsed {@code Number} converted to a {@code BigInteger}. */ @Override protected Object processParsedValue(final Object value, final Format formatter) { - if (value instanceof Long) { - return BigInteger.valueOf(((Long) value).longValue()); - } - if (value instanceof Double) { - // No need to roundtrip with a string. - return BigDecimal.valueOf(((Double) value).doubleValue()).toBigInteger(); - } - return new BigDecimal(value.toString()).toBigInteger(); + return toBigInteger(value); } /** - * Validate/convert a {@code BigInteger} using the default - * {@link Locale}. + * Validates and converts a {@code BigInteger} using the default {@link Locale}. * * @param value The value validation is being performed on. - * @return The parsed {@code BigInteger} if valid or {@code null} - * if invalid. + * @return The parsed {@code BigInteger} if valid or {@code null} if invalid. */ public BigInteger validate(final String value) { return (BigInteger) parse(value, (String) null, (Locale) null); } /** - * Validate/convert a {@code BigInteger} using the - * specified {@link Locale}. + * Validates and converts a {@code BigInteger} using the specified {@link Locale}. * - * @param value The value validation is being performed on. + * @param value The value validation is being performed on. * @param locale The locale to use for the number format, system default if null. * @return The parsed {@code BigInteger} if valid or {@code null} if invalid. */ @@ -193,10 +217,9 @@ public BigInteger validate(final String value, final Locale locale) { } /** - * Validate/convert a {@code BigInteger} using the - * specified pattern. + * Validates and converts a {@code BigInteger} using the specified pattern. * - * @param value The value validation is being performed on. + * @param value The value validation is being performed on. * @param pattern The pattern used to validate the value against. * @return The parsed {@code BigInteger} if valid or {@code null} if invalid. */ @@ -205,13 +228,11 @@ public BigInteger validate(final String value, final String pattern) { } /** - * Validate/convert a {@code BigInteger} using the - * specified pattern and/ or {@link Locale}. + * Validates and converts a {@code BigInteger} using the specified pattern and/ or {@link Locale}. * - * @param value The value validation is being performed on. - * @param pattern The pattern used to validate the value against, or the - * default for the {@link Locale} if {@code null}. - * @param locale The locale to use for the date format, system default if null. + * @param value The value validation is being performed on. + * @param pattern The pattern used to validate the value against, or the default for the {@link Locale} if {@code null}. + * @param locale The locale to use for the date format, system default if null. * @return The parsed {@code BigInteger} if valid or {@code null} if invalid. */ public BigInteger validate(final String value, final String pattern, final Locale locale) { diff --git a/src/test/java/org/apache/commons/validator/routines/BigDecimalValidatorTest.java b/src/test/java/org/apache/commons/validator/routines/BigDecimalValidatorTest.java index 6d47a82c9..688350399 100644 --- a/src/test/java/org/apache/commons/validator/routines/BigDecimalValidatorTest.java +++ b/src/test/java/org/apache/commons/validator/routines/BigDecimalValidatorTest.java @@ -238,4 +238,22 @@ void testBigDecimalValidatorMethods() { assertFalse(BigDecimalValidator.getInstance().isValid(xxxx, pattern), "isValid(B) pattern"); assertFalse(BigDecimalValidator.getInstance().isValid(patternVal, pattern, Locale.GERMAN), "isValid(B) both"); } + + /** + * The {@link Number} overloads inherited from the superclass must compare the exact value against a + * {@code BigDecimal} bound, not values narrowed to a double, so a difference smaller than double precision + * is not lost. + */ + @Test + void testNumberRangeBeyondDoublePrecision() { + final BigDecimalValidator validator = BigDecimalValidator.getInstance(); + final BigDecimal twoPow53 = BigDecimal.valueOf(2).pow(53); + // 2^53 + 1 collapses onto 2^53 as a double, so the double-based comparison cannot tell them apart + final Number value = twoPow53.add(BigDecimal.ONE); + final Number bound = twoPow53; + assertEquals(value.doubleValue(), bound.doubleValue()); + assertFalse(validator.maxValue(value, bound)); + assertTrue(validator.minValue(value, bound)); + assertFalse(validator.isInRange(value, twoPow53.subtract(BigDecimal.ONE), bound)); + } } diff --git a/src/test/java/org/apache/commons/validator/routines/BigIntegerValidatorTest.java b/src/test/java/org/apache/commons/validator/routines/BigIntegerValidatorTest.java index b3f0bc8d3..fe9609f3e 100644 --- a/src/test/java/org/apache/commons/validator/routines/BigIntegerValidatorTest.java +++ b/src/test/java/org/apache/commons/validator/routines/BigIntegerValidatorTest.java @@ -194,4 +194,23 @@ void testMinValueOutsideLongRange() { assertFalse(instance.minValue(belowMin, Long.MIN_VALUE)); assertFalse(instance.minValue(belowMin, Long.MAX_VALUE)); } + + /** + * The {@link Number} overloads inherited from the superclass must compare the exact value, not a value + * narrowed to a long, for BigIntegers outside the long range. + */ + @Test + void testNumberRangeOutsideLongRange() { + final BigIntegerValidator instance = BigIntegerValidator.getInstance(); + final Number min = BigInteger.valueOf(5); + final Number max = BigInteger.valueOf(100); + // 2^63 narrows to Long.MIN_VALUE, which the long-based comparison wrongly reports as below the range + final Number aboveMax = BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.ONE); + assertTrue(instance.minValue(aboveMax, min)); + assertFalse(instance.maxValue(aboveMax, max)); + // 2^64 + 50 narrows to 50, which the long-based comparison wrongly reports as in range + final Number wrapsIntoRange = BigInteger.ONE.shiftLeft(Long.SIZE).add(BigInteger.valueOf(50)); + assertEquals(50L, wrapsIntoRange.longValue()); + assertFalse(instance.isInRange(wrapsIntoRange, min, max)); + } }