Skip to content

reject fractional input in GenericTypeValidator integer helpers#423

Open
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:generictype-integer-fractional
Open

reject fractional input in GenericTypeValidator integer helpers#423
sahvx655-wq wants to merge 1 commit into
apache:masterfrom
sahvx655-wq:generictype-integer-fractional

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

Following the formatLong range-check fix, formatByte, formatInt and formatShort (the String, Locale overloads) still range-check the parsed number with num.doubleValue() and then narrow it with byteValue()/intValue()/shortValue(). The integer-only NumberFormat only stops parsing at the decimal separator, so a fractional value written with a negative exponent and no decimal point is consumed whole and handed back as a Double: under Locale.US "15E-1" (1.5) satisfies the double bounds and byteValue() floors it, so formatByte returns 1 and formatInt("5E-1") returns 0 instead of null. That earlier fix observed the sibling bounds are exact as doubles, which rules out the long magnitude gap but not this fractional one, so the three helpers report an integer the caller never entered.

Guard each of the three on num instanceof Long before the per-type range check, which NumberFormat returns only for a non-fractional value in long range, matching formatLong and routines.Byte/Short/IntegerValidator. Keeping the decision inside the conversion helper means a value that was never integral comes back as null rather than a silently truncated one. Added GenericTypeValidatorTest#testIntegerLocaleFractional, which fails on the current code and passes with the change.

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @sahvx655-wq
Please split the new test into one method per API tested, formatByte, formatShort, formatInt. TY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants