Per documentation:

Return Values: Returns a JSON encoded string on success or FALSE on failure.

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?

I prefer option 2. 
It's an error condition anyway.

Haven't heard any screaming, so let's go with option 2.

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 aharvey@php.net to also look 
at in case they feel the test should remain or be modified to comply with the 
spec.

Thanks.

This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.

Sorry, the fix wasn't correct, had to revert it. Please correct the wrong things 
and reapply.

This fix was reverted on 5.4, but not for 5.3.14.
5.3.14 is now behaving different than 5.4.4: http://3v4l.org/eeXV2

I had previously submitted this PR in light of this bug report, but unaware of 
aharvey at php dot net's patch I withdrew it afterwards. I'm submitting it again 
as a temporary solution since it doesn't break anything else. Should this 
functionality also provide an option for going back to the old behavior as nikic 
suggests in the PR comments? https://github.com/php/php-src/pull/111

stas at php dot net:

If so please let me know what you think...