Edit report at https://bugs.php.net/bug.php?id=61537&edit=1
ID: 61537
Comment by: theanomaly dot is at gmail dot com
Reported by: j...@php.net
Summary: json_encode() incorrectly truncates/discards
information
Status: Assigned
Type: Bug
Package: JSON related
Operating System: all
PHP Version: 5.4.0
Assigned To: aharvey
Block user comment: N
Private report: N
New Comment:
The patch submitted fixes the problem.
json_ecode() has been incorrectly documented all along. Instead of
json_encode()
returning false it has been gladly returning a string containing "null" to
replace invalid data. Now, "null" is valid json, but the problem is the user
never ends up knowing that json_encode() silently failed unless they explicitly
check json_last_error() every single time they call json_encode(). Clearly this
was not the documented behavior.
It should be the case that when the user tries...
<?php
if (!(json_encode() = $json)) {
/* Then we have valid json to work with and there is no error */
} else {
/*
json_encode() failed and we should check json_last_error()
in order to know why
*/
?>
I also submitted a patch to fix the problem not realizing there was another
patch already there which fixes it too and allows it throw the error.
https://github.com/srgoogleguy/php-
src/commit/ba181fce117a3a7d1c7668cf30744b3d7cd7cdbf
However, now there is a test that's going to fail ext/json/tests/bug43941.phpt
which is testing for undocumented behavior. It was in response to this older
bug
report:
https://bugs.php.net/bug.php?id=43941
However, that fix never really fixed the problem. It just made it worse. In my
opinion we should remove this test. I just wanted ahar...@php.net to also look
at in case they feel the test should remain or be modified to comply with the
spec.
Thanks.
Previous Comments:
------------------------------------------------------------------------
[2012-04-02 01:19:07] ahar...@php.net
Haven't heard any screaming, so let's go with option 2.
------------------------------------------------------------------------
[2012-03-28 08:11:39] yohg...@php.net
I prefer option 2.
It's an error condition anyway.
------------------------------------------------------------------------
[2012-03-28 05:12:08] ahar...@php.net
I think we've got two options here:
1. Change the documented behaviour to note that invalid UTF-8 strings are
encoded as NULL, rather than causing json_encode() to return false, and advise
users to check json_last_error() if they're concerned about the output.
2. Change the json_encode() behaviour to match the documentation. Quick and
dirty POC patch which adds a switch to fall back to the current behaviour:
https://gist.github.com/7735daabc56fb38eb511
Option 2 has a BC concern: we've never advertised json_encode() as not
returning
false in this case, but people may rely on it.
Either way, I think we should probably drop the if (PG(display_errors)) { ... }
around the warning in json_escape_string().
Anyone else have thoughts?
------------------------------------------------------------------------
[2012-03-28 05:01:34] j...@php.net
Per documentation:
Return Values: Returns a JSON encoded string on success or FALSE on failure.
------------------------------------------------------------------------
[2012-03-28 05:01:01] j...@php.net
Description:
------------
When json_encode() cannot correctly encode its argument into JSON, the expected
behaviour is that it will return false, and people can check json_last_error()
to
see why it failed. Currently, it will replace all data it could not encode with
the string 'null'. This can lead to what appears to be silently discarding data.
Test script:
---------------
<?php
var_dump(json_encode(chr(215)), json_last_error());
Expected result:
----------------
bool(false)
int(5)
Actual result:
--------------
string(4) 'null'
int(5)
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=61537&edit=1
