API: Raise an error when too many values are passed

Since 2008 the API has truncated with a warning when too many values are
passed to a multi-valued parameter. It's long past time to make this an
error.

Bug: T41936
Change-Id: I0f9efbdf9230373fa0c175a7fcacbca68225cf40

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index 470b9c3..4f30954 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -54,6 +54,8 @@ production.
     from normal parameters. All parameter definitions now include an "index"
     key to allow clients to maintain parameter ordering when merging normal and
     templated parameters.
+* It is now an error to submit too many values for a multi-valued parameter.
+  This has generated a warning since MediaWiki 1.14.
 
 === Action API internal changes in 1.32 ===
 * Added 'ApiParseMakeOutputPage' hook.
@@ -117,6 +119,8 @@ because of Phabricator reports.
   mediawiki.api.login, mediawiki.api.options, mediawiki.api.parse,
   mediawiki.api.upload, mediawiki.api.user, mediawiki.api.watch,
   mediawiki.api.messages, and mediawiki.api.rollback.
+* ApiBase::truncateArray() is deprecated. No replacement, as nothing is known
+  to use it.
 
 === Other changes in 1.32 ===
 * Soft hyphens (U+00AD) are now automatically removed from titles; these

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index c2483cb..b18bf58 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1502,10 +1502,10 @@ abstract class ApiBase extends ContextSource {
                        return $allowedValues;
                }
 
