Reject out-of-range long and float values in NumberConverter#404
Conversation
The Long branch had no bounds check and the Float branch checked only the upper bound, so out-of-range input was silently clamped to Long.MAX_VALUE or to -Infinity instead of throwing. Add the missing checks mirroring the byte/short/int branches.
garydgregory
changed the title
garydgregory
left a comment
garydgregory
left a comment
There was a problem hiding this comment.
Hello @rootvector2
Please see my 2 comments.
| if (value.doubleValue() > Float.MAX_VALUE) { | ||
| throw ConversionException.format("%s value '%s' is too large for %s", toString(sourceType), value, toString(targetType)); | ||
| } | ||
| if (value.doubleValue() < -Float.MAX_VALUE) { |
There was a problem hiding this comment.
Why are we not using MIN_VALUE here?
There was a problem hiding this comment.
Float.MIN_VALUE is the smallest positive value (1.4E-45), not the most negative one, so it can't serve as the lower bound. The most negative finite float is -Float.MAX_VALUE. The Long branch above can use Long.MIN_VALUE because for integral types that is the most negative value, but for floating point types it isn't.
| // Maximum | ||
| assertEquals(Float.valueOf(Float.MAX_VALUE), converter.convert(clazz, max), "Maximum"); | ||
| // Minimum | ||
| assertEquals(Float.valueOf(-Float.MAX_VALUE), converter.convert(clazz, min), "Minimum"); |
There was a problem hiding this comment.
Why are we not using MIN_VALUE here?
There was a problem hiding this comment.
Same reason as in the converter: Float.MIN_VALUE is the smallest positive value, not the most negative, so the lowest representable float is -Float.MAX_VALUE.
|
TY @rootvector2 , merged 🚀 Please verify if this apply to the 1.X branch as well. |
|
yep, same issue on 1.X. the |
The Long branch had no bounds check and the Float branch checked only the upper bound, so out-of-range input was silently clamped to Long.MAX_VALUE or to -Infinity instead of throwing. Add the missing checks mirroring the byte/short/int branches. Ports the master fix (#404) to 1.X.
2 participants
NumberConverter.toNumber(Class, Number)range-checks thebyte/short/intbranches but theLongbranch has no bounds check and theFloatbranch only checks the upper bound, so an out-of-range value (for example a locale-parsed string pastlongrange, whichDecimalFormatreturns as aDouble) is silently clamped toLong.MAX_VALUEor to-Infinityinstead of throwing; found while auditing the numeric converters, fixed by adding the missing bounds checks next to the existing ones.mvn; that'smvnon the command line by itself.