Bug #62010 [Asn->Csd]: json_decode produces invalid byte-sequences

Edit report at https://bugs.php.net/bug.php?id=62010&edit=1 ID: 62010 Updated by: bukka@php.net Reported by: tklingenberg at lastflood dot net Summary: json_decode produces invalid byte-sequences -Status: Assigned +Status: Closed Type: Bug Package: JSON related Operating System: Windows PHP Version: 5.3.13 Assigned To: bukka Block user comment: N Private report: N New Comment: Automatic comment on behalf of bukka Revision: http://git.php.net/?p=php-src.git;a=commit;h=64c371142cbdb82eb0879d247430797d73a8ac2d Log: Fix bug #62010 (json_decode produces invalid byte-sequences) Previous Comments: ------------------------------------------------------------------------ [2015-06-08 18:12:21] bukka@php.net I have been thinking about that and also looking to some other parsers and it makes sense to have it as default. The RFC does not prohibit that but it gives a bit of freedom to the application (PHP parser in our case) to handle it in its way and the only sane way is to return error here. There is probably no use case to allow it so I propose it as a default for PHP 7. If no one objects, I will push it to master in the next few weeks. As I said previously, this is not a bug and as such I don't plan to backport it to PHP 5. However I will release a stable version of jsond in the next few months that will have it. ------------------------------------------------------------------------ [2015-06-05 09:34:49] tklingenberg at lastflood dot net Hi Bukka, I can perfectly see that the ABNF allows such chacarter-sequences. However to make "\u" mark a valid escape sequence, section 7 cleary documents when it qualifies as an escape sequence: > If the character is in the Basic Multilingual Plane (U+0000 through U+FFFF), then it may be represented as a six-character sequence: a reverse solidus, followed by the lowercase letter u, followed by four hexadecimal digits that encode the character's code point. So if the character that might to be expected represented after (that one) "\u" does not represent such a character, it was not this escape sequence. 8.2. only says, that a user providing such data should not expect it to work in a decoder ("[...] suffer fatal runtime exceptions."), it does not forbid to refuse processing this wrong encoded character data. In case of this report, the only last chance for PHP to escape from this madness so far was to document the return value of json_decode as string (without the requirement of it to be valid UTF-8). I think this is the actual flaw: The contract of json_decode is too broad. It should not just return some binary string, but all strings returned from the decoding should be UNICODE with the UTF-8 encoding. Otherwise no stable use of the interface is possible as the result is undeterministic. The error should be caught as early as possible. This could be by refusing the whole result (returning NULL to signal a character-data decomposigin error) or by just preserving the whole sequence: json_decode('"\ud834"') := '\ud834' # (PHP String) (leave the string verbatim, so to skip the wann-be escape sequence and continue afterwards.) I can't see in the specs that such a behaviour would not be allowed, especially by default. It does not even break backwards compatibility as the result earlier was already undetermined, too. So why not fix it, and keep the result by definition undeterministic :) Thanks for all your efforts and yes, please provide a fix. I hate to use another flag only to get something useful, but if that's the way it must be within PHP then be it. The way PHP defenses it's legacy, it's perhaps the only way to get such things in. Keep it rolling Tom ------------------------------------------------------------------------ [2015-05-31 20:30:18] bukka@php.net Just a small correction. The ABNF does not prohibit single unpaired UTF-16 surrogate (not surrogate sequence of course). ------------------------------------------------------------------------ [2015-05-31 20:04:01] bukka@php.net I think you don't understand the RFC and the string ABNF. Please read it carefully once again... mainly https://tools.ietf.org/html/rfc7159#section-7 You will see this ABNF: string = quotation-mark *char quotation-mark char = unescaped / escape ( %x22 / ; " quotation mark U+0022 %x5C / ; \ reverse solidus U+005C %x2F / ; / solidus U+002F %x62 / ; b backspace U+0008 %x66 / ; f form feed U+000C %x6E / ; n line feed U+000A %x72 / ; r carriage return U+000D %x74 / ; t tab U+0009 %x75 4HEXDIG ) ; uXXXX U+XXXX escape = %x5C ; \ quotation-mark = %x22 ; " unescaped = %x20-21 / %x23-5B / %x5D-10FFFF This line specifically shows that: %x75 4HEXDIG It doesn't say anything about prohibiting surrogate sequence for unicode escape. Actually the note about that is the section 8.2 that I quoted before. Especially this sentence says that: However, the ABNF in this specification allows member names and string values to contain bit sequences that cannot encode Unicode characters; for example, "\uDEAD" (a single unpaired UTF-16 surrogate). That is all about escaped sequences (see "\uDEAD"...). Of cource, binary strings have to be correctly encoded in UTF-8 (the only supported input encoding for PHP json parser) as stated in section 8.1. I understand that such unicode escapes might be inconvinient and that's why I emailed internals about introducing new constant for it that will address your issue. I plan to merge it to master next week but it will be just non-default option as we have to have RFC complained parser. I actually had it already implemented as a default when I was rewritting json for PHP 7. Then I noticed that the RFC says this and I removed it ( https://github.com/php/php-src/commit/1119c4d2b210730e6c26f034e9769d116b26ceb1 ). I changed it in jsond as well but in this case I added the constant which is what I plan to add to json now. ------------------------------------------------------------------------ [2015-05-31 15:42:49] tklingenberg at lastflood dot net Hi bukka, thank you for taking the time to look into it. But I'm very sorry to highlight that the information you've provided in your comment is best of all only remotely related to this issue and does not touch the root-cause of the flaw reported here. You've perhaps been misguided by the internals mailings (haven't read those), the part you quote is about binary string data. But the report I created is *not* about binary data, you can see, the string presented is US-ASCII without any control characters. It's about the strings _represented_ by JSON (not in binary), and more specifically the option to use an \uXXXX (six characters) escape sequence for any character in the Basic Multilingual Plane (U+0000 through U+FFFF). Please see Section 7 of the JSON RFC. U+D834 is not a character in the Basic Multilingual Plane (see Unicode, compare with a reference, exemplary: http://www.fileformat.info/info/unicode/char/d834/index.htm). If a string would have been passed json_decode containing the related binary sequence - as what you say would be allowed by the JSON spec - PHP handles it correctly according the documented contract: The binary sequence would qualify as *not* being an UTF-8 string and therefore the result of the function is unexpected: <?php $notUTF8 = "\"\xED\xA0\xB4\""; var_dump($notUTF8); // string(5) ""���"" $result = json_decode($notUTF8); var_dump($result); // NULL That's covered by the specs you quote, but not the flaw I reported here. As you can see, this is a different example and I can't see that PHP violates the spec nor it's own contract here. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=62010 -- Edit this bug report at https://bugs.php.net/bug.php?id=62010&edit=1