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

Handling of null-values in JSONObject and XML.toString() #429

Closed
tobihein opened this issue Aug 3, 2018 · 13 comments
Closed

Handling of null-values in JSONObject and XML.toString() #429

tobihein opened this issue Aug 3, 2018 · 13 comments

Comments

@tobihein
Copy link

tobihein commented Aug 3, 2018

We have a json-string and want to convert it to a corresponding xml-string. In case, that there are null-values in the json-string, we get an unexpected result

JSon-String

{
   "myKey": null
}

What we do

String jsonString="{\"myKey\": null}";
final JSONObject jsonObject = new JSONObject( jsonString );
finalString xmlString = XML.toString( jsonObject));

What we get
<myKey>null</myKey>

What we expect
<myKey/>

Is our expectation correct? If yes, I would do a pull-request and change line 594 in class XML.java

@tobihein tobihein changed the title Handling of null-values in JSon2XML and XML.toString() Handling of null-values in JSONObject and XML.toString() Aug 3, 2018
@johnjaylward
Copy link
Contributor

No, in this case it's working as designed. In Javascript and JSON, null is a value (as opposed to undefined). In order to maintain the JSON value of null when converting to XML we output that as the string "null".

@tobihein
Copy link
Author

tobihein commented Aug 3, 2018

Thanks for your quick reply.

So in our case there is no difference in the xml for the input

{
   "myKey": null,
   "myKey2": "null"
}

Do you have a suggestion how we can get our expected output?

@johnjaylward
Copy link
Contributor

hmm, now that I'm looking at the code and the history of it, this may be a bug. At https://github.com/stleary/JSON-java/blob/master/XML.java#L594 you can see a null check for the value. This is the java null which this library uses to represent javascript undefined. However, there is no further check for the JSON null (https://github.com/stleary/JSON-java/blob/master/JSONObject.java#L165) which has a toString method that returns"null". (This is why null is output for your sample JSON. The library internally converts it to that JSONObject.NULL object).

In reality, the null check in the XML class should never happen since the JSONObject.put methods all remove keys with null values. Hence, the keySet call should never contain values that are JAVA null. I feel like the intent of that check was to actually check for JSON null and not JAVA null.

@stleary Let me know your opinion on this. It may be a small tweak we want to make.

@stleary
Copy link
Owner

stleary commented Aug 3, 2018

@johnjaylward thanks for investigating. Are you suggesting adding a check for JSON null or replacing the existing check?

@johnjaylward
Copy link
Contributor

Replacing the existing check with JSONObject.NULL.equals(value).

@stleary
Copy link
Owner

stleary commented Aug 3, 2018

Sounds reasonable, no objection on this end.

johnjaylward added a commit to johnjaylward/JSON-java that referenced this issue Aug 3, 2018
fixes stleary#429.

Updates JSON-> XML conversion to treat all NULL types the same. Previously a JSONObject having a key with a JSONObject.NULL value would output `<key>null</key>`. However, a JSONObject having a key with a value of JAVA `null` would output as `<key />`.

This change unifies the output so both now output as `<key />`.
@tobihein
Copy link
Author

tobihein commented Aug 3, 2018

Thanks, you two. My pull-request would have looked exactly like yours, johnjaylward.

I'm looking forward to the upcomming release.

@johnjaylward
Copy link
Contributor

I'm still updating it a little due to test failures since we were validating that functionality

@javadev
Copy link
Contributor

javadev commented Oct 18, 2018

Hi,

It may be a new attribute <myKey null="true"/>.

@tobihein
Copy link
Author

@javadev
For me this isn't an option because I can't change the schema of the output-xml.

We added a workaround in our code. We replace all JSONObject.NULL with "". But we would still be glad to have this changed in the library.

@tobihein
Copy link
Author

I saw the pull-request #412 for another feature request. Can you imagine to also make the output configurable by an additional parameter to the XML.toString-method? This would avoid a breaking change.

@javadev
Copy link
Contributor

javadev commented Oct 30, 2018

Thanks for your quick reply.

So in our case there is no difference in the xml for the input

{
   "myKey": null,
   "myKey2": "null"
}

Do you have a suggestion how we can get our expected output?

It may be converter to xml like this:

<?xml version="1.0" encoding="UTF-8"?>
<root>
  <myKey null="true"/>
  <myKey2>null</myKey2>
</root>

@stleary
Copy link
Owner

stleary commented Oct 30, 2018

#412 is a good idea. I will proceed with integrating with the unit tests and getting it committed. A section has been added to the Wiki FAQ that addresses how XML requests will be handled going forward.
https://github.com/stleary/JSON-java/wiki/FAQ#status-of-the-xml-code

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

No branches or pull requests

4 participants