Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove EOFException special casing of JsonStreamParser.next() #2281

Conversation

Marcono1234
Copy link
Collaborator

@Marcono1234 Marcono1234 commented Dec 10, 2022

The previous behavior violated the Iterator contract where for JsonStreamParser("[") a call to hasNext() would return true, but next() would throw a NoSuchElementException.
I currently cannot think of a use case where such behavior would be desired for incomplete JSON. Maybe that code is just a historic remnant from when next() did not delegate to hasNext() which itself would run into the EOFException.

This should hopefully fix issue #2110 and #2176, though we can also keep them open and (hopefully) let oss-fuzz detect on its own that they are resolved.

The previous behavior violated the Iterator contract where for
`JsonStreamParser("[")` a call to `hasNext()` would return true,
but `next()` would throw a NoSuchElementException.
@eamonnmcmanus
Copy link
Member

Thanks! I ran this past all of Google's internal tests and didn't find any problems.

@eamonnmcmanus eamonnmcmanus merged commit f63a1b8 into google:master Dec 14, 2022
@Marcono1234 Marcono1234 deleted the marcono1234/JsonStreamParser-EOFException branch December 14, 2022 21:59
tibor-universe pushed a commit to getuniverse/gson that referenced this pull request Jan 20, 2023
…#2281)

* Remove EOFException special casing of JsonStreamParser.next()

The previous behavior violated the Iterator contract where for
`JsonStreamParser("[")` a call to `hasNext()` would return true,
but `next()` would throw a NoSuchElementException.

* Fix incorrect documented thrown exception type for JsonStreamParser

(cherry picked from commit f63a1b8)
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