From: Brad Jorsch Date: Fri, 18 May 2018 15:22:44 +0000 (+0200) Subject: API: Raise an error when too many values are passed X-Git-Tag: 1.34.0-rc.0~5348^2 X-Git-Url: http://git.cyclocoop.org/wiki/Target_page?a=commitdiff_plain;h=a1cec329538fe787e9bad01a250ff57fee9144f6;p=lhc%2Fweb%2Fwiklou.git 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 470b9c3b57..4f309540bb 100644 --- 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 c2483cb73c..b18bf58dcc 100644 --- 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 573d37c664..68bf6037a0 100644 --- 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 $1. The limit is $2.", "apierror-unknownaction": "The action specified, $1, 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 revids parameter. Any redirects the revids 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 $1. 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 $1 has been deprecated. If for some reason you need to explicitly specify the current time without calculating it client-side, use now.", "apiwarn-unrecognizedvalues": "Unrecognized {{PLURAL:$3|value|values}} for parameter $1: $2.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 086e74bf2d..1ecb077450 100644 --- 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 bb3de4d116..4e3e3fc962 100644 --- 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',