mtschoen
JSON Checker is potentially misleading
Hey Matt,
I love this project and am using it in my own work with Unity. Thanks so much for all your hard work on it!
Just wanted to mention that the JSONChecker is a bit misleading in that there are lots of malformed (invalid JSON) strings that return success in the checker, giving the quick impression that they are valid JSON. I know the screen at the bottom shows the output, but it still confused me for a while. It looks like the checker is checking whether JSONObject was able to parse the string at all, not whether the string is valid JSON. And it seems that the parser succeeds at making sense of lots of junky strings, although the returned data is pretty much always different.
To address this, maybe add a bit of clarifying language to the success message? It might also be worth a bit of extra language in the readme making it clear that the parser sometimes quietly accepts invalid JSON.
This all came about because I was looking for a way to validate JSON input in the Unity inspector, so I was looking for a built-in method in JSONObject to do that. I know adding a JSON validator is not probably something you want to do, but I just wanted to mention this issue since it might come up for others.
Thanks again for a great project!
Just noticed that my issue is pretty similar in spirit to issue #47
Consider mine a more general response to the same issue.
Hey there! Thanks for reporting this issue. Indeed, this non-standard behavior is by design--I want to do the best I can to interpret an invalid JSON string, rather than just throwing an exception or something if the input doesn't match the standard. Maybe this is ill-advised, but it's worked out so far. :)
I used to have a strict argument for the parse method which was an attempt to do proper validation but it wasn't exhaustive, and I never really used it anyway. I rewrote the parsing algorithm last year to be way more performant (the first version allocated a lot of strings and re-tread the input string in pretty horrendous ways) so it will likely act a little differently on non-standard input (as the person who submitted #47 discovered). In general, I consider all of this "undefined behavior" but the intent at least is to fail as gracefully as possible. Having said that, I wouldn't consider this approach to be "bullet-proof" and I wouldn't want this code to be used in some mission critical, life-or-death kind of situation. For folks who want "bullet-proof," I'd recommend Newtonsoft Json or, at this point, just using the built-in JsonUtility class which was introduced at some point after I originally created this library. While Newtonsoft was around at the time, the landscape was very different back in 2010 when I wrote the first version of this. :)
Anyway, I've added a note to JSONChecker in the latest version to make it clear it won't work as a validator. It's a good call since I can see how most people would find themselves in your position, expecting invalid JSON to spit out some sort of error. As for updating the readme--it needs a lot of work in general! 😅 I've made a note to do a pass on it but I've run out of steam for today.
Thanks for the thoughtful reply, Matt, and again for all your hard work :)
The project I'm working on includes some external code which uses JSONObject as a dependency, and at this point I'm not ready to rewrite that, so I'm going forward with JSONObject under the hood, at least for now.
The solution I came up with was to use Newtonsoft Json in my inspector data validation (all my json is stored in google sheets that end up as scriptable objects eventually). So when I inject new json into a scriptable object, Newtonsoft's checker spits out its detailed validation errors if anything is wrong. That way, I know JSONObject is getting clean json, so I don't have to worry about it on the JSONObject side. But since Newtonsoft is only running in the editor, it doesn't add any overhead or extra complexity to the build.
Maybe this approach will work for other folks as well.