Allow passing detailed permission errors data to API

Using the new system introduced in
1c57794e371d74e1d88748de567a1c3358c3ad2e (see T47843).

This change allows Title::getUserPermissionsErrors() to include
MessageSpecifiers instead of string message keys in its return value.
This doesn't seem to have any bad effects, and should work seamlessly as
long as callers aren't trying to do anything stupid and just pass the
value to PermissionsError or OutputPage::showPermissionsErrorPage()
or wfMessage() or some such.

If the callers *are* trying something stupid, nothing worse than
duplicated or otherwise less-than-perfect error messages (in code
which tries to handle some message keys specially) should happen.
(I fixed wfMergeErrorArrays(), but who knows what else lurks in all
this code.) Any problems should only affect new-style errors using
MessageSpecifier, though.

Since MessageSpecifiers tend to be stringable, we probably won't get
fatals, but might get incorrect checks. Should we try to log this
happening somehow?

Goes with I42a0c5b0ea7e61088dd609b764dd7d1396c60cd5 in TitleBlacklist.

Bug: T115258
Change-Id: I1334ba21a2862973a9d8ff5be2c9bec06a82698b

diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index 4275921..35fa887 100644 (file)
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -64,6 +64,9 @@ production.
   setting in the relevant section of $wgLBFactoryConf.
 * User::newSystemUser() may be used to simplify the creation of passwordless
   "system" users for logged actions from scripts and extensions.
+* Extensions can now return detailed error information via the API when
+  preventing user actions using 'getUserPermissionsErrors' and similar hooks
+  by using ApiMessage instances instead of strings for the $result value.
 
 ==== External libraries ====
 

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 427f35e..017416f 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -1521,7 +1521,9 @@ $lang: Language that will be used to render the timestamp
 'getUserPermissionsErrors': Add a permissions error when permissions errors are
 checked for. Use instead of userCan for most cases. Return false if the user
 can't do it, and populate $result with the reason in the form of
-array( messagename, param1, param2, ... ). For consistency, error messages
+array( messagename, param1, param2, ... ) or a MessageSpecifier instance (you
+might want to use ApiMessage to provide machine-readable details for the API).
+For consistency, error messages
 should be plain text with no special coloring, bolding, etc. to show that
 they're errors; presenting them properly to the user as errors is done by the
 caller.
