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

Inconsistency JsonReader vs JsonTreeReader about double precision loss #2817

Open
d-william opened this issue Feb 24, 2025 · 3 comments
Open
Labels

Comments

@d-william
Copy link
Contributor

Relates #1737

Gson version

2.12.1

Description

JsonReader and JsonTreeReader do not have the same behavior while reading a long (or an int) when the next token is a double.
JsonReader will fail.
JsonTreeReader will cast, it should fail also.

String json1 = "{\"value\": 42.123}";
JsonReader reader1 = new JsonReader(new StringReader(json1));
reader1.beginObject();
reader1.nextName();
int result1 = reader1.nextInt(); // fails
System.out.println("Result 1 : " + result1);

vs

JsonObject json2 = new JsonObject();
json2.addProperty("value", 42.123);
JsonTreeReader reader2 = new JsonTreeReader(json2);
reader2.beginObject();
reader2.nextName();
int result2 = reader2.nextInt(); // cast
System.out.println("Result 2 : " + result2); // -> 42
@d-william d-william added the bug label Feb 24, 2025
@Marcono1234
Copy link
Collaborator

Thanks for reporting this! Might be the same as #1994; what do you think? That issue has an existing (but stale) pull request: #2031

@d-william
Copy link
Contributor Author

d-william commented Feb 24, 2025

Yes it is the same (with decimal overflow instead of long overflow).
Another possibility is to made the change in the JsonTreeReader class instead in the JsonPrimitive class (but in JsonPrimitive is better i guess).
Also, personaly, i prefer perform a check like what is done in here and like what has begun to be done here.
Instead of using BigDecimal.

Finaly, if keeping the actual behavior is choosen, the javadoc of JsonTreeReader#nextInt need to be specified.

@Marcono1234
Copy link
Collaborator

the javadoc of JsonTreeReader#nextInt need to be specified

The problem is that JsonTreeReader is not visible to users, the documentation could be changed but users would need to look at the implementation to see it. The documentation for JsonReader#nextInt (and similar) could be changed to mention that implementations might not detect precision loss. But I am not sure how helpful that would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants