Page MenuHomePhabricator

Incorrect JSON decoding in HHVM 3.6.5+dfsg1-1+wm1
Closed, DuplicatePublic

Description

Test code:

<?php

function test( $test ) {
	$res = json_decode( $test );
	$err = $res === null ? ' (' . json_last_error_msg() . ')' : '';
	echo "$test => " . var_export( json_decode( $test ), 1 ) . "$err\n";
}

echo PHP_VERSION . "\n";

echo "\nPassing tests:\n";
test( '"true"' );
test( 'true' );
test( '"false"' );
test( 'false' );
test( '"null"' );
test( 'null' );

echo "\nShould fail:\n";
test( 'trueOBVIOUSLYnot' );
test( 'falseOBVIOUSLYnot' );
test( 'nullOBVIOUSLYnot' );
test( 'true, {} nil; [] false' );

Result in HHVM 3.6.5+dfsg1-1+wm1:

$ php test-107132.php 
5.6.99-hhvm

Passing tests:
"true" => 'true'
true => true
"false" => 'false'
false => false
"null" => 'null'
null => NULL (No error)

Should fail:
trueOBVIOUSLYnot => true
falseOBVIOUSLYnot => false
nullOBVIOUSLYnot => NULL (No error)
true, {} nil; [] false => true

Result in 5.5.9+dfsg-1ubuntu4.11:

5.5.9-1ubuntu4.11

Passing tests:
"true" => 'true'
true => true
"false" => 'false'
false => false
"null" => 'null'
null => NULL (No error)

Should fail:
trueOBVIOUSLYnot => NULL (unexpected character)
falseOBVIOUSLYnot => NULL (unexpected character)
nullOBVIOUSLYnot => NULL (unexpected character)
true, {} nil; [] false => NULL (unexpected character)

Tim reports this is fixed in the newest upstream versions, although the linked diff seems too old. It looks like it actually depends on whether HHVM is compiled to use json-c over its internal "no evil"-licensed version.

According to the documentation for json-c's json_tokener_parse_ex(), HHVM should be performing an additional check that it isn't performing. Upstream bug report is https://github.com/facebook/hhvm/issues/5812.

Event Timeline

Danny_B raised the priority of this task from to High.
Danny_B updated the task description. (Show Details)
Danny_B added subscribers: Danny_B, Anomie, tstarling.

This is apparently due to a fixed bug in json_decode(). I can reproduce it on a random appserver with hhvm -md, but not locally with a more recent HHVM build. Looking through recent changes to ext_json.cpp, this seems like the most likely bugfix commit: https://github.com/facebook/hhvm/commit/04fd8890b2b9853a51f541f90170ead76da9174b

Anomie renamed this task from Incorrect JSON decoding in mw.text.jsonDecode() to Incorrect JSON decoding in HHVM 3.6.5+dfsg1-1+wm1.Jul 28 2015, 2:58 PM
Anomie updated the task description. (Show Details)
Anomie edited projects, added Upstream; removed Scribunto.
Krinkle moved this task from Defect to Blocked on the HHVM board.
Krinkle moved this task from Blocked to Defect on the HHVM board.