Strict Mode - JSONArray converting incorrect input string to array

[rikkarth]

This fixes issue #871

StrictMode Implementation for JSONArray

JSONParserConfiguration

Followed the same pattern already in place in JSONParserConfiguration.

public class JSONParserConfiguration extends ParserConfiguration {

    /**
     * This flag, when set to true, instructs the parser to throw a JSONException if it encounters an invalid character
     * immediately following the final ']' character in the input. This is useful for ensuring strict adherence to the
     * JSON syntax, as any characters after the final closing bracket of a JSON array are considered invalid.
     */
    private boolean strictMode;

    ( ... )

    /**
     * Sets the strict mode configuration for the JSON parser.
     * <p>
     * When strict mode is enabled, the parser will throw a JSONException if it encounters an invalid character
     * immediately following the final ']' character in the input. This is useful for ensuring strict adherence to the
     * JSON syntax, as any characters after the final closing bracket of a JSON array are considered invalid.
     *
     * @param mode a boolean value indicating whether strict mode should be enabled or not
     * @return a new JSONParserConfiguration instance with the updated strict mode setting
     */
    public JSONParserConfiguration withStrictMode(final boolean mode) {
        JSONParserConfiguration clone = this.clone();
        clone.strictMode = mode;

        return clone;
    }

    ( ... )

}

Implementation Details

JSONArray main constructor now requires JSONParserConfiguration which controls the behaviour of the JSONArray.

    /**
     * Constructs a JSONArray from a JSONTokener and a JSONParserConfiguration.
     * JSONParserConfiguration contains strictMode turned off (false) by default.
     *
     * @param x                       A JSONTokener instance from which the JSONArray is constructed.
     * @param jsonParserConfiguration A JSONParserConfiguration instance that controls the behavior of the parser.
     * @throws JSONException If a syntax error occurs during the construction of the JSONArray.
     */
    public JSONArray(JSONTokener x, JSONParserConfiguration jsonParserConfiguration) throws JSONException {
        this();
        if (x.nextClean() != '[') {
            throw x.syntaxError("A JSONArray text must start with '['");
        }

        char nextChar = x.nextClean();
        if (nextChar == 0) {
            // array is unclosed. No ']' found, instead EOF
            throw x.syntaxError("Expected a ',' or ']'");
        }
        if (nextChar != ']') {
            x.back();
            for (; ; ) { ... }
        }
    }

If no JSONParserConfiguration is provided, a default JSONParserConfiguration will be added maintaining all the previous functionality. StrictMode is false/off by default.

    /**
     * Constructs a JSONArray from a JSONTokener.
     * <p>
     * This constructor reads the JSONTokener to parse a JSON array. It uses the default JSONParserConfiguration.
     *
     * @param x A JSONTokener
     * @throws JSONException If there is a syntax error.
     */
    public JSONArray(JSONTokener x) throws JSONException {
        this(x, new JSONParserConfiguration());
    }

2nd Part - Quotes Logic

You can find the quotes logic at the JSONTokener

    /**
     * This method is used to get the next value.
     *
     * @param c          The next character in the JSONTokener.
     * @param strictMode If true, the method will strictly adhere to the JSON syntax, throwing a JSONException if the
     *                   value is not surrounded by quotes.
     * @return An object which is the next value in the JSONTokener.
     * @throws JSONException If the value is not surrounded by quotes when strictMode is true.
     */
    private Object getValue(char c, boolean strictMode) {
        if (strictMode) {
            Object valueToValidate = nextSimpleValue(c, true);

            boolean isNumeric = valueToValidate.toString().chars().allMatch( Character::isDigit );

            if(isNumeric){
                return valueToValidate;
            }

            boolean hasQuotes = valueIsWrappedByQuotes(valueToValidate);

            if (!hasQuotes) {
                throw new JSONException("Value is not surrounded by quotes: " + valueToValidate);
            }

            return valueToValidate;
        }

        return nextSimpleValue(c);
    }

Unit Tests

