Reject non-integer strings in integer locale converters

src/main/java/org/apache/commons/beanutils2/locale/converters/BigIntegerLocaleConverter.java

@@ -80,7 +80,11 @@ protected BigInteger parse(final Object value, final String pattern) throws Pars
             return (BigInteger) result;
         }
         if (result instanceof BigDecimal) {
-            return ((BigDecimal) result).toBigInteger();
+            try {
+                return ((BigDecimal) result).toBigIntegerExact();
+            } catch (final ArithmeticException e) {
+                throw new ConversionException("Supplied number is not an integer: " + result, e);
+            }
         }
         return BigInteger.valueOf(result.longValue());
     }

src/main/java/org/apache/commons/beanutils2/locale/converters/ByteLocaleConverter.java

@@ -74,6 +74,10 @@ protected Byte parse(final Object value, final String pattern) throws ParseExcep
         if (parsed.longValue() != parsed.byteValue()) {
             throw new ConversionException("Supplied number is not of type Byte: " + parsed.longValue());
         }
+        final double doubleValue = parsed.doubleValue();
+        if (doubleValue != Math.rint(doubleValue)) {
+            throw new ConversionException("Supplied number is not an integer: " + parsed);
+        }
         // now returns property Byte
         return Byte.valueOf(parsed.byteValue());
     }

src/main/java/org/apache/commons/beanutils2/locale/converters/IntegerLocaleConverter.java

@@ -74,6 +74,10 @@ protected Integer parse(final Object value, final String pattern) throws ParseEx
         if (parsed.longValue() != parsed.intValue()) {
             throw new ConversionException("Supplied number is not of type Integer: " + parsed.longValue());
         }
+        final double doubleValue = parsed.doubleValue();
+        if (doubleValue != Math.rint(doubleValue)) {
+            throw new ConversionException("Supplied number is not an integer: " + parsed);
+        }
         return Integer.valueOf(parsed.intValue()); // unlike superclass it will return proper Integer
     }
 }

src/main/java/org/apache/commons/beanutils2/locale/converters/LongLocaleConverter.java

@@ -79,6 +79,9 @@ protected Long parse(final Object value, final String pattern) throws ParseExcep
         if (doubleValue < Long.MIN_VALUE || doubleValue > Long.MAX_VALUE) {
             throw new ConversionException("Supplied number is not of type Long: " + result);
         }
+        if (doubleValue != Math.rint(doubleValue)) {
+            throw new ConversionException("Supplied number is not an integer: " + result);
+        }
         return Long.valueOf(result.longValue());
     }
 }

src/main/java/org/apache/commons/beanutils2/locale/converters/ShortLocaleConverter.java

@@ -84,6 +84,10 @@ protected Short parse(final Object value, final String pattern) throws ParseExce
         if (parsed.longValue() != parsed.shortValue()) {
             throw new ConversionException("Supplied number is not of type Short: " + parsed.longValue());
         }
+        final double doubleValue = parsed.doubleValue();
+        if (doubleValue != Math.rint(doubleValue)) {
+            throw new ConversionException("Supplied number is not an integer: " + parsed);
+        }
         // now returns property Short
         return Short.valueOf(parsed.shortValue());
     }

src/test/java/org/apache/commons/beanutils2/converters/BigIntegerLocaleConverterTest.java

