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

Inaccuracies and crashes in JSON parsing #66114

Open
RedMser opened this issue Sep 19, 2022 · 1 comment
Open

Inaccuracies and crashes in JSON parsing #66114

RedMser opened this issue Sep 19, 2022 · 1 comment

Comments

@RedMser
Copy link
Contributor

RedMser commented Sep 19, 2022

Godot version

4.0.dev (63c0dc6)

System information

Windows 10

Issue description

I ran the JSONTestSuite (test_parsing directory) using Godot's JSON.parse and have following findings:

  • Parsing following files crash Godot (due to too deep recursion I assume) -> Add recursion depth check to JSON.stringify/parse #66117:
    • n_structure_100000_opening_arrays.json
    • n_structure_open_array_object.json
  • None of the valid files caused an error (yay!)
  • But out of 318 files tested, 24 files that are malformed did not cause an error, namely:
    • trailing comma in array ["",] and object {"":"",}
      • they simply get ignored (while it is convenient, it does not conform specs)
    • unescaped newline or tab in string
      • get parsed as-is
    • trailing null byte in file (123 followed by a null byte)
      • gets ignored
    • invalid numbers: -01 and -2. and 0.e1 and 2.e+3 and 2.e-3 and 2.e3 and -.123
      • get parsed "as expected", JSON is less lenient on number formatting
    • invalid escapes: trying to \ escape an emoji, tab, x, a, or U -> Disallow invalid escape sequences in JSON.parse #66170
      • the backslash is consumed, but the to-be-escaped character remains
    • whitespace U+000c (formfeed) is illegal
      • Godot treats it as regular whitespace
    • "Unicode parsing error" are only printed to console, but parse still returns error code OK
      • usually seems to ignore the invalid characters

I updated the documentation via #66120 to make these limitations more transparent. But they should probably be tackled in the long-term.

Some of these are more severe than others (the crashes probably being the biggest issue!), so I'll let the core devs decide how to prioritize each of these findings.

Steps to reproduce

Run the MRP and observe the console output. A log file is additionally written, but does not include engine error messages!

Files in test_parsing that start with c are skipped since they crash, but can be renamed to reproduce the crashes.

Minimal reproduction project

json-unit-tests.zip

Also includes a log.txt if you do not intend to manually run the project.

@RedMser RedMser changed the title Inaccuracies in JSON parsing Inaccuracies and crashes in JSON parsing Sep 19, 2022
@Calinou Calinou added this to the 4.0 milestone Sep 19, 2022
@Calinou Calinou moved this to To Assess in 4.x Priority Issues Sep 19, 2022
@Calinou
Copy link
Member

Calinou commented Sep 19, 2022

  • trailing comma in array ["",] and object {"":"",}
    • they simply get ignored (while it is convenient, it does not conform specs)

See #40394.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants