ApiBase: Always validate that 'limit' is numeric

Bug: T231582
Change-Id: I956d4d623bfeace1b542039283e04a970fd40121

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 8b6a3e5..9680601 100644 (file)
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -1308,8 +1308,15 @@ abstract class ApiBase extends ContextSource {
                                                }
                                                break;
                                        case 'limit':
+                                               // Must be a number or 'max'
+                                               if ( $value !== 'max' ) {
+                                                       $value = (int)$value;
+                                               }
+                                               if ( $multi ) {
+                                                       self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
+                                               }
                                                if ( !$parseLimit ) {
-                                                       // Don't do any validation whatsoever
+                                                       // Don't do min/max validation and don't parse 'max'
                                                        break;
                                                }
                                                if ( !isset( $paramSettings[self::PARAM_MAX] )
@@ -1320,21 +1327,16 @@ abstract class ApiBase extends ContextSource {
                                                                "MAX1 or MAX2 are not defined for the limit $encParamName"
                                                        );
                                                }
-                                               if ( $multi ) {
-                                                       self::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
-                                               }
-                                               $min = $paramSettings[self::PARAM_MIN] ?? 0;
-                                               if ( $value == 'max' ) {
+                                               if ( $value === 'max' ) {
                                                        $value = $this->getMain()->canApiHighLimits()
                                                                ? $paramSettings[self::PARAM_MAX2]
                                                                : $paramSettings[self::PARAM_MAX];
                                                        $this->getResult()->addParsedLimit( $this->getModuleName(), $value );
                                                } else {
-                                                       $value = (int)$value;
                                                        $this->validateLimit(
                                                                $paramName,
                                                                $value,
-                                                               $min,
+                                                               $paramSettings[self::PARAM_MIN] ?? 0,
                                                                $paramSettings[self::PARAM_MAX],
                                                                $paramSettings[self::PARAM_MAX2]
                                                        );

diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php
index 6a44ff3..c49d669 100644 (file)
--- a/tests/phpunit/includes/api/ApiBaseTest.php
+++ b/tests/phpunit/includes/api/ApiBaseTest.php
@@ -889,10 +889,24 @@ class ApiBaseTest extends ApiTestCase {
                                [],
                                [ 'internalmode' => false ],
                        ],
-                       'Limit with parseLimits false' => [
+                       'Limit with parseLimits false (numeric)' => [
                                '100',
                                [ ApiBase::PARAM_TYPE => 'limit' ],
-                               '100',
+                               100,
+                               [],
+                               [ 'parseLimits' => false ],
+                       ],
+                       'Limit with parseLimits false (max)' => [
+                               'max',
+                               [ ApiBase::PARAM_TYPE => 'limit' ],
+                               'max',
+                               [],
+                               [ 'parseLimits' => false ],
+                       ],
+                       'Limit with parseLimits false (invalid)' => [
+                               'kitten',
+                               [ ApiBase::PARAM_TYPE => 'limit' ],
+                               0,
                                [],
                                [ 'parseLimits' => false ],
                        ],
@@ -901,7 +915,6 @@ class ApiBaseTest extends ApiTestCase {
                                [
                                        ApiBase::PARAM_TYPE => 'limit',
                                        ApiBase::PARAM_MAX2 => 10,
-                                       ApiBase::PARAM_ISMULTI => true,
                                ],
                                new MWException(
                                        'Internal error in ApiBase::getParameterFromSettings: ' .
@@ -913,7 +926,6 @@ class ApiBaseTest extends ApiTestCase {
                                [
                                        ApiBase::PARAM_TYPE => 'limit',
                                        ApiBase::PARAM_MAX => 10,
-                                       ApiBase::PARAM_ISMULTI => true,
                                ],
                                new MWException(
                                        'Internal error in ApiBase::getParameterFromSettings: ' .