Skip to content

Fixed bug #64874 ("json_decode handles whitespace and case-sensitivity incorrectly") #457

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

Closed

Conversation

hikari-no-yume
Copy link
Contributor

This fixes the JSON parser's handling of top-level primitives. Specifically, it fixes whitespace (previously "[true] " decoded fine but " true" didn't), and case (previously "tRue" decoded fine and [tRue] did not, however both are RFC non-compliant). This is a tiny change, but because of the very unlikely case that there was PHP code relying on deserialising strings like "Null", despite RFC non-compliance and no JSON serialisers outputting it, I must classify this as "backwards-incompatible". Hence, this should be merged into master and hopefully PHP 5.6.

I have a separate request which is the backwards-compatible half of this request that only fixes the whitespace issue. It is supposed to be merged into PHP 5.4 and PHP 5.5, since it is a non-backwards-incompatible bug fix: #456

EDIT: The backwards-compatible portion is now into 5.4, 5.5 and master. Hence this request now purely concerns the case-sensitivity portion.

@bukka
Copy link
Member

bukka commented Oct 16, 2013

Wouldn't be better to implement the white space fix to JSON_parser.c?

@hikari-no-yume
Copy link
Contributor Author

Well, that would involve modifying the parser to support these values at the top level in the first place. Which would mean reimplementing incorrect behaviour (the "tRue" bug) inside the parser and removing the wrapper code in the backwards-compatible half (and I wouldn't be surprised if this would introduce news bugs), then fixing the incorrect behaviour in this request. Unfortunately I don't understand the parser well enough to be able to make it support this. Perhaps the short-circuit is good anyhow, since it may be faster than using the parser directly.

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

Successfully merging this pull request may close these issues.

2 participants