php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64874 json_decode handles whitespace and case-sensitivity incorrectly
Submitted: 2013-05-17 21:48 UTC Modified: 2013-11-18 22:07 UTC
Votes:8
Avg. Score:4.4 ± 0.7
Reproduced:7 of 7 (100.0%)
Same Version:6 (85.7%)
Same OS:5 (71.4%)
From: chrivers at iversen-net dot dk Assigned: ajf
Status: Closed Package: JSON related
PHP Version: 5.4.15 OS: All
Private report: No CVE-ID:
 [2013-05-17 21:48 UTC] chrivers at iversen-net dot dk
Description:
------------
There are 2 problems with the json_decode function.

1) It only sometimes disregards whitespace

The RFC clearly says:

"""Insignificant whitespace is allowed before or after any of the six structural 
characters."""

2) It only sometimes enforces lowercase-ness of identifiers

The RFC clearly says:

"""The literal names MUST be lowercase. No other literal names are allowed."""

The test script demonstrates this

Test script:
---------------
<?

function json_cmp($x, $y)
{
  print var_dump(json_decode($x) === $y);
}

// works
json_cmp("true", true);

// fails - is actually true
json_cmp("tRue", NULL);

// fails - is actually NULL
json_cmp("true ", true);

// works
json_cmp("[true ] ", array(true));

// works, even though the non-array version fails
json_cmp("[tRue]", NULL);

?>


Expected result:
----------------
true * 5

Actual result:
--------------
bool(true)
bool(false)
bool(false)
bool(true)
bool(true)


Patches

JSONFix_with_test_fixed.patch (last revision 2013-09-10 22:07 UTC) by ajf at ajf dot me)
JSONFix_with_test.patch (last revision 2013-09-10 21:38 UTC) by ajf at ajf dot me)
JSONFix.patch (last revision 2013-09-10 21:23 UTC) by ajf at ajf dot me)

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-05-18 15:00 UTC] cmbecker69 at gmx dot de
RFC 4627[1] also states in section 2:

| A JSON text is a serialized object or array.
| 
|    JSON-text = object / array

According to that definition the $json argument of examples 1-3 
is not a valid JSON-text.

Furthermore:
  json_decode('true ');
  var_dump(json_last_error() === JSON_ERROR_SYNTAX);
prints:
  bool(true)
So the returned NULL is actually correct according to the documentation.

[1] <http://www.ietf.org/rfc/rfc4627.txt>
 [2013-05-19 20:04 UTC] chrivers at iversen-net dot dk
Well, the part of the RFC that you're quoting describes the "JSON-text" type, 
which indeed must be non-
primitive.

However, the json_decode() function is documented as taking a "json value", 
which according to the spec 
is 

"""
   A JSON value MUST be an object, array, number, or string, or one of
   the following three literal names:

      false null true
"""

So that's perfectly fine, really.

There are other errors, too. For example, " true" WORKS while "true " fails, 
which makes no sense at 
all. I've created an updated test case:

<?

$fmt = "%-12s %-20s %-20s %-10s %-5s\n";

function json_cmp($x, $y)
{
  global $fmt;
  $error = array("-", "FAIL");
  printf($fmt,
    var_export($x, true),
    str_replace("\n", "", var_export($y, true)),
    str_replace("\n", "", var_export(json_decode($x), true)),
    $error[json_last_error() > 0],
    $error[json_decode($x) !== $y]
  );
}

printf($fmt, "JSON", "Expected", "Actual", "JSON_ERROR", "PASS");
printf("----------------------------------------------------------------------
\n");

// works
json_cmp("true", true);

// fails - is actually true
json_cmp("tRue", NULL);

// fails - is actually NULL
json_cmp("true ", true);

// works
json_cmp("[true ] ", array(true));
json_cmp("[ true ] ", array(true));
json_cmp("[true] ", array(true));

// works, even though the non-array version fails
json_cmp("[tRue]", NULL);

json_cmp("0", 0);
json_cmp("1", 1);

json_cmp("false", false);

json_cmp("'foo'", NULL);

json_cmp('"foo"', "foo");

json_cmp('1.123', 1.123);
json_cmp('1.123 ', 1.123);
json_cmp(' 1.123', 1.123);

json_cmp('42', 42);
json_cmp('42 ', 42);
json_cmp(' 42', 42);

json_cmp(".123", 0.123);

?>


Which gives the following results:

JSON         Expected             Actual               JSON_ERROR PASS 
----------------------------------------------------------------------
'true'       true                 true                 -          -    
'tRue'       NULL                 true                 -          FAIL 
'true '      true                 NULL                 FAIL       FAIL 
'[true ] '   array (  0 => true,) array (  0 => true,) -          -    
'[ true ] '  array (  0 => true,) array (  0 => true,) -          -    
'[true] '    array (  0 => true,) array (  0 => true,) -          -    
'[tRue]'     NULL                 NULL                 FAIL       -    
'0'          0                    0                    -          -    
'1'          1                    1                    -          -    
'false'      false                false                -          -    
'\'foo\''    NULL                 NULL                 FAIL       -    
'"foo"'      'foo'                'foo'                -          -    
'1.123'      1.123                1.123                -          -    
'1.123 '     1.123                NULL                 FAIL       FAIL 
' 1.123'     1.123                1.123                -          -    
'42'         42                   42                   -          -    
'42 '        42                   NULL                 FAIL       FAIL 
' 42'        42                   42                   -          -    
'.123'       0.123                0.123                -          -


I see "FAIL" 4 times, so that seems like 4 bugs to me.
 [2013-09-09 16:24 UTC] ajf at ajf dot me
D:\Projects>php -r "var_dump(json_decode('tRUe'));"
bool(true)

Seems to still be broken, I'll try to fix it.
 [2013-09-09 17:05 UTC] cmbecker69 at gmx dot de
There is a RFC[1] to replace the current JSON implementation with
the jsonc PECL extension, so "fixing" that implementation may not 
be reasonable anyway.

[1] <https://wiki.php.net/rfc/free-json-parser>
 [2013-09-11 10:38 UTC] ajf at ajf dot me
Submitted pull request: https://github.com/php/php-src/pull/436
 [2013-10-16 12:27 UTC] ajf at ajf dot me
The pull request to fix the bug was split into two parts. The first is the backwards-compatible part (whitespace fix), which should go into 5.4 and 5.5:

https://github.com/php/php-src/pull/456

The second is to fix the non-backwards-compatible part (case), which should go into 5.6:

https://github.com/php/php-src/pull/457
 [2013-11-11 00:38 UTC] ajf at ajf dot me
My request to fix the whitespace portion of this bug was merged into the PHP-5.4 branch (and hence will be in the next minor 5.4 and 5.5 releases).

Now I simply need to try and get the case fix bit into 5.6 (it breaks BC so it can't be in 5.4 or 5.5, I bet): https://github.com/php/php-src/pull/457
 [2013-11-14 20:01 UTC] ajf at ajf dot me
The whitespace bit's fixed in 5.5.6, and will be in the next 5.4 release too.

Though it matters little since Debian, Ubuntu, Fedora and so-on do not use the built-in JSON ext.
 [2013-11-18 22:07 UTC] ajf@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: ajf
 [2013-11-18 22:07 UTC] ajf@php.net
The fix for this bug has been committed.

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.


 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Wed Apr 23 09:02:23 2014 UTC