Handle value overflow checks for IonParser.getIntValue() - #428 #481
Conversation
| return _reader.intValue(); | ||
| } | ||
| } catch (IonException | ||
| // 15-Jan-2024, tatu: other OSS-Fuzz tests suggest we need this: |
There was a problem hiding this comment.
Let's not add this back (I think @tgregg is working on handling these edge cases)
There was a problem hiding this comment.
Correct. If fuzzing identifies any new leaked exceptions I'd like to fix those in ion-java; in the meantime we can cover them in failing tests.
|
Looks good @thomasdelange5 -- thank you for contributing this! Only one other thing before I can merge this: if we haven't gotten CLA for you yet (it only needs to be done once per contributor): https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf (or possibly alternate Corporate CLA addendum if this is under some company's; we have a few Amazon contributors for Ion). |
|
Thank you for getting back so quickly @cowtowncoder! I will remove that catch clause and submit the CLA now. |
| @@ -390,7 +390,22 @@ public int getIntValue() throws IOException { | |||
| // @since 2.17 | |||
| private int _getIntValue() throws IOException { | |||
| try { | |||
| return _reader.intValue(); | |||
| if (getNumberType() == NumberType.LONG) { | |||
There was a problem hiding this comment.
Minor: for performance reasons, I recommend storing this value and using it in both branches. ion-java has to do some bounds checking on the values, and we should avoid doing that twice when possible.
There was a problem hiding this comment.
Ok will do. Thanks @tgregg
There was a problem hiding this comment.
was about to suggest same, +1
|
CLA received, will merge for 2.17(.0). Thank you, @thomasdelange5 ! |
|
Awesome, thank you very much!
…On Fri, Feb 9, 2024 at 5:40 PM Tatu Saloranta ***@***.***> wrote:
Merged #481
<#481> into
2.17.
—
Reply to this email directly, view it on GitHub
<#481 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BB3SKLXA4JYV3PQVSZNJK5DYS2QU3AVCNFSM6AAAAABDBVTUTKVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRG43DGMBUG42TOMY>
.
You are receiving this because you were mentioned.Message ID:
<FasterXML/jackson-dataformats-binary/pull/481/issue_event/11763047573@
github.com>
|
Add number size checks for IonParser getIntValue and getLongValue. Fix failing tests for #428