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

RFC 7159 compliancy #34

Open
314eter opened this issue Oct 26, 2016 · 6 comments
Open

RFC 7159 compliancy #34

314eter opened this issue Oct 26, 2016 · 6 comments
Labels
backlog Low-priority tasks likely to not get done anytime soon help wanted testing

Comments

@314eter
Copy link
Contributor

314eter commented Oct 26, 2016

I tested Yojson on the test cases of https://github.com/nst/JSONTestSuite:

CRASH   n_array_extra_comma.json
CRASH   n_array_incomplete_invalid_value.json
CRASH   n_array_number_and_comma.json
CRASH   n_object_pi_in_key_and_trailing_comma.json
CRASH   n_object_trailing_comma.json
CRASH   n_object_with_single_string.json
CRASH   n_structure_end_array.json
CRASH   n_structure_lone-invalid-utf-8.json
CRASH   n_structure_open_array_apostrophe.json
CRASH   n_structure_open_array_comma.json
CRASH   n_structure_open_object_close_array.json
CRASH   n_structure_open_object_comma.json
CRASH   n_structure_open_object_open_array.json
CRASH   n_structure_single_point.json
CRASH   n_structure_single_star.json
SHOULD_HAVE_PASSED  y_string_utf16.json
SHOULD_HAVE_FAILED  n_number_infinity.json
SHOULD_HAVE_FAILED  n_number_minus_infinity.json
SHOULD_HAVE_FAILED  n_number_NaN.json
SHOULD_HAVE_FAILED  n_object_repeated_null_null.json
SHOULD_HAVE_FAILED  n_object_trailing_comment.json
SHOULD_HAVE_FAILED  n_object_trailing_comment_slash_open.json
SHOULD_HAVE_FAILED  n_object_unquoted_key.json
SHOULD_HAVE_FAILED  n_string_invalid_utf-8.json
SHOULD_HAVE_FAILED  n_string_iso_latin_1.json
SHOULD_HAVE_FAILED  n_string_lone_utf8_continuation_byte.json
SHOULD_HAVE_FAILED  n_string_overlong_sequence_2_bytes.json
SHOULD_HAVE_FAILED  n_string_overlong_sequence_6_bytes.json
SHOULD_HAVE_FAILED  n_string_overlong_sequence_6_bytes_null.json
SHOULD_HAVE_FAILED  n_string_unescaped_crtl_char.json
SHOULD_HAVE_FAILED  n_string_unescaped_newline.json
SHOULD_HAVE_FAILED  n_string_unescaped_tab.json
SHOULD_HAVE_FAILED  n_string_UTF8_surrogate_U+D800.json
SHOULD_HAVE_FAILED  n_structure_<null>.json
SHOULD_HAVE_FAILED  n_structure_object_with_comment.json

The problems are:

  1. Some inputs (e.g. Yojson.Safe.from_string "x") raise a "Failure "lexing: empty token" exception instead of a Json_error.
  2. The numeric values NaN and Infinity, duplicate keys, comments, unquoted keys and unquoted control characters, tabs or newlines are not permitted according to RFC 7159. I don't think it's a problem to support some extensions to the specification, as long as to_string always returns valid json. In that case, only NaN and Infinity and duplicate keys are problematic.
  3. All other problems are UTF8 and UTF16 related. The handling of UTF8 and UTF16 is a design choice, so these can be ignored.
@mjambon
Copy link
Member

mjambon commented Oct 26, 2016

Cool.

@mjambon
Copy link
Member

mjambon commented Oct 26, 2016

This is a great test suite, and yojson doesn't have any. Would you like to leave a script that would fetch and run this test suite?

@mjambon mjambon added help wanted backlog Low-priority tasks likely to not get done anytime soon testing labels Jun 2, 2018
@NathanReb
Copy link
Collaborator

I'll add the successful ones to the current test suite and the rest of the test cases to a separate one.

Then we can start fixing the ones worth fixing and migrate them from one test suite to the other to ensure we don't break compliance in the future.

@314eter
Copy link
Contributor Author

314eter commented Feb 22, 2019

The crashes were all fixed by #35, so only the SHOULD_HAVE_FAILED are left.

@NathanReb
Copy link
Collaborator

Yeah I just ran the test suite I added right now with Yojson.Safe and the remaining failures are:

  • n_structure_object_with_comment.json
  • n_structure_angle_bracket_null.json
  • n_string_unescaped_tab.json
  • n_string_unescaped_newline.json
  • n_string_unescaped_crtl_char.json
  • n_object_unquoted_key.json
  • n_object_trailing_comment_slash_open.json
  • n_object_trailing_comment.json
  • n_object_repeated_null_null.json
  • n_number_minus_infinity.json
  • n_number_infinity.json
  • n_number_NaN.json

I'll polish the test suite a bit to get a better output on the failures and get it merged so we have something to work on.

@NathanReb
Copy link
Collaborator

It seems like it's a slightly different set of failures but the test suite now covers the more up-to-date RFC 8259 which explains some differences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Low-priority tasks likely to not get done anytime soon help wanted testing
Projects
None yet
Development

No branches or pull requests

3 participants