From: C. Scott Ananian <cscott@cscott.net>
Date: Tue, 27 Nov 2018 22:47:56 +0000 (-0500)
Subject: Don't silently fail if API result fails to encode
X-Git-Tag: 1.34.0-rc.0~3327^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=afd3dbaa95f1557423baf6bc13a64ba7e59dfaca

Don't silently fail if API result fails to encode

Ensure that errors encoding API results produce actionable log entries.

Part of the follow-up to T210550.

Change-Id: I6f311451e3b07b540f14352ce25af9d74a053d19
---

diff --git a/includes/api/ApiFormatJson.php b/includes/api/ApiFormatJson.php
index 9dcde8f32b..8cb22dd1ba 100644
--- a/includes/api/ApiFormatJson.php
+++ b/includes/api/ApiFormatJson.php
@@ -88,6 +88,13 @@ class ApiFormatJson extends ApiFormatBase {
 		}
 		$data = $this->getResult()->getResultData( null, $transform );
 		$json = FormatJson::encode( $data, $this->getIsHtml(), $opt );
+		if ( $json === false ) {
+			// This should never happen, but it's a bug which could crop up
+			// if you use ApiResult::NO_VALIDATE for instance.
+			// @codeCoverageIgnoreStart
+			$this->dieDebug( __METHOD__, 'Unable to encode API result as JSON' );
+			// @codeCoverageIgnoreEnd
+		}
 
 		// T68776: OutputHandler::mangleFlashPolicy() avoids a nasty bug in
 		// Flash, but what it does isn't friendly for the API, so we need to
diff --git a/tests/phpunit/includes/api/format/ApiFormatJsonTest.php b/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
index 2408518356..4909e3cb07 100644
--- a/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
+++ b/tests/phpunit/includes/api/format/ApiFormatJsonTest.php
@@ -128,6 +128,37 @@ class ApiFormatJsonTest extends ApiFormatTestBase {
 
 				// Cross-domain mangling
 				[ [ '< Cross-Domain-Policy >' ], '["\u003C Cross-Domain-Policy >"]' ],
+
+				// Invalid UTF-8: bytes 192, 193, and 245-255 are off-limits
+				[
+					[ 'foo' => "\xFF" ],
+					"{\"foo\":\"\u{FFFD}\"}", // Mangled when validated (T210548)
+				],
+				[
+					[ 'foo' => "\xFF" ],
+					new MWException(
+						'Internal error in ApiFormatJson::execute: ' .
+						'Unable to encode API result as JSON'
+					),
+					[],
+					[ 'flags' => ApiResult::NO_VALIDATE ],
+				],
+				// NaN is also not allowed
+				[
+					[ 'foo' => NAN ],
+					new InvalidArgumentException(
+						'Cannot add non-finite floats to ApiResult'
+					),
+				],
+				[
+					[ 'foo' => NAN ],
+					new MWException(
+						'Internal error in ApiFormatJson::execute: ' .
+						'Unable to encode API result as JSON'
+					),
+					[],
+					[ 'flags' => ApiResult::NO_VALIDATE ],
+				],
 			] )
 			// @todo Test rawfm
 		);
diff --git a/tests/phpunit/includes/api/format/ApiFormatTestBase.php b/tests/phpunit/includes/api/format/ApiFormatTestBase.php
index ca20cb63f2..27ec73a94c 100644
--- a/tests/phpunit/includes/api/format/ApiFormatTestBase.php
+++ b/tests/phpunit/includes/api/format/ApiFormatTestBase.php
@@ -38,6 +38,7 @@ abstract class ApiFormatTestBase extends MediaWikiTestCase {
 			$options = [ 'class' => $options ];
 		}
 		$printerName = $options['name'] ?? $this->printerName;
+		$flags = $options['flags'] ?? 0;
 
 		$context = new RequestContext;
 		$context->setRequest( new FauxRequest( $params, true ) );
@@ -49,7 +50,7 @@ abstract class ApiFormatTestBase extends MediaWikiTestCase {
 		$result = $main->getResult();
 		$result->addArrayType( null, 'default' );
 		foreach ( $data as $k => $v ) {
-			$result->addValue( null, $k, $v );
+			$result->addValue( null, $k, $v, $flags );
 		}
 
 		$ret = [];