@@ -1534,7 +1536,9 @@ $result: User permissions error to add. If none, return true.
 called only if expensive checks are enabled. Add a permissions error when
 permissions errors are checked for. Return false if the user can't do it, and
 populate $result with the reason in the form of array( messagename, param1,
-param2, ... ). For consistency, error messages should be plain text with no
+param2, ... ) or a MessageSpecifier instance (you might want to use ApiMessage
+to provide machine-readable details for the API). For consistency, error
+messages should be plain text with no
 special coloring, bolding, etc. to show that they're errors; presenting them
 properly to the user as errors is done by the caller.
 $title: Title object being checked against

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 791bf33..7c57207 100644 (file)
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -308,10 +308,15 @@ function wfMergeErrorArrays( /*...*/ ) {
        $out = array();
        foreach ( $args as $errors ) {
                foreach ( $errors as $params ) {
+                       $originalParams = $params;
+                       if ( $params[0] instanceof MessageSpecifier ) {
+                               $msg = $params[0];
+                               $params = array_merge( array( $msg->getKey() ), $msg->getParams() );
+                       }
                        # @todo FIXME: Sometimes get nested arrays for $params,
                        # which leads to E_NOTICEs
                        $spec = implode( "\t", $params );
-                       $out[$spec] = $params;
+                       $out[$spec] = $originalParams;
                }
        }
        return array_values( $out );

diff --git a/includes/Title.php b/includes/Title.php
index 9ada4f3..13e7ffb 100644 (file)
--- a/includes/Title.php
+++ b/includes/Title.php
@@ -1944,7 +1944,7 @@ class Title {
         *   - secure : does cheap and expensive checks, using the master as needed
         * @param array $ignoreErrors Array of Strings Set this to a list of message keys
         *   whose corresponding errors may be ignored.
-        * @return array Array of arguments to wfMessage to explain permissions problems.
+        * @return array Array of arrays of the arguments to wfMessage to explain permissions problems.
         */
        public function getUserPermissionsErrors(
                $action, $user, $rigor = 'secure', $ignoreErrors = array()
@@ -1953,9 +1953,12 @@ class Title {
 
                // Remove the errors being ignored.
                foreach ( $errors as $index => $error ) {
-                       $error_key = is_array( $error ) ? $error[0] : $error;
+                       $errKey = is_array( $error ) ? $error[0] : $error;
 
-                       if ( in_array( $error_key, $ignoreErrors ) ) {
+                       if ( in_array( $errKey, $ignoreErrors ) ) {
+                               unset( $errors[$index] );
+                       }
+                       if ( $errKey instanceof MessageSpecifier && in_array( $errKey->getKey(), $ignoreErrors ) ) {
                                unset( $errors[$index] );
                        }
                }
@@ -2054,6 +2057,9 @@ class Title {
                } elseif ( $result !== '' && is_string( $result ) ) {
                        // A string representing a message-id
                        $errors[] = array( $result );
+               } elseif ( $result instanceof MessageSpecifier ) {
+                       // A message specifier representing an error
+                       $errors[] = array( $result );
                } elseif ( $result === false ) {
                        // a generic "We don't want them to do that"
                        $errors[] = array( 'badaccess-group0' );

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 1465543..8af08a5 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1375,10 +1375,11 @@ abstract class ApiBase extends ContextSource {
         *
         * @since 1.23
         * @param Status $status
+        * @param array|null &$extraData Set if extra data from IApiMessage is available (since 1.27)
         * @return array Array of code and error string
         * @throws MWException
         */
-       public function getErrorFromStatus( $status ) {
+       public function getErrorFromStatus( $status, &$extraData = null ) {
                if ( $status->isGood() ) {
                        throw new MWException( 'Successful status passed to ApiBase::dieStatus' );
                }
@@ -1397,7 +1398,12 @@ abstract class ApiBase extends ContextSource {
                // error messages.
                if ( $errors[0] instanceof Message ) {
                        $msg = $errors[0];
-                       $code = $msg->getKey();
+                       if ( $msg instanceof IApiMessage ) {
+                               $extraData = $msg->getApiData();
+                               $code = $msg->getApiCode();
+                       } else {
+                               $code = $msg->getKey();
+                       }
                } else {
                        $code = array_shift( $errors[0] );
                        $msg = wfMessage( $code, $errors[0] );
@@ -1418,8 +1424,9 @@ abstract class ApiBase extends ContextSource {
         * @throws UsageException always
         */
        public function dieStatus( $status ) {
-               list( $code, $msg ) = $this->getErrorFromStatus( $status );
-               $this->dieUsage( $msg, $code );
+               $extraData = null;
+               list( $code, $msg ) = $this->getErrorFromStatus( $status, $extraData );
+               $this->dieUsage( $msg, $code, 0, $extraData );
        }
 
        // @codingStandardsIgnoreStart Allow long lines. Cannot split these.
@@ -1952,7 +1959,8 @@ abstract class ApiBase extends ContextSource {
                        $error = array( $error );
                }
                $parsed = $this->parseMsg( $error );
-               $this->dieUsage( $parsed['info'], $parsed['code'] );
+               $extraData = isset( $parsed['data'] ) ? $parsed['data'] : null;
+               $this->dieUsage( $parsed['info'], $parsed['code'], 0, $extraData );
        }
 
        /**
@@ -2005,6 +2013,14 @@ abstract class ApiBase extends ContextSource {
                        $key = array_shift( $error );
                }
 
+               if ( $key instanceof IApiMessage ) {
+                       return array(
+                               'code' => $key->getApiCode(),
+                               'info' => $key->inLanguage( 'en' )->useDatabase( false )->text(),
+                               'data' => $key->getApiData()
+                       );
+               }
+
                if ( isset( self::$messageMap[$key] ) ) {
                        return array(
                                'code' => wfMsgReplaceArgs( self::$messageMap[$key]['code'], $error ),

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 5e13e98..b456b0d 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -363,6 +363,10 @@ class ApiUpload extends ApiBase {
                $data['invalidparameter'] = $parameter;
 
                $parsed = $this->parseMsg( $error );
+               if ( isset( $parsed['data'] ) ) {
+                       $data = array_merge( $data, $parsed['data'] );
+               }
+
                $this->dieUsage( $parsed['info'], $parsed['code'], 0, $data );
        }