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 possible NPE #160

Merged
merged 2 commits into from
Oct 18, 2015
Merged

Fixes possible NPE #160

merged 2 commits into from
Oct 18, 2015

Conversation

johnjaylward
Copy link
Contributor

The comparison for (object == null) ? "null" : escape(object.toString()) was doing nothing, escape(object.toString()) would always be called since object could never be null at this point. I have added a null check around the possible NPE areas so this check actually does something.

If we want the NPE, then I propose we just change the (object == null) ? "null" : escape(object.toString()) tertiary to escape(object.toString())

@stleary
Copy link
Owner

stleary commented Oct 14, 2015

Looks like the intended behavior is for XML.toString(Object) to emit the string ""null"" on line 483 when the object is null. However, an NPE is thrown when line 472 is executed, so this never happens. The unit tests expose the bug, but this was not understood at the time. Proposal is to fix this by rearranging the flow of control so that when the object is null, line 472 is not executed but line 483 is executed. The unit tests will need to be updated for the new behavior. See https://github.com/stleary/JSON-Java-unit-test/tree/XML-npe-test-fix.

@stleary
Copy link
Owner

stleary commented Oct 17, 2015

What problem does this code solve?
XML.toString(null) is intended to return "null" (in quotes), but instead a NullPointerException is thrown. This code enables the correct behavior.

Changes to the API?
No

Changes to how the code behaves?
The only change is within XML.toString(). The flow of control is corrected so that "null" is returned when the parameter is null.

Does it break the unit tests?
Yes, stleary/JSON-Java-unit-test#26 will need to be committed when this change is pushed.

Will this require a new release?
No, this change will be rolled into the next release.

Should the documentation be updated?
No, this is an internal bug fix.

stleary added a commit that referenced this pull request Oct 18, 2015
Fixes possible NullPointerException in XML.toString(object, tagName)
@stleary stleary merged commit 09b6af4 into stleary:master Oct 18, 2015
@johnjaylward johnjaylward deleted the FixXMLNPE branch October 18, 2015 21:53
BGehrels pushed a commit to BGehrels/JSON-java that referenced this pull request Apr 29, 2020
Fixes NPE in XML for pull request stleary#160 in the json-java project
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