-               if ( self::truncateArray( $valuesList, $sizeLimit ) ) {
-                       $this->addDeprecation(
-                               [ 'apiwarn-toomanyvalues', $valueName, $sizeLimit ],
-                               "too-many-$valueName-for-{$this->getModulePath()}"
+               if ( count( $valuesList ) > $sizeLimit ) {
+                       $this->dieWithError(
+                               [ 'apierror-toomanyvalues', $valueName, $sizeLimit ],
+                               "too-many-$valueName"
                        );
                }
 
@@ -1739,22 +1739,6 @@ abstract class ApiBase extends ContextSource {
                WatchAction::doWatchOrUnwatch( $value, $titleObj, $this->getUser() );
        }
 
-       /**
-        * Truncate an array to a certain length.
-        * @param array &$arr Array to truncate
-        * @param int $limit Maximum length
-        * @return bool True if the array was truncated, false otherwise
-        */
-       public static function truncateArray( &$arr, $limit ) {
-               $modified = false;
-               while ( count( $arr ) > $limit ) {
-                       array_pop( $arr );
-                       $modified = true;
-               }
-
-               return $modified;
-       }
-
        /**
         * Gets the user for whom to get the watchlist
         *
@@ -3039,6 +3023,24 @@ abstract class ApiBase extends ContextSource {
                ] ];
        }
 
+       /**
+        * Truncate an array to a certain length.
+        * @deprecated since 1.32, no replacement
+        * @param array &$arr Array to truncate
+        * @param int $limit Maximum length
+        * @return bool True if the array was truncated, false otherwise
+        */
+       public static function truncateArray( &$arr, $limit ) {
+               wfDeprecated( __METHOD__, '1.32' );
+               $modified = false;
+               while ( count( $arr ) > $limit ) {
+                       array_pop( $arr );
+                       $modified = true;
+               }
+
+               return $modified;
+       }
+
        /**@}*/
 }
 

diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json
index 573d37c..68bf603 100644 (file)
--- a/includes/api/i18n/en.json
+++ b/includes/api/i18n/en.json
@@ -1826,6 +1826,7 @@
        "apierror-templateexpansion-notwikitext": "Template expansion is only supported for wikitext content. $1 uses content model $2.",
        "apierror-timeout": "The server did not respond within the expected time.",
        "apierror-toofewexpiries": "$1 expiry {{PLURAL:$1|timestamp was|timestamps were}} provided where $2 {{PLURAL:$2|was|were}} needed.",
+       "apierror-toomanyvalues": "Too many values supplied for parameter <var>$1</var>. The limit is $2.",
        "apierror-unknownaction": "The action specified, <kbd>$1</kbd>, is not recognized.",
        "apierror-unknownerror-editpage": "Unknown EditPage error: $1.",
        "apierror-unknownerror-nocode": "Unknown error.",
@@ -1874,7 +1875,6 @@
        "apiwarn-redirectsandrevids": "Redirect resolution cannot be used together with the <var>revids</var> parameter. Any redirects the <var>revids</var> point to have not been resolved.",
        "apiwarn-tokennotallowed": "Action \"$1\" is not allowed for the current user.",
        "apiwarn-tokens-origin": "Tokens may not be obtained when the same-origin policy is not applied.",
-       "apiwarn-toomanyvalues": "Too many values supplied for parameter <var>$1</var>. The limit is $2.",
        "apiwarn-truncatedresult": "This result was truncated because it would otherwise be larger than the limit of $1 bytes.",
        "apiwarn-unclearnowtimestamp": "Passing \"$2\" for timestamp parameter <var>$1</var> has been deprecated. If for some reason you need to explicitly specify the current time without calculating it client-side, use <kbd>now</kbd>.",
        "apiwarn-unrecognizedvalues": "Unrecognized {{PLURAL:$3|value|values}} for parameter <var>$1</var>: $2.",

diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json
index 086e74b..1ecb077 100644 (file)
--- a/includes/api/i18n/qqq.json
+++ b/includes/api/i18n/qqq.json
@@ -1714,6 +1714,7 @@
        "apierror-templateexpansion-notwikitext": "{{doc-apierror}}\n\nParameters:\n* $1 - Page title.\n* $2 - Content model.",
        "apierror-timeout": "{{doc-apierror}}\nAPI error message that can be used for client side localisation of API errors.",
        "apierror-toofewexpiries": "{{doc-apierror}}\n\nParameters:\n* $1 - Number provided.\n* $2 - Number needed.",
+       "apierror-toomanyvalues": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n* $2 - Maximum number of values allowed.",
        "apierror-unknownaction": "{{doc-apierror}}\n\nParameters:\n* $1 - Action provided.",
        "apierror-unknownerror-editpage": "{{doc-apierror}}\n\nParameters:\n* $1 - Error code (an integer).",
        "apierror-unknownerror-nocode": "{{doc-apierror}}",
@@ -1761,7 +1762,6 @@
        "apiwarn-redirectsandrevids": "{{doc-apierror}}",
        "apiwarn-tokennotallowed": "{{doc-apierror}}\n\nParameters:\n* $1 - Token type being requested, typically named after the action requiring the token.",
        "apiwarn-tokens-origin": "{{doc-apierror}}",
-       "apiwarn-toomanyvalues": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n* $2 - Maximum number of values allowed.",
        "apiwarn-truncatedresult": "{{doc-apierror}}\n\nParameters:\n* $1 - Size limit in bytes.",
        "apiwarn-unclearnowtimestamp": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n* $2 - Supplied value.",
        "apiwarn-unrecognizedvalues": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n* $2 - List of unknown values supplied.\n* $3 - Number of unknown values.",

diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php
index bb3de4d..4e3e3fc 100644 (file)
--- a/tests/phpunit/includes/api/ApiBaseTest.php
+++ b/tests/phpunit/includes/api/ApiBaseTest.php
@@ -363,8 +363,10 @@ class ApiBaseTest extends ApiTestCase {
                                        ApiBase::PARAM_ISMULTI => true,
                                        ApiBase::PARAM_ISMULTI_LIMIT1 => 2,
                                ],
-                               [ 'a', 'b' ],
-                               [ [ 'apiwarn-toomanyvalues', 'myParam', 2 ] ],
+                               ApiUsageException::newWithMessage(
+                                       null, [ 'apierror-toomanyvalues', 'myParam', 2 ], 'too-many-myParam'
+                               ),
+                               []
                        ],
                        'Multi-valued parameter with exceeded limits for non-bot' => [
                                'a|b|c',
@@ -373,8 +375,10 @@ class ApiBaseTest extends ApiTestCase {
                                        ApiBase::PARAM_ISMULTI_LIMIT1 => 2,
                                        ApiBase::PARAM_ISMULTI_LIMIT2 => 3,
                                ],
-                               [ 'a', 'b' ],
-                               [ [ 'apiwarn-toomanyvalues', 'myParam', 2 ] ],
+                               ApiUsageException::newWithMessage(
+                                       null, [ 'apierror-toomanyvalues', 'myParam', 2 ], 'too-many-myParam'
+                               ),
+                               []
                        ],
                        'Multi-valued parameter with non-exceeded limits for bot' => [
                                'a|b|c',