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

Bugfix: NullPointerException thrown for invalid input on JSONTokener without strict mode #936

Conversation

Simulant87
Copy link
Contributor

@Simulant87 Simulant87 commented Jan 8, 2025

expect input to be parsed as the invalid characters are at the end after a valid object, but got a NullPointerException instead, providing a fix with null checks.

fixing #933

expect JSONException but got NullPointerException
@Simulant87 Simulant87 changed the title NullPointerException for invalid input on JSONTokener Bugfix: NullPointerException thrown for invalid input on JSONTokener without strict mode Jan 8, 2025
@stleary
Copy link
Owner

stleary commented Jan 10, 2025

What if JSONTokener had a default JSONParserConfiguration?

@Simulant87
Copy link
Contributor Author

What if JSONTokener had a default JSONParserConfiguration?

That would also fix the NullPointerExceptio, if the JSONTokener would be initiated with a default JSONParserConfiguration.

However, the JSONParserConfiguration is not passed in the constructor and is not final. it can be changed by the setter after creation of the JSONTokener, which could also set it to null. In the case of passing null to the setter, it should then also create a new default JSONParserConfiguration.

@stleary
Copy link
Owner

stleary commented Jan 11, 2025

@Simulant87 The new unit tests can be kept after #937 is merged, but I believe the code in JSONObject and JSONArray will no longer be needed. But let me know if you disagree.

@Simulant87
Copy link
Contributor Author

I agree, the null checks would no be longer necessary. I integrated everything in #938 which validates it.

@Simulant87
Copy link
Contributor Author

this PR should be closed in favour of #938 (when that one is merged).

@stleary
Copy link
Owner

stleary commented Jan 17, 2025

Closing this PR as the work is being done in other tickets

@stleary stleary closed this Jan 17, 2025
@stleary stleary removed the In review label Jan 19, 2025
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