Large integers do not work

[effad]

The following piece of code surprisingly outputs class java.lang.String, instead of a number-class like BigDecimal or BigInteger.

import org.json.JSONException;
import org.json.JSONObject;


public class CodeTest {

    public static void main(String[] args) throws JSONException {
        String str = "{\"number_value\":9223372036854776000}";
        JSONObject x = new JSONObject(str);
        System.out.println(x.get("number_value").getClass());
    }
}

Since JSON does not limit the size of numbers (to the best of my knowledge) I consider this to be a bug.
The culprit is JSONObject.stringToValue which will not work with numbers beyond the range of Java Long.

[stleary]

Please see #135 and a full discussion of the issue in #126.

[johnjaylward]

@stleary I'm not sure this should be closed yet. It could be considered a bug under the same assumptions you made in #126 (comment)

If this was in a shop, it would probably be flagged as a bug and fixed. The actual JSON is specifying an INT value, but the conversion from a JSON-String to a JSONObject is not seeing it as such.

[stleary]

I don't think the JSON spec requires an implementation to maintain any particular level of numeric support, but agree that it is not optimal to be inconsistent, My recollection from the code fix was that this particular result fell under the requirement not to inconvenience users by changing existing behavior. Is there a particular use case that you think might override this requirement?

[effad]

While I can see the problem of backward compatibility, I think it intolerable that you cannot make an identical copy of a JSONObject by using new JSONObject(existing.toString());

Here's a full example:

import org.json.JSONException;
import org.json.JSONObject;
import java.math.BigDecimal;

public class CodeTest {
    public static void main(String[] args) throws JSONException {
        JSONObject x = new JSONObject();
        x.put("num", new BigDecimal("9223372036854776000"));
        JSONObject y = new JSONObject(x.toString());
        System.out.println(x.get("num").equals(y.get("num")));
    }
}

I also agree that JSON does not require a particular level of numeric support, but since this implementation does support BigDecimal it should do so in a consistent way.

If any current usage of the library relies on the fact that { x: 123..lotsofdigits... } returns a string while { x: 123...notsomanydigits...} returns a Number object ... I would consider such usage broken and my guess is that more people will be affected by the inconsistent handling

[mouse07410]

I'd prefer that { x: 123...321} always returned a Number object rather than a String, regardless of how big that number is.

[stleary]

@effad, So far as I can tell, the API makes no representation that the JSONObject(String) ctor and toString() are reversible. Are you saying the library should be required to have this behavior?

The big numbers enhancement came with a requirement to absolutely guarantee existing apps are not impacted. I don't think our use case can be that their apps were pre-broken anyway, or at least I would not feel comfortable taking that position.

[johnjaylward]

My pull request doesn't fix the double rounding issues that currently exist, it only extends the range of doubles so that if converting a valid decimal value to double would cause NaN or +-Infinity to be returned, it returns a BigDecimal instead.

It does however fix the BigInteger problem so that all values of large integers are stored correctly.

You could make the argument that the API should store all numeric values as either BigDecimal or BigInteger. The get/opt methods are already doing the conversions on the fly:

    /**
     * Get the long value associated with a key.
     *
     * @param key
     *            A key string.
     * @return The long value.
     * @throws JSONException
     *             if the key is not found or if the value cannot be converted
     *             to a long.
     */
    public long getLong(String key) throws JSONException {
        Object object = this.get(key);
        try {
            return object instanceof Number ? ((Number) object).longValue()
                    : Long.parseLong((String) object);
        } catch (Exception e) {
            throw new JSONException("JSONObject[" + quote(key)
                    + "] is not a long.");
        }
    }

[johnjaylward]

@stleary the JSON spec states that {x:123456789123456789123456789} is a number. If the API converts it to a string, then the API is not following the JSON spec.

I'd say this is a bug, and anyone relying on this functionality now is doing so as a workaround to this bug.

[johnjaylward]

If we do fix this though, we should be very clear in the release notes about this fix, and that it could break existing code.

[douglascrockford]

The JSON spec allows implementations to put reasonable limits on things.

Is this actually a bug, or are you just staying so?

[johnjaylward]

Thanks, I missed that in the RFC, I was going off the flow chart @ http://json.org. I found the note though:

This specification allows implementations to set limits on the range
and precision of numbers accepted.  Since software that implements
IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
generally available and widely used, good interoperability can be
achieved by implementations that expect no more precision or range
than these provide, in the sense that implementations will
approximate JSON numbers within the expected precision.  A JSON
number such as 1E400 or 3.141592653589793238462643383279 may indicate
potential interoperability problems, since it suggests that the
software that created it expects receiving software to have greater
capabilities for numeric magnitude and precision than is widely
available.

Note that when such software is used, numbers that are integers and
are in the range [-(2**53)+1, (2**53)-1] are interoperable in the
sense that implementations will agree exactly on their numeric
values.

[johnjaylward]

although, in this issues defense, http://json.org, nor this projects README references the RFC that I pulled the text from. JSON.org references ECMA-404 The JSON Data Interchange Standard

I propose that if we do not fix this, then we note in the ReadMe what the limitation on integer and double values are as well as link to the RFC : https://tools.ietf.org/html/rfc7159

[effad]

While there is nothing in RFC-4627 that prevents a parser to transform JSON input into whatever it wants to I find the current state of affairs extremely misleading. On the one hand the library suggests it supports BigDecimal while on the other hand it does not and does so in a silent and most unexpected way.
If an exception (IllegalStateException?) were raised, the implementation limit would at least be communicated clearly. However this would also change the behaviour of the library.

BTW at 9a0471d the behaviour was also changed by removing an API function.

[johnjaylward]

@effad that was before the API was "stable" and also to enforce the "json keys are unordered". The golang JSON implementation purposefully randomizes the keys so that people don't even rely on the hash function hashing the keys into the same order.

However, I still feel that if we are going to support "java8" and BigDecimal/BigInteger, this should be corrected and not put off as "but the RFC says we can be lazy about it"

[johnjaylward]

if anyone cares, I updated my branch for this change to use exact precision for all numbers as well as to validate that string input matches JSON numbers and not any java number.

you can see the changes at branch https://github.com/johnjaylward/JSON-java/tree/FixBigIntRead

I also updated all the test cases to pass for these changes which can be seen at https://github.com/johnjaylward/JSON-Java-unit-test/tree/FixBigIntRead

I made updates after pr #158 was closed. The new changes remove exceptions as flow control and clean up a lot of my initial implementation.

[coderextreme]

-0 is in the JSON spec, and it's not a huge number and it gets converted to a string. Can this particular part of the bug be fixed? I don't mind large numbers not being handled. I realize NEGATIVE_ZERO may not be a Java primitive value. Puleese? This is breaking JSON Schema validation as -0 is not a number.

[johnjaylward]

you should comment on your open issue, not this closed one

[coderextreme]

k