Skip to content

Fixes Unclosed json array stack overflow#373

Merged
stleary merged 1 commit intostleary:masterfrom
johnjaylward:UnclosedJSONArray
Nov 3, 2017
Merged

Fixes Unclosed json array stack overflow#373
stleary merged 1 commit intostleary:masterfrom
johnjaylward:UnclosedJSONArray

Conversation

@johnjaylward
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

rebased against master

@stleary
Copy link
Copy Markdown
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
Copy Markdown
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