php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #69187 json_last_error return BC in PHP7
Submitted: 2015-03-04 18:13 UTC Modified: 2015-03-08 10:54 UTC
From: hdtfonseca at gmail dot com Assigned: bukka (profile)
Status: Closed Package: JSON related
PHP Version: master-Git-2015-03-04 (Git) OS: linux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: hdtfonseca at gmail dot com
New email:
PHP Version: OS:

 

 [2015-03-04 18:13 UTC] hdtfonseca at gmail dot com
Description:
------------
The output of the function json_last_error() is different in PHP master in some situations and this is causing a BC break.



Test script:
---------------
json_decode(null, true); 
echo json_last_error();

Expected result:
----------------
0

Actual result:
--------------
4

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-03-04 19:36 UTC] aharvey@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: bukka
 [2015-03-04 19:36 UTC] aharvey@php.net
Jakub, is this expected, and if so, can you update UPGRADING with the situations where this will happen, please?
 [2015-03-05 18:23 UTC] bukka@php.net
Hi Adam, this is actually caused by your commit ( https://github.com/php/php-src/commit/a7b3abe4e6f5e2fdfd8d55b676c9ca6b3f9c8cc8 ) :). The reason is the null is converted to empty string... The same will be for false.

I think that it makes sense to not emit error for null or false but only for empty string. json_decode already returns number when you pass number (json_decode(1)) so it makes sense to have it consistent. There is still a small inconsistency with boolean - json_decode(false) returns NULL and json_decode(true) returns 1. However I'm not sure if it's worthy it to break BC (even slightly)...

I will apply the patch for NULL and FALSE in a couple days if no objections.
 [2015-03-05 19:23 UTC] aharvey@php.net
Well, that was embarrassing. :)

Thanks for looking into this and in advance for the patch!
 [2015-03-06 07:23 UTC] nikic@php.net
@bukka I think the current PHP 7 behavior is correct. The json_decode function accepts a string and is documented to accept a string. According to our coercion rules null and false correspond to the empty string "", which is rightly no longer valid.

As such I don't think anything needs to change here. json_decode does not give you any guarantees that json_decode($nonString) === $nonString and I don't see why we'd want to introduce such a behavior.
 [2015-03-06 11:37 UTC] hdtfonseca at gmail dot com
I think @nikic has a point. What about, TRUE, 1 and 0 values? Shouldn't they all output JSON_ERROR_SYNTAX?

In PHP 5.6 they all return JSON_ERROR_NONE but I think that 1, 0, "", TRUE, FALSE and NULL should be consistent. At least "", FALSE and 0 should.
 [2015-03-06 12:20 UTC] nikic@php.net
@hdtfonseca: PHP also supports (as an extension to JSON - this is not supported by the standard itself) decoding JSON values that appear outside of object or array literals. E.g. it's possible to decode the string "1" to the value int(1). It just so happens that the string representation of bool(true) in PHP is "1", which is why json_decode(true) === 1 (without error).
 [2015-03-06 12:53 UTC] bukka@php.net
@nikic I think you are right. It should stay as a standard string conversion and we shouldn't have a special "hack" exceptions for specific types (null, false). I also think that we shouldn't have error for true because then we would have to have error for scalar int or float which would be unnecessary BC break. As such I think that it will be best to add just note about the conversion to the UPGRADING where is already mentioned that empty string will emit error.

btw. The decoding JSON values that appear outside of object or array literals is not just a PHP extension since March 2014 when RFC 7159 became a standard (see https://tools.ietf.org/html/rfc7159#section-2 ) ;)
 [2015-03-06 13:48 UTC] hdtfonseca at gmail dot com
Let's see if I got this.
NULL  will return 4
""    will return 4
FALSE will return 4
TRUE  will return 0
0     will return 0
1     will return 0
If I'm correct, I think that FALSE should return 0 instead.
(I'm creating a little test for this if you don't mind)
 [2015-03-06 18:52 UTC] bukka@php.net
Yes this is correct. The reason for that is type casting. You could write it as:

(string) NULL  === ""
(string) FALSE === ""
(string) TRUE  === "1"
(string) 0     === "0"
(string) 1     === "1"

Nikic has a strong point that the json_decode is documented with first argument "string $json" and as such we should follow ZPP casting rules in that case. It makes sense IMHO as it will be consistent with most internal functions.

I have updated UPGRADING in https://github.com/php/php-src/commit/9d037d574cc359405c7a818c2235644633705999
 [2015-03-06 23:12 UTC] hdtfonseca at gmail dot com
ok got it. I've finish the little tests. You can close this now.
 [2015-03-08 10:54 UTC] bukka@php.net
-Status: Assigned +Status: Closed
 [2015-03-08 10:55 UTC] bukka@php.net
Closed and the test merged. Thanks for the contribution.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Fri Dec 04 18:01:23 2020 UTC