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

Fixes Unclosed json array stack overflow #373

Merged
merged 1 commit into from
Nov 3, 2017

Conversation

johnjaylward
Copy link
Contributor

@johnjaylward johnjaylward commented Oct 30, 2017

Key Changes:

  • Adds check for EOF in array parsing

What problem does this code solve?
Fixes #372.

Risks
None. This is a bug fix and no functionality is changed.

Changes to the API?
none

Will this require a new release?
No. This bug fix only changes what exception is thrown in a syntax error. It does not change parsing of valid documents.

Should the documentation be updated?
No

Does it break the unit tests?
No. All unit tests should pass. New unit tests were created to verify the correct behavior. see stleary/JSON-Java-unit-test#80

Was any code refactored in this commit?
no

Review status
ACCEPTED Starting 3 day comment window

Corrects behavior of unclosed arrays
@johnjaylward
Copy link
Contributor Author

rebased against master

@stleary
Copy link
Owner

stleary commented Oct 31, 2017

How did a non-cyclical stackoverflow stay hidden in the code all these years? @agouz Thanks for catching this, and @johnjaylward thanks for the fix. Starting 3 day window.

@stleary stleary merged commit d2a66a4 into stleary:master Nov 3, 2017
@johnjaylward
Copy link
Contributor Author

@stleary WTH was I thinking:

throw new JSONException(x.syntaxError("Expected a ',' or ']'"));

... that's wrong. should be:

throw x.syntaxError("Expected a ',' or ']'");

@johnjaylward johnjaylward deleted the UnclosedJSONArray branch November 3, 2017 02:34
@johnjaylward johnjaylward mentioned this pull request May 17, 2018
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.

StackOverflowError when creating a new JSONArray
2 participants