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

JsonExclusiveBadTerminationTestCase and EOF behaviour, expect exception? #10

Open
ssilverman opened this issue Oct 23, 2020 · 2 comments
Assignees

Comments

@ssilverman
Copy link
Contributor

ssilverman commented Oct 23, 2020

END_AFTER_ITEM_SEPARATOR("[1,", 2, true),
EOI_AFTER_COLON("{\"a\":", 2, true),
EOI_AFTER_PROPERTY_SEPARATOR("{\"a\": 1,", 3, true);

Similar to #9, since hasNext() is used to check for a next event, and since there's no next event because of an unexpected EOF, shouldn't a JsonParsingException be thrown instead? I'm wondering why this behaviour should be any different than the exceptions expected from the other test cases in these lists (JsonExclusiveBadTerminationTestCase and BadTerminationTestCase).

Open question: When should hasNext() return false and when should it throw an exception due to malformed JSON, i.e. EOF?
Personally, I believe that an unexpected EOF violates the JSON grammar and should cause a parsing exception, and the only time hasNext() should return false is at the end of input.

@leadpony leadpony self-assigned this Nov 1, 2020
@leadpony
Copy link
Owner

leadpony commented Nov 7, 2020

@ssilverman
Sorry for my slow response.
I checked the test cases and found inconsistencies in hasNext() method in the Reference Implementation, as shown below.

No. JSON to parse Result of hasNext()
1 (empty) hasNext(); // returns true 😕
2 { next();
hasNext(); // throws JsonParsingException
3 {"a" next();
next();
hasNext(); // throws JsonParsingException
4 {"a": next();
next();
hasNext(); // returns true 😕
5 {"a":1 next();
next();
next();
hasNext(); // throws JsonParsingException
6 {"a":1, next();
next();
next();
hasNext(); // returns true 😕
7 [ next();
hasNext(); // throws JsonParsingException
8 [1 next();
next();
hasNext(); // throws JsonParsingException
9 [1, next();
next();
hasNext(); // returns true 😕

I believe that all of hasNext() calls in No.1, 4, 6, and 9 should throw JsonParsingException.
If you can agree with me, I will post a new issue in Eclipse Jakarta JSON Processing.

@ssilverman
Copy link
Contributor Author

I agree with you.

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

2 participants