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

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 53c0a0b..2ec4570 100644 (file)
--- 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 4600551..0dc64df 100644 (file)
--- 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 );