Skip to content

Conversation

@yuhengfdada
Copy link
Contributor

Analysis

As mentioned by Paul's comment in the jira, this is a bug in JsonParserCharArray. The problem does not exist in JsonParserUsingCharacterSource.
After debugging through both classes, both parser methods look exactly the same, except that CharacterSource would automatically add a 10(line feed) char at the end, which triggers an exception. But for JsonParserCharArray it ends right at the last curly brace so the exception is overlooked.

Fix

To keep the behavior consistent across JsonParserCharArray and JsonParserUsingCharacterSource, a line feed character is added to the end of the char array on initialization of JsonParserCharArray.

Concern

Performance

@yuhengfdada
Copy link
Contributor Author

With this change, this unit test is failing in JsonSlurperLaxTest.groovy:

    void testArrayOfArrayWithSimpleValues() {
        assert parser.parseText('[1, 2, 3, ["a", "b", "c", [true, false], "d"], 4]') ==
                [1, 2, 3, ["a", "b", "c", [true, false], "d"], 4]

        shouldFail(JsonException) { parser.parseText('[') }
        parser.parseText('[,]')
        shouldFail(JsonException) { parser.parseText('[1') }
        parser.parseText('[1,')
        shouldFail(JsonException) { parser.parseText('[1, 2') }
        parser.parseText('[1, 2, [3, 4]')
    }

Not sure why the author expected something like parser.parseText('[1, 2, [3, 4]') to not fail. Seems like an invalid JSON String.

@yuhengfdada
Copy link
Contributor Author

With this change, this unit test is failing in JsonSlurperLaxTest.groovy:

    void testArrayOfArrayWithSimpleValues() {
        assert parser.parseText('[1, 2, 3, ["a", "b", "c", [true, false], "d"], 4]') ==
                [1, 2, 3, ["a", "b", "c", [true, false], "d"], 4]

        shouldFail(JsonException) { parser.parseText('[') }
        parser.parseText('[,]')
        shouldFail(JsonException) { parser.parseText('[1') }
        parser.parseText('[1,')
        shouldFail(JsonException) { parser.parseText('[1, 2') }
        parser.parseText('[1, 2, [3, 4]')
    }

Not sure why the author expected something like parser.parseText('[1, 2, [3, 4]') to not fail. Seems like an invalid JSON String.

Having read the documentation, maybe JsonParserLax is expected to have this behavior. Unfortunately JsonParserLax extends JsonParserCharArray so this fix won't work. Closing the PR.

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

Successfully merging this pull request may close these issues.

1 participant