Page MenuHomePhabricator

JsonContent incorrectly reports invalid JSON is valid, only on HHVM
Closed, DeclinedPublic

Description

I've been debugging a test that only fails in CI and found that it's due to an HHVM glitch. JsonContent calls FormatJson::parse, which calls the native json_decode and checks for errors. Here's my test script and output on different environments,

$badJson = '"foo": b }';

$decoded = json_decode( $badJson, true );
$error = json_last_error();

print( var_export( $decoded, true ) . "\n" );
print( var_export( $error , true ) . "\n" );

On Zend PHP 7.1.16, the output is:

NULL
4

With HHVM 3.18.6-dev, I get:

'foo'
0

Note that both evaluation engines correctly throw errors for a test string 'foo: b }', so I'm assuming the edge case has to do with a valid JSON item coming at the beginning of the string, and HHVM ignoring the "leftovers".

Event Timeline

There are clues in T128029, T74778, T128029. I've worked around for us, and hopefully the json-schema check gives us strict enough validation that we won't be allowing JSON that fails when app nodes are using Zend PHP.

Gross, another edge case is the empty string, both parsers return NULL but HHVM returns no error code so FormatJson::parse emits Status::newGood( null ).

Krinkle added a subscriber: Krinkle.

Declining per T192166.

Change 547804 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/Jade@master] Restore commented-out test now that HHVM isn't supported

https://gerrit.wikimedia.org/r/547804

Change 547804 merged by jenkins-bot:
[mediawiki/extensions/Jade@master] Restore commented-out test now that HHVM isn't supported

https://gerrit.wikimedia.org/r/547804