json_decode edge case bugs!

ID:2301891
 
Oct 4 2017, 9:17 am
BYOND Version:511
Operating System:Linux
Web Browser:Firefox 58.0
Applies to:Dream Daemon
Status: Open
Issue hasn't been assigned a status value.
So somebody once went and tested a whole lot of JSON parsers. The blog post is here: http://seriot.ch/parsing_json.php, and the results are here: http://seriot.ch/json/parsing.html

So I went and made BYOND run on all of them. It's got lots of bugs to say the least.

Here's the results file from running BYOND, compared to Python 3.5.2 (because I didn't want to run the full suite): http://84.195.252.227/static/byond_json_parsing.html

Keep in mind that unlisted tests means the results were correct on both.

The code for the fork with the BYOND test can be found here: https://github.com/PJB3005/JSONTestSuite/tree/byond.

To run them (paths relative to folder containing BYOND-specific code):
  1. First be on Linux because I don't think you can even do proper command line arguments on BYOND on Windows. If you can tell me it's only some small code changes.
  2. Compile the test.dm file in parsers/test_BYOND.
  3. Compile the Rust project located in extlib/ with i686 Rust, copy target/debug/libextlib.so to be next to the dmb.
  4. Run the "BYOND" test with the project's test runner.


The Rust project was needed because I don't know of a better way to make DD exit with a controlled return code, which the test runner needs. The Rust code just gets compiled to a dylib that's invoked with call()() which exits the process.
Oct 4 2017, 9:52 am
A number of the "should have failed but succeeded" cases really don't bother me in the slightest since being a little more permissive isn't a bad thing as far as BYOND's handling is concerned, although some of these are bad ones like unmatched brackets. I'm puzzled why NaN and Infinity failed though, because those are parsed; and it's bizarre that negative Infinity succeeded when the others failed.
Oct 4 2017, 10:34 am
I was unable to get problem results from Infinity or NaN. Oddly your parser result document only shows a problem when comparing to Python, but in the section for BYOND itself those don't appear there, so something seems off about that test.

Some of the other cases, especially trailing garbage or bogus commas, were much bigger deals and also low-hanging fruit to fix, so a number of those should be fixed going forward. Even when being permissive I don't see that a case like ""x should make it through, and I have no problem with json_decode() freaking out on an extra comma (even though it's nicer not to).

The UTF-8, etc. issues are all relatively moot until such time as BYOND might support those in the future.
Oct 4 2017, 11:01 am
The issue with infinity and NaN and such specifically is that JSON doesn't define them, so it shouldn't pass.

The way the document is laid out is that the top-most table is every parser in one table, whereas the lower tables are specific to a single parser. I added Python because it was very easy for me to (just change the filter list), and I felt like it would provide some context. The -Infinity one is correctly handled (fail to parse) so it's not listed in the BYOND-only table.

I didn't expect all of them to get fixed (especially the unicode handling ones), though it did uncover ones like [1 true] so it's still useful info.
Oct 4 2017, 1:18 pm
Indeed, very useful info and I appreciate you taking the time to look into it. I think the improved parsing will be a big benefit.

Although JSON doesn't define Infinity or NaN, I still felt it was important to handle those cases. JavaScript itself does define them, so better to handle them. Likewise I see no harm in allowing single-quoted strings, although I draw the line at the ES6 backtick.
Oct 5 2017, 1:45 pm
Leniency when parsing JSON is better than leniency when stringifying into it. (Just another note, UTF-8 does not use 0xFF, so you can still support UTF8 in BYOND.)