From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Tue, 13 Nov 2018 02:44:02 +0000 (+0000)
Subject: Merge "Comments, tests, and tweaks for JSON decoding quirks"
X-Git-Tag: 1.34.0-rc.0~3528
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/pie.php?a=commitdiff_plain;h=d7fba6c62decdcbd108cdd26b49a9075377e6368;hp=d65e96b7638279f7884dec827b75096bd8f2683a;p=lhc%2Fweb%2Fwiklou.git

Merge "Comments, tests, and tweaks for JSON decoding quirks"
---

diff --git a/includes/CommentStore.php b/includes/CommentStore.php
index 1be79510cb..7a2726f291 100644
--- a/includes/CommentStore.php
+++ b/includes/CommentStore.php
@@ -349,14 +349,13 @@ class CommentStore {
 
 		$msg = null;
 		if ( $data !== null ) {
-			$data = FormatJson::decode( $data );
-			if ( !is_object( $data ) ) {
+			$data = FormatJson::decode( $data, true );
+			if ( !is_array( $data ) ) {
 				// @codeCoverageIgnoreStart
 				wfLogWarning( "Invalid JSON object in comment: $data" );
 				$data = null;
 				// @codeCoverageIgnoreEnd
 			} else {
-				$data = (array)$data;
 				if ( isset( $data['_message'] ) ) {
 					$msg = self::decodeMessage( $data['_message'] )
 						->setInterfaceMessageFlag( true );
diff --git a/includes/json/FormatJson.php b/includes/json/FormatJson.php
index fbcb3bd3b8..9ec3d96002 100644
--- a/includes/json/FormatJson.php
+++ b/includes/json/FormatJson.php
@@ -153,6 +153,16 @@ class FormatJson {
 	 * which returns more comprehensive result in case of an error, and has
 	 * more parsing options.
 	 *
+	 * In PHP versions before 7.1, decoding a JSON string containing an empty key
+	 * without passing $assoc as true results in a return object with a property
+	 * named "_empty_" (because true empty properties were not supported pre-PHP-7.1).
+	 * Instead, consider passing $assoc as true to return an associative array.
+	 *
+	 * But be aware that in all supported PHP versions, decoding an empty JSON object
+	 * with $assoc = true returns an array, not an object, breaking round-trip consistency.
+	 *
+	 * See https://phabricator.wikimedia.org/T206411 for more details on these quirks.
+	 *
 	 * @param string $value The JSON string being decoded
 	 * @param bool $assoc When true, returned objects will be converted into associative arrays.
 	 *
diff --git a/tests/phpunit/includes/json/FormatJsonTest.php b/tests/phpunit/includes/json/FormatJsonTest.php
index a4ab879ffb..2760cb9f22 100644
--- a/tests/phpunit/includes/json/FormatJsonTest.php
+++ b/tests/phpunit/includes/json/FormatJsonTest.php
@@ -172,7 +172,7 @@ class FormatJsonTest extends MediaWikiTestCase {
 	/**
 	 * Test data for testParseTryFixing.
 	 *
-	 * Some PHP interpreters use json-c rather than the JSON.org cannonical
+	 * Some PHP interpreters use json-c rather than the JSON.org canonical
 	 * parser to avoid being encumbered by the "shall be used for Good, not
 	 * Evil" clause of the JSON.org parser's license. By default, json-c
 	 * parses in a non-strict mode which allows trailing commas for array and
@@ -372,4 +372,56 @@ class FormatJsonTest extends MediaWikiTestCase {
 
 		return $cases;
 	}
+
+	public function provideEmptyJsonKeyStrings() {
+		return [
+			[
+				'{"":"foo"}',
+				'{"":"foo"}',
+				''
+			],
+			[
+				'{"_empty_":"foo"}',
+				'{"_empty_":"foo"}',
+				'_empty_' ],
+			[
+				'{"\u005F\u0065\u006D\u0070\u0074\u0079\u005F":"foo"}',
+				'{"_empty_":"foo"}',
+				'_empty_'
+			],
+			[
+				'{"_empty_":"bar","":"foo"}',
+				'{"_empty_":"bar","":"foo"}',
+				''
+			],
+			[
+				'{"":"bar","_empty_":"foo"}',
+				'{"":"bar","_empty_":"foo"}',
+				'_empty_'
+			]
+		];
+	}
+
+	/**
+	 * @covers FormatJson::encode
+	 * @covers FormatJson::decode
+	 * @dataProvider provideEmptyJsonKeyStrings
+	 * @param string $json
+	 *
+	 * Decoding behavior with empty keys can be surprising.
+	 * See https://phabricator.wikimedia.org/T206411
+	 */
+	public function testEmptyJsonKeyArray( $json, $expect, $php71Name ) {
+		// Decoding to array is consistent across supported PHP versions
+		$this->assertSame( $expect, FormatJson::encode(
+			FormatJson::decode( $json, true ) ) );
+
+		// Decoding to object differs between supported PHP versions
+		$obj = FormatJson::decode( $json );
+		if ( version_compare( PHP_VERSION, '7.1', '<' ) ) {
+			$this->assertEquals( 'foo', $obj->_empty_ );
+		} else {
+			$this->assertEquals( 'foo', $obj->{$php71Name} );
+		}
+	}
 }