@@ -176,12 +176,12 @@ void testConstructorMain() {
         convertInvalid(converter, "(A)", defaultValue);
         convertNull(converter, "(A)", defaultValue);
         // **************************************************************************
-        // Convert value in the wrong format - maybe you would expect it to throw an
-        // exception and return the default - it doesn't, DecimalFormat parses it
-        // quite happily turning "1,234" into "1"
-        // I guess this is one of the limitations of DecimalFormat
+        // Convert value in the wrong format - the localized (German) converter reads
+        // ',' as the decimal separator, so "1,234" parses to the fractional value
+        // 1.234; the integer converter now rejects a non-integer result and returns
+        // the default.
         // **************************************************************************
-        convertValueNoPattern(converter, "(B)", defaultIntegerValue, new BigInteger("1"));
+        convertValueNoPattern(converter, "(B)", defaultIntegerValue, defaultValue);
         // **************************************************************************
         // Convert with non-localized pattern - the trailing characters left after the
         // partial parse are now rejected, so the converter returns the default.
@@ -203,4 +203,13 @@ void testConstructorMain() {
         convertInvalid(converter, "(C)", defaultValue);
         convertNull(converter, "(C)", defaultValue);
     }
+
+    /**
+     * Tests that a non-integer value is rejected rather than silently truncated to an integer.
+     */
+    @Test
+    void testNonIntegerRejected() {
+        converter = BigIntegerLocaleConverter.builder().setDefault(defaultValue).setLocale(defaultLocale).get();
+        convertValueNoPattern(converter, "non-integer", "5.5", defaultValue);
+    }
 }

src/test/java/org/apache/commons/beanutils2/converters/ByteLocaleConverterTest.java

@@ -169,12 +169,12 @@ void testConstructorMain() {
         convertInvalid(converter, "(A)", defaultValue);
         convertNull(converter, "(A)", defaultValue);
         // **************************************************************************
-        // Convert value in the wrong format - maybe you would expect it to throw an
-        // exception and return the default - it doesn't, DecimalFormat parses it
-        // quite happily turning ",123" into "0"
-        // I guess this is one of the limitations of DecimalFormat
+        // Convert value in the wrong format - the localized (German) converter reads
+        // ',' as the decimal separator, so ",123" parses to the fractional value
+        // 0.123; the integer converter now rejects a non-integer result and returns
+        // the default.
         // **************************************************************************
-        convertValueNoPattern(converter, "(B)", defaultIntegerValue, Byte.valueOf("0"));
+        convertValueNoPattern(converter, "(B)", defaultIntegerValue, defaultValue);
         // **************************************************************************
         // Convert with non-localized pattern
         // **************************************************************************
@@ -196,4 +196,13 @@ void testConstructorMain() {
         convertInvalid(converter, "(C)", defaultValue);
         convertNull(converter, "(C)", defaultValue);
     }
+
+    /**
+     * Tests that a non-integer value is rejected rather than silently truncated to an integer.
+     */
+    @Test
+    void testNonIntegerRejected() {
+        converter = ByteLocaleConverter.builder().setDefault(defaultValue).setLocale(defaultLocale).get();
+        convertValueNoPattern(converter, "non-integer", "5.5", defaultValue);
+    }
 }

src/test/java/org/apache/commons/beanutils2/converters/IntegerLocaleConverterTest.java

@@ -164,12 +164,12 @@ void testConstructorMain() {
         convertInvalid(converter, "(A)", defaultValue);
         convertNull(converter, "(A)", defaultValue);
         // **************************************************************************
-        // Convert value in the wrong format - maybe you would expect it to throw an
-        // exception and return the default - it doesn't, DecimalFormat parses it
-        // quite happily turning "1,234" into "1"
-        // I guess this is one of the limitations of DecimalFormat
+        // Convert value in the wrong format - the localized (German) converter reads
+        // ',' as the decimal separator, so "1,234" parses to the fractional value
+        // 1.234; the integer converter now rejects a non-integer result and returns
+        // the default.
         // **************************************************************************
-        convertValueNoPattern(converter, "(B)", defaultIntegerValue, Integer.valueOf("1"));
+        convertValueNoPattern(converter, "(B)", defaultIntegerValue, defaultValue);
         // **************************************************************************
         // Convert with non-localized pattern - the trailing characters left after the
         // partial parse are now rejected, so the converter returns the default.
@@ -215,4 +215,13 @@ void testToPrimitiveType() {
         final int result = converter.convert(target, (Object) value.toString());
         assertEquals(value.intValue(), result, "Wrong result");
     }
+
+    /**
+     * Tests that a non-integer value is rejected rather than silently truncated to an integer.
+     */
+    @Test
+    void testNonIntegerRejected() {
+        converter = IntegerLocaleConverter.builder().setDefault(defaultValue).setLocale(defaultLocale).get();
+        convertValueNoPattern(converter, "non-integer", "5.5", defaultValue);
+    }
 }