You can find my test cases in JSONParserConfigurationTest

Every time a non-compliant case is found while in StrictMode, a JSONException is thrown at the exact position of where the conflict happened.

If a non-compliant case is found while StrictMode is off, then no exception will be thrown and the JSONArray will behave as before.

I've added several test cases on the unit test and created a platform to add more easily, feel free to play with it and add more if I missed some case.

Non Compliant Test Cases Log

Bellow is how the errors will be shown to the user, depending on which type of error the user produced with StrictMode it will attempt to show exactly what happened.

I hope this meets your requirements. Feel free to contact me for any further clarification,

Ricardo Mendes - a fan of this lib

[stleary]

@rikkarth Thanks for the PR. This project maintains backwards compatibility with Java 6 for the benefit of Android developers. Please look into updating the code to compile with Java 6. Also, please revert all refactorings that are not directly related to the purpose of this PR.

[rikkarth]

@stleary thank you for looking into my PR and for providing your feedback. I have done as you requested.

Let me know if there's any further action you require.

[stleary]

Please revert all refactorings that are not directly related to the purpose of this PR. Currently, there are about 3000 changed lines of code in your PR, which is too large for an effective review.

[rikkarth]

Please revert all refactorings that are not directly related to the purpose of this PR. Currently, there are about 3000 changed lines of code in your PR, which is too large for an effective review.

Sorry I now understand what you mean. I will do it and provide a more compliant commit.

Thank you.

[rikkarth]

Ok, it should be a lot more manageable now. I left only the changes that I consider directly related to this PR. Sorry for the bloat before.

[stleary]

@rikkarth Due to a recent merge, this code is out of date, please merge from the latest master branch

[rikkarth]

Good morning,

@stleary, Done as requested.

Let me know if there's anything else you need.

Have a good day.

[stleary]

Changing JSONTokener and the parsing code can be risky. These changes typically require additional review because some contributors are not fully up to speed on how parsers work, and the unit tests do not have good coverage of parser internals. Previous experience has shown that seemingly simple parser changes can result in unexpected regressions. Going forward you can expedite the review process by explaining your reasoning for changes to critical sections of the code, either in the PR or as comments in the code itself. Alternatively, you can try an implementation that minimizes changes to the parsing code. In the meanwhile, I will continue reviewing this code.

[stleary]

@rikkarth Looks good! I like the approach. There are some opportunities to simplify the parsing code, see comments.

[rikkarth]

I will apply the modifications you've suggested at my first opportunity.

Thank you for the review. ✌️🙂

[stleary]

@rikkarth Also, FYI, a recent commit that modified JSONTokener is causing a conflict. Please merge from the latest master branch and resolve the conflict, I don't think it will be a complex merge.

[stleary]

Notes to myself for a future PR, no action needed at this time.

• Strict mode now works for JSONArray, but not for JSONObject or for JSONArrays embedded in JSONObjects. A follow-up PR should add JSONObject support. For example, the following code parses without error:

        String str = "{\"a\": [b]}";
        JSONObject jsonObject = new JSONObject(str, new JSONParserConfiguration().withStrictMode(true));

• The indentation in nextString(char, boolean) is incorrect, fix it in a future PR.

[rikkarth]

Thank you again mr @stleary . I've performed all the changes as requested, please let me know if there's anything else that should be adjusted or changed and thank you for your incredible support in this PR.

[stleary]

What problem does this code solve? JSONArray should support strict mode, which disallows unquoted keys, unquoted values, and invalid chars a the end of the document, without breaking existing functionality. Strict mode is opt-in via a suitably initialized JSONParserConfiguration object.

Does the code still compile with Java6? Yes

Risks Low

Changes to the API? New API methods that use JSONParserConfiguration

Will this require a new release? No

Should the documentation be updated? Not at this time (full doc update should include all parser config settings)

Does it break the unit tests? No, new unit tests were added

Was any code refactored in this commit?

Review status APPROVED

Starting 3-day comment window

Thanks @rikkarth this was a lot of work, it looks good.