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

Large numbers parsed as strings #497

Closed
slisaasquatch opened this issue Nov 21, 2019 · 12 comments
Closed

Large numbers parsed as strings #497

slisaasquatch opened this issue Nov 21, 2019 · 12 comments

Comments

@slisaasquatch
Copy link

slisaasquatch commented Nov 21, 2019

Here's the sample code:

  @Test
  public void testLargeNum() {
    final JSONObject j = new JSONObject("{\r\n" +
        "  \"foo\": 10000000000000000000000000000000\r\n" +
        "}");
    // The following line prints class java.lang.String!!!
    System.out.println(j.get("foo").getClass());
  }
@stleary
Copy link
Owner

stleary commented Nov 22, 2019

@slisaasquatch Thanks for bringing up this issue; interesting that numbers are handled differently depending on whether chars like '.' or 'e' are present. Do you want to present a case that the code is not in compliance with ECMA-404 and/or RFC8259? Otherwise, I believe this behavior has been in the code since the first commit for this repo (see stringToValue()).

@johnjaylward
Copy link
Contributor

I was pretty sure we changed this behavior a few years ago to use BigInteger instead for these cases. Let me try with a recent build

@johnjaylward
Copy link
Contributor

Ah, we never made the full switch. I left notes in the code on where it would be handled though:

https://github.com/stleary/JSON-java/blob/master/JSONObject.java#L2173-L2218

@stleary
Copy link
Owner

stleary commented Nov 22, 2019

I think that code review is still open, if you mean #453, due to changes to existing behavior.

@slisaasquatch
Copy link
Author

@slisaasquatch Thanks for bringing up this issue; interesting that numbers are handled differently depending on whether chars like '.' or 'e' are present. Do you want to present a case that the code is not in compliance with ECMA-404 and/or RFC8259? Otherwise, I believe this behavior has been in the code since the first commit for this repo (see stringToValue()).

Thanks for your quick response.

I checked the specs for numbers here, and all it says about really large numbers is that implementations may approximate the numbers to fit double precision floating points, and says nothing about turning large number into strings. And in this case, the number actually does fit into a double precision floating point.

@johnjaylward
Copy link
Contributor

@stleary, yes. that PR would fix this and open up the parser to have more exact representation of what was in the original JSON text.

@stleary
Copy link
Owner

stleary commented Nov 22, 2019

My best guess is that this implementation only guarantees integer interoperability to 2 ** 53 per the spec via Java Long type, and Double was only used to support decimal and scientific notation. I think if you have access to the software that generates very large numbers, it is probably better to just serialize them as strings.

@stleary
Copy link
Owner

stleary commented Nov 22, 2019

@johnjaylward Agreed, exact representation is preferred. I wanted to keep the PR open to see if there is a way to implement it as an option, to ensure existing applications don't break.

@johnjaylward
Copy link
Contributor

johnjaylward commented Nov 22, 2019

@stleary, gotcha. If I have time, I'll see if I can come up with a "parser config" type object that could be used to configure how items are parsed.

@slisaasquatch as a work around for your specific example, you can represent the number with a decimal point, or use scientific notation to force the parser to try to use a double as the backing type instead of an integer.

@Test
  public void testLargeNum() {
    final JSONObject j = new JSONObject("{\r\n" +
        "  \"foo\": 10000000000000000000000000000000.0\r\n" +
        "}");
    // The following line prints class Double!!!
    System.out.println(j.get("foo").getClass());
  }

OR:

@Test
  public void testLargeNum() {
    final JSONObject j = new JSONObject("{\r\n" +
        "  \"foo\": 1e31\r\n" +
        "}");
    // The following line prints class Double!!!
    System.out.println(j.get("foo").getClass());
  }

@javadev
Copy link
Contributor

javadev commented Dec 9, 2019

  @Test
  public void testLargeNum() {
    final JSONObject j = new JSONObject("{\r\n" +
        "  \"foo\": 10000000000000000000000000000000\r\n" +
        "}");
    // The following line prints class java.lang.String!!!
    System.out.println(j.get("foo").getClass());
  }

The expected value is class java.math.BigInteger.

@johnjaylward
Copy link
Contributor

@javadev yes. This has been a known limitation of the library for quite some time. When a number can not be parsed as an integer or a double, it stores it as a string internally. In our case, we only parse using a double if the value "looks" like a double (contains a . or e), otherwise we parse it as a long.

In cases where the caller requires a number, this can be worked around by using the getNumber, getDouble, getBigInteger or getBigDecimal, or their opt* equivalent methods to get a consistent numeric type.

According to the JSON specifications, a parser has no obligation to handle values that don't fit in 64-bit integer or floating point data type (long, or double in Java). This parser historically tried to maintain the value as close as possible by keeping the value as a string instead of ignoring it or throwing an exception.

@stleary
Copy link
Owner

stleary commented Jan 5, 2020

Closed due to working as designed.

@stleary stleary closed this as completed Jan 5, 2020
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