src/test/java/org/apache/commons/beanutils2/converters/LongLocaleConverterTest.java

@@ -168,12 +168,12 @@ void testConstructorMain() {
         convertInvalid(converter, "(A)", defaultValue);
         convertNull(converter, "(A)", defaultValue);
         // **************************************************************************
-        // Convert value in the wrong format - maybe you would expect it to throw an
-        // exception and return the default - it doesn't, DecimalFormat parses it
-        // quite happily turning "1,234" into "1"
-        // I guess this is one of the limitations of DecimalFormat
+        // Convert value in the wrong format - the localized (German) converter reads
+        // ',' as the decimal separator, so "1,234" parses to the fractional value
+        // 1.234; the integer converter now rejects a non-integer result and returns
+        // the default.
         // **************************************************************************
-        convertValueNoPattern(converter, "(B)", defaultIntegerValue, Long.valueOf("1"));
+        convertValueNoPattern(converter, "(B)", defaultIntegerValue, defaultValue);
         // **************************************************************************
         // Convert with non-localized pattern - the trailing characters left after the
         // partial parse are now rejected, so the converter returns the default.
@@ -208,4 +208,13 @@ void testLongLimits() {
         assertThrows(ConversionException.class, () -> converter.convert("99999999999999999999"));
         assertThrows(ConversionException.class, () -> converter.convert("-99999999999999999999"));
     }
+
+    /**
+     * Tests that a non-integer value is rejected rather than silently truncated to an integer.
+     */
+    @Test
+    void testNonIntegerRejected() {
+        converter = LongLocaleConverter.builder().setDefault(defaultValue).setLocale(defaultLocale).get();
+        convertValueNoPattern(converter, "non-integer", "5.5", defaultValue);
+    }
 }

src/test/java/org/apache/commons/beanutils2/converters/ShortLocaleConverterTest.java

@@ -162,12 +162,12 @@ void testConstructorMain() {
         convertInvalid(converter, "(A)", defaultValue);
         convertNull(converter, "(A)", defaultValue);
         // **************************************************************************
-        // Convert value in the wrong format - maybe you would expect it to throw an
-        // exception and return the default - it doesn't, DecimalFormat parses it
-        // quite happily turning "1,234" into "1"
-        // I guess this is one of the limitations of DecimalFormat
+        // Convert value in the wrong format - the localized (German) converter reads
+        // ',' as the decimal separator, so "1,234" parses to the fractional value
+        // 1.234; the integer converter now rejects a non-integer result and returns
+        // the default.
         // **************************************************************************
-        convertValueNoPattern(converter, "(B)", defaultIntegerValue, Short.valueOf("1"));
+        convertValueNoPattern(converter, "(B)", defaultIntegerValue, defaultValue);
         // **************************************************************************
         // Convert with non-localized pattern - the trailing characters left after the
         // partial parse are now rejected, so the converter returns the default.
@@ -189,4 +189,13 @@ void testConstructorMain() {
         convertInvalid(converter, "(C)", defaultValue);
         convertNull(converter, "(C)", defaultValue);
     }
+
+    /**
+     * Tests that a non-integer value is rejected rather than silently truncated to an integer.
+     */
+    @Test
+    void testNonIntegerRejected() {
+        converter = ShortLocaleConverter.builder().setDefault(defaultValue).setLocale(defaultLocale).get();
+        convertValueNoPattern(converter, "non-integer", "5.5", defaultValue);
+    }
 }