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

JSONObject constructors do not check for non-finite numbers #713

Closed
Madjosz opened this issue Dec 13, 2022 · 5 comments · Fixed by #779
Closed

JSONObject constructors do not check for non-finite numbers #713

Madjosz opened this issue Dec 13, 2022 · 5 comments · Fixed by #779

Comments

@Madjosz
Copy link
Contributor

Madjosz commented Dec 13, 2022

Problem
The constructors org.json.JSONObject.JSONObject(Map<?, ?>) and org.json.JSONObject.JSONObject(Object) do not check occuring numbers in values for finiteness and thus allow creating of invalid JSON object which then run into strange behaviour when stringifying.

The Map constructor even mentiones @throws JSONException - If a value in the map is non-finite number. but this exception can only occur here when a nested call throws it, e.g. a value with a List containing non-finite numbers.

Code example

import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.Map;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.jupiter.api.Test;

class JSONObjectTest {
    @Test void jsonObject_map() {
        assertThrows(JSONException.class, () -> new JSONObject(Map.of("a", Double.POSITIVE_INFINITY)));
    }

    @Test void jsonObject_bean() {
        assertThrows(JSONException.class, () -> new JSONObject(new MyBean()));
    }
    public static class MyBean {
        public double getA() { return Double.POSITIVE_INFINITY; }
    }
}

Both tests fail and instead the constructors return an instance of a JSONObject with a key "a" mapped to a non-finite double.

Cascading problems
Calling toString() on the returned JSONObject will internally throw a JSONException in toString(0) while writing the non-finite value and this will be catched resulting in a null returned from the method. obj.getDouble("a") on the other hand will return the non-finite double.

Idea
While JSONObject.put(String, Object) checks for finiteness before putting the value to the map

testValidity(value);
this.map.put(key, value);
the other two calls
this.map.put(String.valueOf(e.getKey()), wrap(value));
this.map.put(key, wrap(result, objectsRecord));
just wrap() the value so it might be a good idea to do the finiteness-check there and document the JSONException.

@tsufiyan
Copy link

tsufiyan commented Apr 6, 2023

Shall I work on this? if yes, please assign

@stleary
Copy link
Owner

stleary commented Sep 30, 2023

Closing due to lack of activity. Please post here if you think it should be reopened.

@stleary stleary closed this as completed Sep 30, 2023
@Madjosz
Copy link
Contributor Author

Madjosz commented Oct 1, 2023

Why not apply my suggested fix instead or at least discuss why it could be bad?

@stleary
Copy link
Owner

stleary commented Oct 1, 2023

@Madjosz Thanks for reaching out. After reviewing this issue, I think it is worth fixing. The general rules here are:

  • The project is in maintenance mode, so most new or non-compliant features are not accepted.
  • Changes are user-driven. Someone has to step up, make the fix, and submit a pull request.
  • The change should fix a real problem.

Occasionally exceptions are made for trivial changes like updating Javadocs, adding a new API method, or for XML transformation fixes (which is a whole other problem area).

In this case, the problem and a potential fix are well-documented, but no one stepped up to fix it. Since Hacktoberfest has just started, I will label the issue accordingly. Someone will probably come along, make the change, and submit the PR. Or, you can do it yourself if you want.

@dubeyaditya29
Copy link

raised a PR #777

Madjosz added a commit to Madjosz/JSON-java that referenced this issue Oct 4, 2023
* fixes stleary#713
* document JSONException in JavaDoc
* remove unused Comparable<T> boundary to reuse GenericBean in test
dubeyaditya29 added a commit to dubeyaditya29/JSON-java that referenced this issue Oct 5, 2023
dubeyaditya29 added a commit to dubeyaditya29/JSON-java that referenced this issue Oct 5, 2023
Madjosz added a commit to Madjosz/JSON-java that referenced this issue Oct 7, 2023
* fixes stleary#713
* document JSONException in JavaDoc
* remove unused Comparable<T> boundary to reuse GenericBean in test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants