From 426df4cd70face4fb9596ddd0c11ef1b83ca9d13 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Fri, 1 Mar 2019 09:49:05 -0500
Subject: [PATCH] API: Handle Messages in errorArrayToStatus()

Two bugs here:
* If the error array contains an entry using a Message object instead of
  a string as the key, it'll blow up trying to do
  `self::$blockMsgMap[$error[0]]`.
* If the error array contains a Message object not wrapped in an array,
  it'll blow up trying to do `...(array)$error`.

Bug: T217382
Change-Id: I2a08e02bca0fb194416b3f2e6a1d6192d5c13cb2
---
 includes/api/ApiBase.php                   |  7 +++++--
 tests/phpunit/includes/api/ApiBaseTest.php | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 53c0a0b0bd..2ec4570069 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1805,13 +1805,16 @@ abstract class ApiBase extends ContextSource {
 
 		$status = Status::newGood();
 		foreach ( $errors as $error ) {
-			if ( is_array( $error ) && isset( self::$blockMsgMap[$error[0]] ) && $user->getBlock() ) {
+			if ( !is_array( $error ) ) {
+				$error = [ $error ];
+			}
+			if ( is_string( $error[0] ) && isset( self::$blockMsgMap[$error[0]] ) && $user->getBlock() ) {
 				list( $msg, $code ) = self::$blockMsgMap[$error[0]];
 				$status->fatal( ApiMessage::create( $msg, $code,
 					[ 'blockinfo' => ApiQueryUserInfo::getBlockInfo( $user->getBlock() ) ]
 				) );
 			} else {
-				$status->fatal( ...(array)$error );
+				$status->fatal( ...$error );
 			}
 		}
 		return $status;
diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php
index 4600551993..0dc64df87f 100644
--- a/tests/phpunit/includes/api/ApiBaseTest.php
+++ b/tests/phpunit/includes/api/ApiBaseTest.php
@@ -1297,6 +1297,8 @@ class ApiBaseTest extends ApiTestCase {
 	public function testErrorArrayToStatus() {
 		$mock = new MockApi();
 
+		$msg = new Message( 'mainpage' );
+
 		// Sanity check empty array
 		$expect = Status::newGood();
 		$this->assertEquals( $expect, $mock->errorArrayToStatus( [] ) );
@@ -1307,12 +1309,16 @@ class ApiBaseTest extends ApiTestCase {
 		$expect->fatal( 'autoblockedtext' );
 		$expect->fatal( 'systemblockedtext' );
 		$expect->fatal( 'mainpage' );
+		$expect->fatal( $msg );
+		$expect->fatal( $msg, 'foobar' );
 		$expect->fatal( 'parentheses', 'foobar' );
 		$this->assertEquals( $expect, $mock->errorArrayToStatus( [
 			[ 'blockedtext' ],
 			[ 'autoblockedtext' ],
 			[ 'systemblockedtext' ],
 			'mainpage',
+			$msg,
+			[ $msg, 'foobar' ],
 			[ 'parentheses', 'foobar' ],
 		] ) );
 
@@ -1333,12 +1339,16 @@ class ApiBaseTest extends ApiTestCase {
 		$expect->fatal( ApiMessage::create( 'apierror-autoblocked', 'autoblocked', $blockinfo ) );
 		$expect->fatal( ApiMessage::create( 'apierror-systemblocked', 'blocked', $blockinfo ) );
 		$expect->fatal( 'mainpage' );
+		$expect->fatal( $msg );
+		$expect->fatal( $msg, 'foobar' );
 		$expect->fatal( 'parentheses', 'foobar' );
 		$this->assertEquals( $expect, $mock->errorArrayToStatus( [
 			[ 'blockedtext' ],
 			[ 'autoblockedtext' ],
 			[ 'systemblockedtext' ],
 			'mainpage',
+			$msg,
+			[ $msg, 'foobar' ],
 			[ 'parentheses', 'foobar' ],
 		], $user ) );
 	}
@@ -1346,6 +1356,8 @@ class ApiBaseTest extends ApiTestCase {
 	public function testAddBlockInfoToStatus() {
 		$mock = new MockApi();
 
+		$msg = new Message( 'mainpage' );
+
 		// Sanity check empty array
 		$expect = Status::newGood();
 		$test = Status::newGood();
@@ -1358,6 +1370,8 @@ class ApiBaseTest extends ApiTestCase {
 		$expect->fatal( 'autoblockedtext' );
 		$expect->fatal( 'systemblockedtext' );
 		$expect->fatal( 'mainpage' );
+		$expect->fatal( $msg );
+		$expect->fatal( $msg, 'foobar' );
 		$expect->fatal( 'parentheses', 'foobar' );
 		$test = clone $expect;
 		$mock->addBlockInfoToStatus( $test );
@@ -1380,12 +1394,16 @@ class ApiBaseTest extends ApiTestCase {
 		$expect->fatal( ApiMessage::create( 'apierror-autoblocked', 'autoblocked', $blockinfo ) );
 		$expect->fatal( ApiMessage::create( 'apierror-systemblocked', 'blocked', $blockinfo ) );
 		$expect->fatal( 'mainpage' );
+		$expect->fatal( $msg );
+		$expect->fatal( $msg, 'foobar' );
 		$expect->fatal( 'parentheses', 'foobar' );
 		$test = Status::newGood();
 		$test->fatal( 'blockedtext' );
 		$test->fatal( 'autoblockedtext' );
 		$test->fatal( 'systemblockedtext' );
 		$test->fatal( 'mainpage' );
+		$test->fatal( $msg );
+		$test->fatal( $msg, 'foobar' );
 		$test->fatal( 'parentheses', 'foobar' );
 		$mock->addBlockInfoToStatus( $test, $user );
 		$this->assertEquals( $expect, $test );
-- 
2.20.1