# json_encode() bug - Thread: https://externals.io/message/34933 - Original archive: https://news-web.php.net/php.internals/34933 - Started by Stanislav Malyshev on 2008-01-25T22:20:42+00:00 - Messages: 13 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #34933 — Stanislav Malyshev — 2008-01-25T22:20:42+00:00 URL: https://externals.io/message/34933#34933 Hi! Right now, if json_encode sees wrong UTF-8 data, it just cuts the string in the middle, no error returned, no message produced. Example: var_dump(json_encode("ab\xE0")); var_dump(json_encode("ab\xE0\"")); Both strings get cut at "ab". I think it's not a good idea to just silently cut the data. In fact, I think it is a bug caused by this code in ext/json/utf8_to_utf16.c: if (c < 0) { return UTF8_END ? the_index : UTF8_ERROR; } which inherited this bug from code published on json.org. It should be: if (c < 0) { return (c == UTF8_END) ? the_index : UTF8_ERROR; } Now this is an easy fix but would lead to bad strings silently converted to empty strings. The question is - should we have an error there? If so, which one - E_WARNING, E_NOTICE? I'm for E_WARNING. Also filed as bug #43941. Any comments? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #34935 — Nathan Rixham — 2008-01-26T04:35:43+00:00 URL: https://externals.io/message/34933#34935 · In reply to: #34933 quick work around for now.. base64_decode(json_decode(json_encode(base64_encode("ab\xE0\" something")))) Stanislav Malyshev wrote: > Hi! > > Right now, if json_encode sees wrong UTF-8 data, it just cuts the string > in the middle, no error returned, no message produced. Example: > > var_dump(json_encode("ab\xE0")); > var_dump(json_encode("ab\xE0\"")); > > Both strings get cut at "ab". I think it's not a good idea to just > silently cut the data. In fact, I think it is a bug caused by this code > in ext/json/utf8_to_utf16.c: > if (c < 0) { > return UTF8_END ? the_index : UTF8_ERROR; > } > which inherited this bug from code published on json.org. It should be: > if (c < 0) { > return (c == UTF8_END) ? the_index : UTF8_ERROR; > } > Now this is an easy fix but would lead to bad strings silently converted > to empty strings. The question is - should we have an error there? If > so, which one - E_WARNING, E_NOTICE? I'm for E_WARNING. > Also filed as bug #43941. > Any comments? --- ## #35068 — Cristian Rodriguez — 2008-02-01T19:13:37+00:00 URL: https://externals.io/message/34933#35068 · In reply to: #34933 2008/1/25, Stanislav Malyshev : > Now this is an easy fix but would lead to bad strings silently converted > to empty strings. The question is - should we have an error there? If > so, which one - E_WARNING, E_NOTICE? I'm for E_WARNING. Yes , `E_WARNING` is the right thing to have IMHO. -- http://www.cristianrodriguez.net --- ## #35069 — Pierre Joye — 2008-02-01T19:39:38+00:00 URL: https://externals.io/message/34933#35069 · In reply to: #35068 > 2008/1/25, Stanislav Malyshev : > > > […] > > Yes , `E_WARNING` is the right thing to have IMHO. Stupid question, who actually checks for E_* in his code at runtime after having called such functions? Not me and I would hate to. It sounds to me like a perfect exception use case. As this function can return nearly everything scalar we have (boolean, string, null, integer,...) we can't use our normal "returns `FALSE` on failure". Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #35070 — Stanislav Malyshev — 2008-02-01T20:06:49+00:00 URL: https://externals.io/message/34933#35070 · In reply to: #35069 > Stupid question, who actually checks for E_* in his code at runtime > after having called such functions? Not me and I would hate to. It > sounds to me like a perfect exception use case. As this function can General policy in PHP as far as I know is that non-OO functions do not do exceptions. > return nearly everything scalar we have (boolean, string, null, > integer,...) we can't use our normal "returns `FALSE` on failure". This function can return only a string, but I'm not sure returning false if somewhere in some value deep down your data is some bad utf-8 is warranted. It is doable, though, but I'm afraid most of current code never check for return of `json_encode()` so they are in for big surprises when json_encode won't produce valid JSON. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #35071 — Pierre Joye — 2008-02-01T20:25:09+00:00 URL: https://externals.io/message/34933#35071 · In reply to: #35070 > > […] > > General policy in PHP as far as I know is that non-OO functions do not > do exceptions. > > > […] > > This function can return only a string, but I'm not sure returning false > if somewhere in some value deep down your data is some bad utf-8 is > warranted. It is doable, though, but I'm afraid most of current code > never check for return of `json_encode()` so they are in for big surprises > when json_encode won't produce valid JSON. Oh right, I thought about/read json_decode. But the idea is the same yes. Not checking return values is a very bad habit, I can't count how many times I saw codes dying miserably only because they don't check returns values. Would it make sense to use exception for such cases? Aka to change our policy? I'm not an excpeption fan, but these cases are good examples of exception usages. We can invent a meta type "error" but it would be rather confusing ;) -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #35098 — Derick Rethans — 2008-02-02T11:40:41+00:00 URL: https://externals.io/message/34933#35098 · In reply to: #34933 > Hi! > > Right now, if json_encode sees wrong UTF-8 data, it just cuts the string in > the middle, no error returned, no message produced. Example: > > var_dump(json_encode("ab\xE0")); > var_dump(json_encode("ab\xE0\"")); > > Both strings get cut at "ab". I think it's not a good idea to just silently > cut the data. In fact, I think it is a bug caused by this code in > ext/json/utf8_to_utf16.c: > if (c < 0) { > return UTF8_END ? the_index : UTF8_ERROR; > } > which inherited this bug from code published on json.org. It should be: > if (c < 0) { > return (c == UTF8_END) ? the_index : UTF8_ERROR; > } > Now this is an easy fix but would lead to bad strings silently converted to > empty strings. The question is - should we have an error there? If so, which > one - E_WARNING, E_NOTICE? I'm for E_WARNING. > Also filed as bug #43941. > Any comments? Like I mentioned before (I think), it should not return an empty string of course because programmatically it's not possible to check for this. As most of our functions return false in those cases, so should this function. Derick --- ## #35156 — Stanislav Malyshev — 2008-02-04T01:51:59+00:00 URL: https://externals.io/message/34933#35156 · In reply to: #35098 > Like I mentioned before (I think), it should not return an empty string > of course because programmatically it's not possible to check for this. > As most of our functions return false in those cases, so should this > function. AFAIR false is not valid JSON, so it would break a lot of code. Also, I am not sure we should change json_encode to return false on whole structure if one of the fields contains invalid utf-8. -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com --- ## #35162 — Derick Rethans — 2008-02-04T09:28:34+00:00 URL: https://externals.io/message/34933#35162 · In reply to: #35156 > > […] > > AFAIR false is not valid JSON, so it would break a lot of code. Also, I am not > sure we should change json_encode to return false on whole structure if one of > the fields contains invalid utf-8. Of course we should make it possible to detect if there is broken data. I think our normal mantra is "be strict with creating, be lenient while parsing". Because we're generating data here, we should *abort* if we find an error, and not return half baked results. To detect if we have broken data, the option is to check a return value. As normally this function returns a string, false (or null) is a good candidate for an error-result. regards, Derick -- Derick Rethans http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org --- ## #35201 — Richard Lynch — 2008-02-05T22:19:30+00:00 URL: https://externals.io/message/34933#35201 · In reply to: #35156 > > […] > > AFAIR false is not valid JSON, so it would break a lot of code. Also, > I > am not sure we should change json_encode to return false on whole > structure if one of the fields contains invalid utf-8. I see what you are saying, but... If you want to write good quality code, you have to do something reasonable here. PERHAPS: Return false `E_WARNING` Put partially decoded value somewhere in a variable to be accessed, so that the programmer can make an informed decision about what to do. -- Some people have a "gift" link here. Know what I want? I want you to buy a CD from some indie artist. http://cdbaby.com/from/lynch Yeah, I get a buck. So? --- ## #35225 — Derick Rethans — 2008-02-06T08:27:34+00:00 URL: https://externals.io/message/34933#35225 · In reply to: #35201 > > > […] [snip] > Put partially decoded value somewhere in a variable to be accessed, so > that the programmer can make an informed decision about what to do. We're talking about encoding here though... regards, Derick -- Derick Rethans http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org --- ## #37142 — Alan Knowles — 2008-04-19T04:43:07+00:00 URL: https://externals.io/message/34933#37142 · In reply to: #34933 This should really be fixed similar to the iconv //IGNORE flag - so that bad characters are just replaced with '?' We use it to render spam email summaries, and dont really care if the encoding is incorrect, just as long as it shows something. Throwing a warning without having a fix/workaround, just reduces the usefulness of the function. Regards Alan Stanislav Malyshev wrote: > Hi! > > Right now, if json_encode sees wrong UTF-8 data, it just cuts the > string in the middle, no error returned, no message produced. Example: > > var_dump(json_encode("ab\xE0")); > var_dump(json_encode("ab\xE0\"")); > > Both strings get cut at "ab". I think it's not a good idea to just > silently cut the data. In fact, I think it is a bug caused by this > code in ext/json/utf8_to_utf16.c: > if (c < 0) { > return UTF8_END ? the_index : UTF8_ERROR; > } > which inherited this bug from code published on json.org. It should be: > if (c < 0) { > return (c == UTF8_END) ? the_index : UTF8_ERROR; > } > Now this is an easy fix but would lead to bad strings silently > converted to empty strings. The question is - should we have an error > there? If so, which one - E_WARNING, E_NOTICE? I'm for E_WARNING. > Also filed as bug #43941. > Any comments? --- ## #37158 — Stanislav Malyshev — 2008-04-21T16:28:44+00:00 URL: https://externals.io/message/34933#37158 · In reply to: #37142 Hi! > This should really be fixed similar to the iconv //IGNORE flag - so > that bad characters are just replaced with '?' Maybe using iconv is a way to go in your scenario? -- Stanislav Malyshev, Zend Software Architect stas@zend.com http://www.zend.com/ (408)253-8829 MSN: stas@zend.com