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

Unexcpected result when converting a xml string value starting with '0E' #265

Closed
janvandeklok opened this issue Aug 10, 2016 · 7 comments
Closed

Comments

@janvandeklok
Copy link

janvandeklok commented Aug 10, 2016

Hello people,

I am using your brilliant library org.json for converting xml to json.

I noticed that the 20160212 20160807 versions have a serious bug. When I convert this xml:
<OLTT_TUE_CURSUS_R><Cursus>0E010</Cursus><Cursus>0E050</Cursus><Cursus>0F010</Cursus><Cursus>aaaaaaa</Cursus></OLTT_TUE_CURSUS_R>

With the 20160807 version, I get this result : {"OLTT_TUE_CURSUS_R":{"Cursus":[0,0,"0F010","aaaaaaa"]}}

With the 20160212 version, I get this result : {"OLTT_TUE_CURSUS_R":{"Cursus":[0,0,"0F010","aaaaaaa"]}}
With the 20151123 version, I get this result : {"OLTT_TUE_CURSUS_R":{"Cursus":["0E010","0E050","0F010","aaaaaaa"]}}

It looks like that the 2016 versions interprets Strings that start with ‘0E’ as numeric values with exponents.

I would like to use the 2016 version since it supports OSGI. Is there a work around (some config setting or so) to handle this problem or do I have to wait for a software fix?

Here is the test code :

   String expectedResult="{\"OLTT_TUE_CURSUS_R\":{\"Cursus\":[\"0E010\",\"0E050\",\"0F010\",\"aaaaaaa\"]}}";
   String xml =  "<OLTT_TUE_CURSUS_R><Cursus>0E010</Cursus><Cursus>0E050</Cursus><Cursus>0F010</Cursus><Cursus>aaaaaaa</Cursus></OLTT_TUE_CURSUS_R> ";
 assertEquals(expectedResult, XML.toJSONObject(xml).toString());

Kind regards,

Jan van de Klok
CACI bv, Claudius Prinsenlaan 146 A, 4818 CP Breda, The Netherlands
T: +31 (0)88 6543429 | M: +31 (0) 655177129 | W: http://www.caci.nl

@stleary
Copy link
Owner

stleary commented Aug 10, 2016

@janvandeklok thanks for reporting this, we will investigate and get back to you soon.

@johnjaylward
Copy link
Contributor

@janvandeklok this is a duplicate of #243 . The latest version of the library that is being released later today will have support for this from PR #253.

In summary, PR #253 adds support for processing all XML values as strings. Your XML processing calls could be replaced as follows:

String expectedResult="{\"OLTT_TUE_CURSUS_R\":{\"Cursus\":[\"0E010\",\"0E050\",\"0F010\",\"aaaaaaa\"]}}";
   String xml =  "<OLTT_TUE_CURSUS_R><Cursus>0E010</Cursus><Cursus>0E050</Cursus><Cursus>0F010</Cursus><Cursus>aaaaaaa</Cursus></OLTT_TUE_CURSUS_R> ";
 assertEquals(expectedResult, XML.toJSONObject(xml,true).toString());

You can read the JavaDoc here: https://github.com/stleary/JSON-java/blob/master/XML.java#L333

@janvandeklok
Copy link
Author

janvandeklok commented Aug 10, 2016

@stleary I think I known the problem, the converter thinks it is a double value with an 'E' notation.
I noticed I can provide a boolean that will treat all values as strings but that is not feasible for me.
It would be nice to have an additional boolean argument that will allow me to ignore double values when the are in the notation '00E0000' or even better that we can provide an regex pattern for recognizing double values. In that way we can exclude certain patterns.

@johnjaylward
Copy link
Contributor

Yes, that is the issue. Your solution sounds reasonable, but I think it would be a fairly large refactor to support.

I personally dislike the E notation processing and wouldn't mind removing it altogether. I'm not sure how that would affect other users and how many actually like or expect that processing to work as it is today.

Would it be possible for you to use the new boolean flag, then post-process the resulting JSONObject to coerce other values?

The new JSONPointer (https://github.com/stleary/JSON-java/blob/master/JSONPointer.java) notation may be useful for the post-processing.

@janvandeklok
Copy link
Author

I've a workaround now. I checked out the code base and added a boolean for ignoring a 'E' notation for doubles : here is my Q&D solution in class JSONObject added 1 extra if statement and pass the boolean ignoreDoubleENotation around up to XML.toJsonObject(String, boollean, boolean :

public static Object stringToValue(String string, boolean ignoreDoubleENotation) {
if (string.equals("")) {
return string;
}
if (string.equalsIgnoreCase("true")) {
return Boolean.TRUE;
}
if (string.equalsIgnoreCase("false")) {
return Boolean.FALSE;
}
if (string.equalsIgnoreCase("null")) {
return JSONObject.NULL;
}

    /*
     * If it might be a number, try converting it. If a number cannot be
     * produced, then the value will just be a string.
     */

    char initial = string.charAt(0);
    if ((initial >= '0' && initial <= '9') || initial == '-') {
        try {
            if ((string.indexOf('e') > -1  || string.indexOf('E') > -1) && ignoreDoubleENotation) {
                 throw new Exception("Double E notation ignored");
            }

            if (string.indexOf('.') > -1  || "-0".equals(string) || string.indexOf('e') > -1  || string.indexOf('E') > -1) {
                Double d = Double.valueOf(string);
                if (!d.isInfinite() && !d.isNaN()) {
                    return d;
                }
            }  else {
                Long myLong = new Long(string);
                if (string.equals(myLong.toString())) {
                    if (myLong.longValue() == myLong.intValue()) {
                        return Integer.valueOf(myLong.intValue());
                    }
                    return myLong;
                }
            }

        } catch (Exception ignore) {
        }
    }
    return string;
}

@johnjaylward
Copy link
Contributor

That looks feasible. Instead of throwing the exception, you'd just return the value instead for the final product.

@johnjaylward
Copy link
Contributor

On second thought, after reviewing the spec again for another ticket, I don't think this is a good idea. The JSON Spec specifically allows "e" notation numbers and having a flag to disable them would be a weird option since it would explicitly break the spec.

Short of parsing the entire XML with the new boolean flag to force string values and then post-process, I'm not sure there is a good solution for you using this library as stock.

Feel free to fork it with your change though as it does look feasible for your needs.

@stleary stleary closed this as completed Aug 15, 2016
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

3 participants