From d3da5e08d35f67c967bd2a29ad2983685b66f0d3 Mon Sep 17 00:00:00 2001 From: Aryeh Gregor Date: Tue, 20 Mar 2018 19:34:03 +0200 Subject: [PATCH] Improve test coverage for ApiBase.php One bug fixed: a timestamp of '00' or similar would get interpreted as 'now' by mistake instead of Unix timestamp 0, without throwing the warning for using 0 instead of 'now'. This is because it called wfTimestamp() once to parse the input date, got a Unix timestamp of 0 back, and then tried passing that 0 back to wfTimestamp again to reformat as a wiki date, but it got reinterpreted as 'now'. Also fixed parameters with type "user" to validate usernames more correctly. This might be risky, though, if I missed any valid usernames, or if API clients were for some reason relying on passing in invalid usernames. If we don't actually want to do this, we should add a comment explaining why we're allowing any title without a fragment rather than validating properly. Still lots more work to do here. Change-Id: I56b4290263df8698efdbddda71a7eabd9e303abc --- includes/WebRequest.php | 2 +- includes/api/ApiBase.php | 62 +- includes/api/ApiMain.php | 4 +- includes/api/i18n/en.json | 1 - includes/api/i18n/qqq.json | 1 - tests/phpunit/includes/api/ApiBaseTest.php | 1099 +++++++++++++++++++- 6 files changed, 1116 insertions(+), 53 deletions(-) diff --git a/includes/WebRequest.php b/includes/WebRequest.php index 6f0307da8d..fa8f84d697 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -432,7 +432,7 @@ class WebRequest { * selected by a drop-down menu). For freeform input, see getText(). * * @param string $name - * @param string $default Optional default (or null) + * @param string|null $default Optional default (or null) * @return string|null */ public function getVal( $name, $default = null ) { diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index 22202c0c11..5da56537a7 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -1129,8 +1129,8 @@ abstract class ApiBase extends ContextSource { ) { $type = array_merge( $type, $paramSettings[self::PARAM_EXTRA_NAMESPACES] ); } - // By default, namespace parameters allow ALL_DEFAULT_STRING to be used to specify - // all namespaces. + // Namespace parameters allow ALL_DEFAULT_STRING to be used to + // specify all namespaces irrespective of PARAM_ALL. $allowAll = true; } if ( isset( $value ) && $type == 'submodule' ) { @@ -1436,22 +1436,15 @@ abstract class ApiBase extends ContextSource { return $value; } - if ( is_array( $allowedValues ) ) { - $values = array_map( function ( $v ) { - return '' . wfEscapeWikiText( $v ) . ''; - }, $allowedValues ); - $this->dieWithError( [ - 'apierror-multival-only-one-of', - $valueName, - Message::listParam( $values ), - count( $values ), - ], "multival_$valueName" ); - } else { - $this->dieWithError( [ - 'apierror-multival-only-one', - $valueName, - ], "multival_$valueName" ); - } + $values = array_map( function ( $v ) { + return '' . wfEscapeWikiText( $v ) . ''; + }, $allowedValues ); + $this->dieWithError( [ + 'apierror-multival-only-one-of', + $valueName, + Message::listParam( $values ), + count( $values ), + ], "multival_$valueName" ); } if ( is_array( $allowedValues ) ) { @@ -1537,7 +1530,7 @@ abstract class ApiBase extends ContextSource { } /** - * Validate and normalize of parameters of type 'timestamp' + * Validate and normalize parameters of type 'timestamp' * @param string $value Parameter value * @param string $encParamName Parameter name * @return string Validated and normalized parameter @@ -1559,15 +1552,15 @@ abstract class ApiBase extends ContextSource { return wfTimestamp( TS_MW ); } - $unixTimestamp = wfTimestamp( TS_UNIX, $value ); - if ( $unixTimestamp === false ) { + $timestamp = wfTimestamp( TS_MW, $value ); + if ( $timestamp === false ) { $this->dieWithError( [ 'apierror-badtimestamp', $encParamName, wfEscapeWikiText( $value ) ], "badtimestamp_{$encParamName}" ); } - return wfTimestamp( TS_MW, $unixTimestamp ); + return $timestamp; } /** @@ -1609,7 +1602,7 @@ abstract class ApiBase extends ContextSource { } /** - * Validate and normalize of parameters of type 'user' + * Validate and normalize parameters of type 'user' * @param string $value Parameter value * @param string $encParamName Parameter name * @return string Validated and normalized parameter @@ -1619,15 +1612,32 @@ abstract class ApiBase extends ContextSource { return $value; } - $title = Title::makeTitleSafe( NS_USER, $value ); - if ( $title === null || $title->hasFragment() ) { + $titleObj = Title::makeTitleSafe( NS_USER, $value ); + + if ( $titleObj ) { + $value = $titleObj->getText(); + } + + if ( + !User::isValidUserName( $value ) && + // We allow ranges as well, for blocks. + !IP::isIPAddress( $value ) && + // See comment for User::isIP. We don't just call that function + // here because it also returns true for things like + // 300.300.300.300 that are neither valid usernames nor valid IP + // addresses. + !preg_match( + '/^' . RE_IP_BYTE . '\.' . RE_IP_BYTE . '\.' . RE_IP_BYTE . '\.xxx$/', + $value + ) + ) { $this->dieWithError( [ 'apierror-baduser', $encParamName, wfEscapeWikiText( $value ) ], "baduser_{$encParamName}" ); } - return $title->getText(); + return $value; } /**@}*/ diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index a7e3c1bd54..01e889dbf1 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -1720,8 +1720,8 @@ class ApiMain extends ApiBase { /** * Get a request value, and register the fact that it was used, for logging. * @param string $name - * @param mixed $default - * @return mixed + * @param string|null $default + * @return string|null */ public function getVal( $name, $default = null ) { $this->mParamsUsed[$name] = true; diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index 756852650d..b0cbc8dd2b 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1736,7 +1736,6 @@ "apierror-missingtitle-byname": "The page $1 doesn't exist.", "apierror-moduledisabled": "The $1 module has been disabled.", "apierror-multival-only-one-of": "{{PLURAL:$3|Only|Only one of}} $2 is allowed for parameter $1.", - "apierror-multival-only-one": "Only one value is allowed for parameter $1.", "apierror-multpages": "$1 may only be used with a single page.", "apierror-mustbeloggedin-changeauth": "You must be logged in to change authentication data.", "apierror-mustbeloggedin-generic": "You must be logged in.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index fc0de4e333..122fe89940 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -1625,7 +1625,6 @@ "apierror-missingtitle-byname": "{{doc-apierror}}", "apierror-moduledisabled": "{{doc-apierror}}\n\nParameters:\n* $1 - Name of the module.", "apierror-multival-only-one-of": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n* $2 - Possible values for the parameter.\n* $3 - Number of values.", - "apierror-multival-only-one": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.", "apierror-multpages": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name", "apierror-mustbeloggedin-changeauth": "{{doc-apierror}}", "apierror-mustbeloggedin-generic": "{{doc-apierror}}", diff --git a/tests/phpunit/includes/api/ApiBaseTest.php b/tests/phpunit/includes/api/ApiBaseTest.php index 3f6cac9a00..dbedfc5ac8 100644 --- a/tests/phpunit/includes/api/ApiBaseTest.php +++ b/tests/phpunit/includes/api/ApiBaseTest.php @@ -6,12 +6,45 @@ use Wikimedia\TestingAccessWrapper; * @group API * @group Database * @group medium + * + * @covers ApiBase */ class ApiBaseTest extends ApiTestCase { - /** - * @covers ApiBase::requireOnlyOneParameter + * This covers a variety of stub methods that return a fixed value. + * + * @param string|array $method Name of method, or [ name, params... ] + * @param string $value Expected value + * + * @dataProvider provideStubMethods */ + public function testStubMethods( $expected, $method, $args = [] ) { + // Some of these are protected + $mock = TestingAccessWrapper::newFromObject( new MockApi() ); + $result = call_user_func_array( [ $mock, $method ], $args ); + $this->assertSame( $expected, $result ); + } + + public function provideStubMethods() { + return [ + [ null, 'getModuleManager' ], + [ null, 'getCustomPrinter' ], + [ [], 'getHelpUrls' ], + // @todo This is actually overriden by MockApi + // [ [], 'getAllowedParams' ], + [ true, 'shouldCheckMaxLag' ], + [ true, 'isReadMode' ], + [ false, 'isWriteMode' ], + [ false, 'mustBePosted' ], + [ false, 'isDeprecated' ], + [ false, 'isInternal' ], + [ false, 'needsToken' ], + [ null, 'getWebUITokenSalt', [ [] ] ], + [ null, 'getConditionalRequestData', [ 'etag' ] ], + [ null, 'dynamicParameterDocumentation' ], + ]; + } + public function testRequireOnlyOneParameterDefault() { $mock = new MockApi(); $mock->requireOnlyOneParameter( @@ -23,7 +56,6 @@ class ApiBaseTest extends ApiTestCase { /** * @expectedException ApiUsageException - * @covers ApiBase::requireOnlyOneParameter */ public function testRequireOnlyOneParameterZero() { $mock = new MockApi(); @@ -35,7 +67,6 @@ class ApiBaseTest extends ApiTestCase { /** * @expectedException ApiUsageException - * @covers ApiBase::requireOnlyOneParameter */ public function testRequireOnlyOneParameterTrue() { $mock = new MockApi(); @@ -45,38 +76,230 @@ class ApiBaseTest extends ApiTestCase { ); } + public function testRequireOnlyOneParameterMissing() { + $this->setExpectedException( ApiUsageException::class, + 'One of the parameters "foo" and "bar" is required.' ); + $mock = new MockApi(); + $mock->requireOnlyOneParameter( + [ "filename" => "foo.txt", "enablechunks" => false ], + "foo", "bar" ); + } + + public function testRequireMaxOneParameterZero() { + $mock = new MockApi(); + $mock->requireMaxOneParameter( + [ 'foo' => 'bar', 'baz' => 'quz' ], + 'squirrel' ); + $this->assertTrue( true ); + } + + public function testRequireMaxOneParameterOne() { + $mock = new MockApi(); + $mock->requireMaxOneParameter( + [ 'foo' => 'bar', 'baz' => 'quz' ], + 'foo', 'squirrel' ); + $this->assertTrue( true ); + } + + public function testRequireMaxOneParameterTwo() { + $this->setExpectedException( ApiUsageException::class, + 'The parameters "foo" and "baz" can not be used together.' ); + $mock = new MockApi(); + $mock->requireMaxOneParameter( + [ 'foo' => 'bar', 'baz' => 'quz' ], + 'foo', 'baz' ); + } + + public function testRequireAtLeastOneParameterZero() { + $this->setExpectedException( ApiUsageException::class, + 'At least one of the parameters "foo" and "bar" is required.' ); + $mock = new MockApi(); + $mock->requireAtLeastOneParameter( + [ 'a' => 'b', 'c' => 'd' ], + 'foo', 'bar' ); + } + + public function testRequireAtLeastOneParameterOne() { + $mock = new MockApi(); + $mock->requireAtLeastOneParameter( + [ 'a' => 'b', 'c' => 'd' ], + 'foo', 'a' ); + $this->assertTrue( true ); + } + + public function testRequireAtLeastOneParameterTwo() { + $mock = new MockApi(); + $mock->requireAtLeastOneParameter( + [ 'a' => 'b', 'c' => 'd' ], + 'a', 'c' ); + $this->assertTrue( true ); + } + + public function testGetTitleOrPageIdBadParams() { + $this->setExpectedException( ApiUsageException::class, + 'The parameters "title" and "pageid" can not be used together.' ); + $mock = new MockApi(); + $mock->getTitleOrPageId( [ 'title' => 'a', 'pageid' => 7 ] ); + } + + public function testGetTitleOrPageIdTitle() { + $mock = new MockApi(); + $result = $mock->getTitleOrPageId( [ 'title' => 'Foo' ] ); + $this->assertInstanceOf( WikiPage::class, $result ); + $this->assertSame( 'Foo', $result->getTitle()->getPrefixedText() ); + } + + public function testGetTitleOrPageIdInvalidTitle() { + $this->setExpectedException( ApiUsageException::class, + 'Bad title "|".' ); + $mock = new MockApi(); + $mock->getTitleOrPageId( [ 'title' => '|' ] ); + } + + public function testGetTitleOrPageIdSpecialTitle() { + $this->setExpectedException( ApiUsageException::class, + "Namespace doesn't allow actual pages." ); + $mock = new MockApi(); + $mock->getTitleOrPageId( [ 'title' => 'Special:RandomPage' ] ); + } + + public function testGetTitleOrPageIdPageId() { + $result = ( new MockApi() )->getTitleOrPageId( + [ 'pageid' => Title::newFromText( 'UTPage' )->getArticleId() ] ); + $this->assertInstanceOf( WikiPage::class, $result ); + $this->assertSame( 'UTPage', $result->getTitle()->getPrefixedText() ); + } + + public function testGetTitleOrPageIdInvalidPageId() { + $this->setExpectedException( ApiUsageException::class, + 'There is no page with ID 2147483648.' ); + $mock = new MockApi(); + $mock->getTitleOrPageId( [ 'pageid' => 2147483648 ] ); + } + + public function testGetTitleFromTitleOrPageIdBadParams() { + $this->setExpectedException( ApiUsageException::class, + 'The parameters "title" and "pageid" can not be used together.' ); + $mock = new MockApi(); + $mock->getTitleFromTitleOrPageId( [ 'title' => 'a', 'pageid' => 7 ] ); + } + + public function testGetTitleFromTitleOrPageIdTitle() { + $mock = new MockApi(); + $result = $mock->getTitleFromTitleOrPageId( [ 'title' => 'Foo' ] ); + $this->assertInstanceOf( Title::class, $result ); + $this->assertSame( 'Foo', $result->getPrefixedText() ); + } + + public function testGetTitleFromTitleOrPageIdInvalidTitle() { + $this->setExpectedException( ApiUsageException::class, + 'Bad title "|".' ); + $mock = new MockApi(); + $mock->getTitleFromTitleOrPageId( [ 'title' => '|' ] ); + } + + public function testGetTitleFromTitleOrPageIdPageId() { + $result = ( new MockApi() )->getTitleFromTitleOrPageId( + [ 'pageid' => Title::newFromText( 'UTPage' )->getArticleId() ] ); + $this->assertInstanceOf( Title::class, $result ); + $this->assertSame( 'UTPage', $result->getPrefixedText() ); + } + + public function testGetTitleFromTitleOrPageIdInvalidPageId() { + $this->setExpectedException( ApiUsageException::class, + 'There is no page with ID 298401643.' ); + $mock = new MockApi(); + $mock->getTitleFromTitleOrPageId( [ 'pageid' => 298401643 ] ); + } + /** * @dataProvider provideGetParameterFromSettings * @param string|null $input * @param array $paramSettings * @param mixed $expected + * @param array $options Key-value pairs: + * 'parseLimits': true|false + * 'apihighlimits': true|false + * 'internalmode': true|false * @param string[] $warnings - * @covers ApiBase::getParameterFromSettings */ - public function testGetParameterFromSettings( $input, $paramSettings, $expected, $warnings ) { + public function testGetParameterFromSettings( + $input, $paramSettings, $expected, $warnings, $options = [] + ) { $mock = new MockApi(); $wrapper = TestingAccessWrapper::newFromObject( $mock ); $context = new DerivativeContext( $mock ); - $context->setRequest( new FauxRequest( $input !== null ? [ 'foo' => $input ] : [] ) ); + $context->setRequest( new FauxRequest( + $input !== null ? [ 'myParam' => $input ] : [] ) ); $wrapper->mMainModule = new ApiMain( $context ); - if ( $expected instanceof ApiUsageException ) { + $parseLimits = isset( $options['parseLimits'] ) ? + $options['parseLimits'] : true; + + if ( !empty( $options['apihighlimits'] ) ) { + $context->setUser( self::$users['sysop']->getUser() ); + } + + if ( isset( $options['internalmode'] ) && !$options['internalmode'] ) { + $mainWrapper = TestingAccessWrapper::newFromObject( $wrapper->mMainModule ); + $mainWrapper->mInternalMode = false; + } + + // If we're testing tags, set up some tags + if ( isset( $paramSettings[ApiBase::PARAM_TYPE] ) && + $paramSettings[ApiBase::PARAM_TYPE] === 'tags' + ) { + ChangeTags::defineTag( 'tag1' ); + ChangeTags::defineTag( 'tag2' ); + } + + if ( $expected instanceof Exception ) { try { - $wrapper->getParameterFromSettings( 'foo', $paramSettings, true ); - } catch ( ApiUsageException $ex ) { + $wrapper->getParameterFromSettings( 'myParam', $paramSettings, + $parseLimits ); + $this->fail( 'No exception thrown' ); + } catch ( Exception $ex ) { $this->assertEquals( $expected, $ex ); } } else { - $result = $wrapper->getParameterFromSettings( 'foo', $paramSettings, true ); - $this->assertSame( $expected, $result ); - $this->assertSame( $warnings, $mock->warnings ); + $result = $wrapper->getParameterFromSettings( 'myParam', + $paramSettings, $parseLimits ); + if ( isset( $paramSettings[ApiBase::PARAM_TYPE] ) && + $paramSettings[ApiBase::PARAM_TYPE] === 'timestamp' && + $expected === 'now' + ) { + // Allow one second of fuzziness. Make sure the formats are + // correct! + $this->assertRegExp( '/^\d{14}$/', $result ); + $this->assertLessThanOrEqual( 1, + abs( wfTimestamp( TS_UNIX, $result ) - time() ), + "Result $result differs from expected $expected by " . + 'more than one second' ); + } else { + $this->assertSame( $expected, $result ); + } + $actualWarnings = array_map( function ( $warn ) { + return $warn instanceof Message + ? array_merge( [ $warn->getKey() ], $warn->getParams() ) + : $warn; + }, $mock->warnings ); + $this->assertSame( $warnings, $actualWarnings ); + } + + if ( !empty( $paramSettings[ApiBase::PARAM_SENSITIVE] ) || + ( isset( $paramSettings[ApiBase::PARAM_TYPE] ) && + $paramSettings[ApiBase::PARAM_TYPE] === 'password' ) + ) { + $mainWrapper = TestingAccessWrapper::newFromObject( $wrapper->getMain() ); + $this->assertSame( [ 'myParam' ], + $mainWrapper->getSensitiveParams() ); } } public static function provideGetParameterFromSettings() { $warnings = [ - [ 'apiwarn-badutf8', 'foo' ], + [ 'apiwarn-badutf8', 'myParam' ], ]; $c0 = ''; @@ -88,7 +311,7 @@ class ApiBaseTest extends ApiTestCase { : '�'; } - return [ + $returnArray = [ 'Basic param' => [ 'bar', null, 'bar', [] ], 'Basic param, C0 controls' => [ $c0, null, $enc, $warnings ], 'String param' => [ 'bar', '', 'bar', [] ], @@ -97,7 +320,8 @@ class ApiBaseTest extends ApiTestCase { 'String param, required, empty' => [ '', [ ApiBase::PARAM_DFLT => 'default', ApiBase::PARAM_REQUIRED => true ], - ApiUsageException::newWithMessage( null, [ 'apierror-missingparam', 'foo' ] ), + ApiUsageException::newWithMessage( null, + [ 'apierror-missingparam', 'myParam' ] ), [] ], 'Multi-valued parameter' => [ @@ -124,12 +348,846 @@ class ApiBaseTest extends ApiTestCase { [ substr( $enc, 0, -3 ), '' ], $warnings ], + 'Multi-valued parameter with limits' => [ + 'a|b|c', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ISMULTI_LIMIT1 => 3, + ], + [ 'a', 'b', 'c' ], + [], + ], + 'Multi-valued parameter with exceeded limits' => [ + 'a|b|c', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ISMULTI_LIMIT1 => 2, + ], + [ 'a', 'b' ], + [ [ 'apiwarn-toomanyvalues', 'myParam', 2 ] ], + ], + 'Multi-valued parameter with exceeded limits for non-bot' => [ + 'a|b|c', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ISMULTI_LIMIT1 => 2, + ApiBase::PARAM_ISMULTI_LIMIT2 => 3, + ], + [ 'a', 'b' ], + [ [ 'apiwarn-toomanyvalues', 'myParam', 2 ] ], + ], + 'Multi-valued parameter with non-exceeded limits for bot' => [ + 'a|b|c', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ISMULTI_LIMIT1 => 2, + ApiBase::PARAM_ISMULTI_LIMIT2 => 3, + ], + [ 'a', 'b', 'c' ], + [], + [ 'apihighlimits' => true ], + ], + 'Multi-valued parameter with prohibited duplicates' => [ + 'a|b|a|c', + [ ApiBase::PARAM_ISMULTI => true ], + // Note that the keys are not sequential! This matches + // array_unique, but might be unexpected. + [ 0 => 'a', 1 => 'b', 3 => 'c' ], + [], + ], + 'Multi-valued parameter with allowed duplicates' => [ + 'a|a', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ALLOW_DUPLICATES => true, + ], + [ 'a', 'a' ], + [], + ], + 'Empty boolean param' => [ + '', + [ ApiBase::PARAM_TYPE => 'boolean' ], + true, + [], + ], + 'Boolean param 0' => [ + '0', + [ ApiBase::PARAM_TYPE => 'boolean' ], + true, + [], + ], + 'Boolean param false' => [ + 'false', + [ ApiBase::PARAM_TYPE => 'boolean' ], + true, + [], + ], + 'Boolean multi-param' => [ + 'true|false', + [ + ApiBase::PARAM_TYPE => 'boolean', + ApiBase::PARAM_ISMULTI => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'Multi-values not supported for myParam' + ), + [], + ], + 'Empty boolean param with non-false default' => [ + '', + [ + ApiBase::PARAM_TYPE => 'boolean', + ApiBase::PARAM_DFLT => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + "Boolean param myParam's default is set to '1'. " . + 'Boolean parameters must default to false.' ), + [], + ], + 'Deprecated parameter' => [ + 'foo', + [ ApiBase::PARAM_DEPRECATED => true ], + 'foo', + [ [ 'apiwarn-deprecation-parameter', 'myParam' ] ], + ], + 'Deprecated parameter value' => [ + 'a', + [ ApiBase::PARAM_DEPRECATED_VALUES => [ 'a' => true ] ], + 'a', + [ [ 'apiwarn-deprecation-parameter', 'myParam=a' ] ], + ], + 'Multiple deprecated parameter values' => [ + 'a|b|c|d', + [ ApiBase::PARAM_DEPRECATED_VALUES => + [ 'b' => true, 'd' => true ], + ApiBase::PARAM_ISMULTI => true ], + [ 'a', 'b', 'c', 'd' ], + [ + [ 'apiwarn-deprecation-parameter', 'myParam=b' ], + [ 'apiwarn-deprecation-parameter', 'myParam=d' ], + ], + ], + 'Deprecated parameter value with custom warning' => [ + 'a', + [ ApiBase::PARAM_DEPRECATED_VALUES => [ 'a' => 'my-msg' ] ], + 'a', + [ 'my-msg' ], + ], + '"*" when wildcard not allowed' => [ + '*', + [ ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => [ 'a', 'b', 'c' ] ], + [], + [ [ 'apiwarn-unrecognizedvalues', 'myParam', + [ 'list' => [ '*' ], 'type' => 'comma' ], 1 ] ], + ], + 'Wildcard "*"' => [ + '*', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => [ 'a', 'b', 'c' ], + ApiBase::PARAM_ALL => true, + ], + [ 'a', 'b', 'c' ], + [], + ], + 'Wildcard "*" with multiples not allowed' => [ + '*', + [ + ApiBase::PARAM_TYPE => [ 'a', 'b', 'c' ], + ApiBase::PARAM_ALL => true, + ], + ApiUsageException::newWithMessage( null, + [ 'apierror-unrecognizedvalue', 'myParam', '*' ], + 'unknown_myParam' ), + [], + ], + 'Wildcard "*" with unrestricted type' => [ + '*', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_ALL => true, + ], + [ '*' ], + [], + ], + 'Wildcard "x"' => [ + 'x', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => [ 'a', 'b', 'c' ], + ApiBase::PARAM_ALL => 'x', + ], + [ 'a', 'b', 'c' ], + [], + ], + 'Wildcard conflicting with allowed value' => [ + 'a', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => [ 'a', 'b', 'c' ], + ApiBase::PARAM_ALL => 'a', + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'For param myParam, PARAM_ALL collides with a possible ' . + 'value' ), + [], + ], + 'Namespace with wildcard' => [ + '*', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'namespace', + ], + MWNamespace::getValidNamespaces(), + [], + ], + // PARAM_ALL is ignored with namespace types. + 'Namespace with wildcard suppressed' => [ + '*', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'namespace', + ApiBase::PARAM_ALL => false, + ], + MWNamespace::getValidNamespaces(), + [], + ], + 'Namespace with wildcard "x"' => [ + 'x', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'namespace', + ApiBase::PARAM_ALL => 'x', + ], + [], + [ [ 'apiwarn-unrecognizedvalues', 'myParam', + [ 'list' => [ 'x' ], 'type' => 'comma' ], 1 ] ], + ], + 'Password' => [ + 'dDy+G?e?txnr.1:(@[Ru', + [ ApiBase::PARAM_TYPE => 'password' ], + 'dDy+G?e?txnr.1:(@[Ru', + [], + ], + 'Sensitive field' => [ + 'I am fond of pineapples', + [ ApiBase::PARAM_SENSITIVE => true ], + 'I am fond of pineapples', + [], + ], + 'Upload with default' => [ + '', + [ + ApiBase::PARAM_TYPE => 'upload', + ApiBase::PARAM_DFLT => '', + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + "File upload param myParam's default is set to ''. " . + 'File upload parameters may not have a default.' ), + [], + ], + 'Multiple upload' => [ + '', + [ + ApiBase::PARAM_TYPE => 'upload', + ApiBase::PARAM_ISMULTI => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'Multi-values not supported for myParam' ), + [], + ], + // @todo Test actual upload + 'Namespace -1' => [ + '-1', + [ ApiBase::PARAM_TYPE => 'namespace' ], + ApiUsageException::newWithMessage( null, + [ 'apierror-unrecognizedvalue', 'myParam', '-1' ], + 'unknown_myParam' ), + [], + ], + 'Extra namespace -1' => [ + '-1', + [ + ApiBase::PARAM_TYPE => 'namespace', + ApiBase::PARAM_EXTRA_NAMESPACES => [ '-1' ], + ], + '-1', + [], + ], + // @todo Test with PARAM_SUBMODULE_MAP unset, need + // getModuleManager() to return something real + 'Nonexistent module' => [ + 'not-a-module-name', + [ + ApiBase::PARAM_TYPE => 'submodule', + ApiBase::PARAM_SUBMODULE_MAP => + [ 'foo' => 'foo', 'bar' => 'foo+bar' ], + ], + ApiUsageException::newWithMessage( + null, + [ + 'apierror-unrecognizedvalue', + 'myParam', + 'not-a-module-name', + ], + 'unknown_myParam' + ), + [], + ], + '\\x1f with multiples not allowed' => [ + "\x1f", + [], + ApiUsageException::newWithMessage( null, + 'apierror-badvalue-notmultivalue', + 'badvalue_notmultivalue' ), + [], + ], + 'Integer with unenforced min' => [ + '-2', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => -1, + ], + -1, + [ [ 'apierror-integeroutofrange-belowminimum', 'myParam', -1, + -2 ] ], + ], + 'Integer with enforced min' => [ + '-2', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => -1, + ApiBase::PARAM_RANGE_ENFORCE => true, + ], + ApiUsageException::newWithMessage( null, + [ 'apierror-integeroutofrange-belowminimum', 'myParam', + '-1', '-2' ], 'integeroutofrange', + [ 'min' => -1, 'max' => null, 'botMax' => null ] ), + [], + ], + 'Integer with unenforced max (internal mode)' => [ + '8', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MAX => 7, + ], + 8, + [], + ], + 'Integer with enforced max (internal mode)' => [ + '8', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MAX => 7, + ApiBase::PARAM_RANGE_ENFORCE => true, + ], + 8, + [], + ], + 'Integer with unenforced max (non-internal mode)' => [ + '8', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MAX => 7, + ], + 7, + [ [ 'apierror-integeroutofrange-abovemax', 'myParam', 7, 8 ] ], + [ 'internalmode' => false ], + ], + 'Integer with enforced max (non-internal mode)' => [ + '8', + [ + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MAX => 7, + ApiBase::PARAM_RANGE_ENFORCE => true, + ], + ApiUsageException::newWithMessage( + null, + [ 'apierror-integeroutofrange-abovemax', 'myParam', '7', '8' ], + 'integeroutofrange', + [ 'min' => null, 'max' => 7, 'botMax' => 7 ] + ), + [], + [ 'internalmode' => false ], + ], + 'Array of integers' => [ + '3|12|966|-1', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'integer', + ], + [ 3, 12, 966, -1 ], + [], + ], + 'Array of integers with unenforced min/max (internal mode)' => [ + '3|12|966|-1', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => 0, + ApiBase::PARAM_MAX => 100, + ], + [ 3, 12, 966, 0 ], + [ [ 'apierror-integeroutofrange-belowminimum', 'myParam', 0, -1 ] ], + ], + 'Array of integers with enforced min/max (internal mode)' => [ + '3|12|966|-1', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => 0, + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_RANGE_ENFORCE => true, + ], + ApiUsageException::newWithMessage( + null, + [ 'apierror-integeroutofrange-belowminimum', 'myParam', 0, -1 ], + 'integeroutofrange', + [ 'min' => 0, 'max' => 100, 'botMax' => 100 ] + ), + [], + ], + 'Array of integers with unenforced min/max (non-internal mode)' => [ + '3|12|966|-1', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => 0, + ApiBase::PARAM_MAX => 100, + ], + [ 3, 12, 100, 0 ], + [ + [ 'apierror-integeroutofrange-abovemax', 'myParam', 100, 966 ], + [ 'apierror-integeroutofrange-belowminimum', 'myParam', 0, -1 ] + ], + [ 'internalmode' => false ], + ], + 'Array of integers with enforced min/max (non-internal mode)' => [ + '3|12|966|-1', + [ + ApiBase::PARAM_ISMULTI => true, + ApiBase::PARAM_TYPE => 'integer', + ApiBase::PARAM_MIN => 0, + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_RANGE_ENFORCE => true, + ], + ApiUsageException::newWithMessage( + null, + [ 'apierror-integeroutofrange-abovemax', 'myParam', 100, 966 ], + 'integeroutofrange', + [ 'min' => 0, 'max' => 100, 'botMax' => 100 ] + ), + [], + [ 'internalmode' => false ], + ], + 'Limit with parseLimits false' => [ + '100', + [ ApiBase::PARAM_TYPE => 'limit' ], + '100', + [], + [ 'parseLimits' => false ], + ], + 'Limit with no max' => [ + '100', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX2 => 10, + ApiBase::PARAM_ISMULTI => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'MAX1 or MAX2 are not defined for the limit myParam' ), + [], + ], + 'Limit with no max2' => [ + '100', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 10, + ApiBase::PARAM_ISMULTI => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'MAX1 or MAX2 are not defined for the limit myParam' ), + [], + ], + 'Limit with multi-value' => [ + '100', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 10, + ApiBase::PARAM_MAX2 => 10, + ApiBase::PARAM_ISMULTI => true, + ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + 'Multi-values not supported for myParam' ), + [], + ], + 'Valid limit' => [ + '100', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 100, + ], + 100, + [], + ], + 'Limit max' => [ + 'max', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 100, + [], + ], + 'Limit max for apihighlimits' => [ + 'max', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 101, + [], + [ 'apihighlimits' => true ], + ], + 'Limit too large (internal mode)' => [ + '101', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 101, + [], + ], + 'Limit okay for apihighlimits (internal mode)' => [ + '101', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 101, + [], + [ 'apihighlimits' => true ], + ], + 'Limit too large for apihighlimits (internal mode)' => [ + '102', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 102, + [], + [ 'apihighlimits' => true ], + ], + 'Limit too large (non-internal mode)' => [ + '101', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 100, + [ [ 'apierror-integeroutofrange-abovemax', 'myParam', 100, 101 ] ], + [ 'internalmode' => false ], + ], + 'Limit okay for apihighlimits (non-internal mode)' => [ + '101', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 101, + [], + [ 'internalmode' => false, 'apihighlimits' => true ], + ], + 'Limit too large for apihighlimits (non-internal mode)' => [ + '102', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 101, + ], + 101, + [ [ 'apierror-integeroutofrange-abovebotmax', 'myParam', 101, 102 ] ], + [ 'internalmode' => false, 'apihighlimits' => true ], + ], + 'Limit too small' => [ + '-2', + [ + ApiBase::PARAM_TYPE => 'limit', + ApiBase::PARAM_MIN => -1, + ApiBase::PARAM_MAX => 100, + ApiBase::PARAM_MAX2 => 100, + ], + -1, + [ [ 'apierror-integeroutofrange-belowminimum', 'myParam', -1, + -2 ] ], + ], + 'Timestamp' => [ + wfTimestamp( TS_UNIX, '20211221122112' ), + [ ApiBase::PARAM_TYPE => 'timestamp' ], + '20211221122112', + [], + ], + 'Timestamp 0' => [ + '0', + [ ApiBase::PARAM_TYPE => 'timestamp' ], + // Magic keyword + 'now', + [ [ 'apiwarn-unclearnowtimestamp', 'myParam', '0' ] ], + ], + 'Timestamp empty' => [ + '', + [ ApiBase::PARAM_TYPE => 'timestamp' ], + 'now', + [ [ 'apiwarn-unclearnowtimestamp', 'myParam', '' ] ], + ], + // wfTimestamp() interprets this as Unix time + 'Timestamp 00' => [ + '00', + [ ApiBase::PARAM_TYPE => 'timestamp' ], + '19700101000000', + [], + ], + 'Timestamp now' => [ + 'now', + [ ApiBase::PARAM_TYPE => 'timestamp' ], + 'now', + [], + ], + 'Invalid timestamp' => [ + 'a potato', + [ ApiBase::PARAM_TYPE => 'timestamp' ], + ApiUsageException::newWithMessage( + null, + [ 'apierror-badtimestamp', 'myParam', 'a potato' ], + 'badtimestamp_myParam' + ), + [], + ], + 'Timestamp array' => [ + '100|101', + [ + ApiBase::PARAM_TYPE => 'timestamp', + ApiBase::PARAM_ISMULTI => 1, + ], + [ wfTimestamp( TS_MW, 100 ), wfTimestamp( TS_MW, 101 ) ], + [], + ], + 'User' => [ + 'foo_bar', + [ ApiBase::PARAM_TYPE => 'user' ], + 'Foo bar', + [], + ], + 'Invalid username "|"' => [ + '|', + [ ApiBase::PARAM_TYPE => 'user' ], + ApiUsageException::newWithMessage( null, + [ 'apierror-baduser', 'myParam', '|' ], + 'baduser_myParam' ), + [], + ], + 'Invalid username "300.300.300.300"' => [ + '300.300.300.300', + [ ApiBase::PARAM_TYPE => 'user' ], + ApiUsageException::newWithMessage( null, + [ 'apierror-baduser', 'myParam', '300.300.300.300' ], + 'baduser_myParam' ), + [], + ], + 'IP range as username' => [ + '10.0.0.0/8', + [ ApiBase::PARAM_TYPE => 'user' ], + '10.0.0.0/8', + [], + ], + 'IPv6 as username' => [ + '::1', + [ ApiBase::PARAM_TYPE => 'user' ], + '0:0:0:0:0:0:0:1', + [], + ], + 'Obsolete cloaked usemod IP address as username' => [ + '1.2.3.xxx', + [ ApiBase::PARAM_TYPE => 'user' ], + '1.2.3.xxx', + [], + ], + 'Invalid username containing IP address' => [ + 'This is [not] valid 1.2.3.xxx, ha!', + [ ApiBase::PARAM_TYPE => 'user' ], + ApiUsageException::newWithMessage( + null, + [ 'apierror-baduser', 'myParam', 'This is [not] valid 1.2.3.xxx, ha!' ], + 'baduser_myParam' + ), + [], + ], + 'External username' => [ + 'M>Foo bar', + [ ApiBase::PARAM_TYPE => 'user' ], + 'M>Foo bar', + [], + ], + 'Array of usernames' => [ + 'foo|bar', + [ + ApiBase::PARAM_TYPE => 'user', + ApiBase::PARAM_ISMULTI => true, + ], + [ 'Foo', 'Bar' ], + [], + ], + 'tag' => [ + 'tag1', + [ ApiBase::PARAM_TYPE => 'tags' ], + [ 'tag1' ], + [], + ], + 'Array of one tag' => [ + 'tag1', + [ + ApiBase::PARAM_TYPE => 'tags', + ApiBase::PARAM_ISMULTI => true, + ], + [ 'tag1' ], + [], + ], + 'Array of tags' => [ + 'tag1|tag2', + [ + ApiBase::PARAM_TYPE => 'tags', + ApiBase::PARAM_ISMULTI => true, + ], + [ 'tag1', 'tag2' ], + [], + ], + 'Invalid tag' => [ + 'invalid tag', + [ ApiBase::PARAM_TYPE => 'tags' ], + new ApiUsageException( null, + Status::newFatal( 'tags-apply-not-allowed-one', + 'invalid tag', 1 ) ), + [], + ], + 'Unrecognized type' => [ + 'foo', + [ ApiBase::PARAM_TYPE => 'nonexistenttype' ], + new MWException( + 'Internal error in ApiBase::getParameterFromSettings: ' . + "Param myParam's type is unknown - nonexistenttype" ), + [], + ], + 'Too many bytes' => [ + '1', + [ + ApiBase::PARAM_MAX_BYTES => 0, + ApiBase::PARAM_MAX_CHARS => 0, + ], + ApiUsageException::newWithMessage( null, + [ 'apierror-maxbytes', 'myParam', 0 ] ), + [], + ], + 'Too many chars' => [ + '§§', + [ + ApiBase::PARAM_MAX_BYTES => 4, + ApiBase::PARAM_MAX_CHARS => 1, + ], + ApiUsageException::newWithMessage( null, + [ 'apierror-maxchars', 'myParam', 1 ] ), + [], + ], + 'Omitted required param' => [ + null, + [ ApiBase::PARAM_REQUIRED => true ], + ApiUsageException::newWithMessage( null, + [ 'apierror-missingparam', 'myParam' ] ), + [], + ], + 'Empty multi-value' => [ + '', + [ ApiBase::PARAM_ISMULTI => true ], + [], + [], + ], + 'Multi-value \x1f' => [ + "\x1f", + [ ApiBase::PARAM_ISMULTI => true ], + [], + [], + ], + 'Allowed non-multi-value with "|"' => [ + 'a|b', + [ ApiBase::PARAM_TYPE => [ 'a|b' ] ], + 'a|b', + [], + ], + 'Prohibited multi-value' => [ + 'a|b', + [ ApiBase::PARAM_TYPE => [ 'a', 'b' ] ], + ApiUsageException::newWithMessage( null, + [ + 'apierror-multival-only-one-of', + 'myParam', + Message::listParam( [ 'a', 'b' ] ), + 2 + ], + 'multival_myParam' + ), + [], + ], + ]; + + // The following really just test PHP's string-to-int conversion. + $integerTests = [ + [ '+1', 1 ], + [ '-1', -1 ], + [ '1e3', 1 ], + [ '1.5', 1 ], + [ '-1.5', -1 ], + [ '1abc', 1 ], + [ ' 1', 1 ], + [ "\t1", 1, '\t1' ], + [ "\r1", 1, '\r1' ], + [ "\f1", 0, '\f1', 'badutf-8' ], + [ "\n1", 1, '\n1' ], + [ "\v1", 0, '\v1', 'badutf-8' ], + [ "\e1", 0, '\e1', 'badutf-8' ], + [ "\x001", 0, '\x001', 'badutf-8' ], ]; + + foreach ( $integerTests as $test ) { + $desc = isset( $test[2] ) ? $test[2] : $test[0]; + $warnings = isset( $test[3] ) ? + [ [ 'apiwarn-badutf8', 'myParam' ] ] : []; + $returnArray["\"$desc\" as integer"] = [ + $test[0], + [ ApiBase::PARAM_TYPE => 'integer' ], + $test[1], + $warnings, + ]; + } + + return $returnArray; } - /** - * @covers ApiBase::errorArrayToStatus - */ public function testErrorArrayToStatus() { $mock = new MockApi(); @@ -179,9 +1237,6 @@ class ApiBaseTest extends ApiTestCase { ], $user ) ); } - /** - * @covers ApiBase::dieStatus - */ public function testDieStatus() { $mock = new MockApi(); -- 2.20.1