From 114d6db0167725022b35bb544e010a72ffd63d13 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 22:32:39 +0000 Subject: [PATCH] Allow anonymous revision-metadata queries for Page Previews Since 1.7.0 action=query sub-modules are matched against $wgCrawlerProtectedApiModules, so protecting 'revisions' broke Page Previews (Popups) for anonymous readers: its preview request includes prop=revisions&rvprop=timestamp for a single title. Add $wgCrawlerProtectionAllowRevisionMetadata (default true). When set, an anonymous action=query request is not denied on account of 'revisions' if it is the only protected module, the request names exactly one page via titles or pageids (no generator, no revids), rvprop is given explicitly and contains only 'timestamp', and no other rv* parameter is present. The CrawlerProtectionShouldDeny hook still receives the final decision. The multi-value separator logic is factored out into CrawlerProtectionService::splitApiMultiValue() and shared with Hooks::getApiModuleNames(). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Dk5bcSgoy2euC39AzvNmnV --- README.md | 29 ++ extension.json | 5 +- includes/CrawlerProtectionService.php | 108 +++++ includes/Hooks.php | 5 +- .../CrawlerProtectionIntegrationTest.php | 77 ++++ tests/phpunit/namespaced-stubs.php | 7 + .../unit/CrawlerProtectionServiceTest.php | 406 +++++++++++++++++- 7 files changed, 631 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 03d7e3e..271a851 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,35 @@ addresses in `$wgCrawlerProtectionAllowedIPs` are always permitted. ```php $wgCrawlerProtectedApiModules = [ 'compare', 'parse', 'revisions', 'recentchanges', 'backlinks' ]; ``` +* `$wgCrawlerProtectionAllowRevisionMetadata` - when `true` (default), an + anonymous `action=query` request is not denied on account of `'revisions'` + being in `$wgCrawlerProtectedApiModules` if it only asks for the timestamp + of the latest revision of a single page. This keeps + [Page Previews](https://www.mediawiki.org/wiki/Extension:Popups) working + for anonymous readers: its preview request is + `action=query&prop=info|extracts|pageimages|revisions|info&rvprop=timestamp&titles=` + (plus `formatversion`, `redirects`, `inprop`, and the `ex*` and `pi*` + parameters of TextExtracts and PageImages), and without this exemption every + preview fails with "There was an issue displaying this preview." A request + is let through only when all of the following hold: + - `revisions` is the only protected module in the request; any other + protected module (or a protected `query`) still denies it; + - it names exactly one page through a single `titles` or `pageids` value, + and uses neither `generator` nor `revids`; + - `rvprop` is present and contains only `timestamp`. A missing `rvprop` + does not qualify, because the API then returns its default props, which + include the user and the edit summary; + - no other `rv*` parameter is present (`rvlimit`, `rvstart`, `rvend`, + `rvstartid`, `rvendid`, `rvdir`, `rvuser`, `rvexcludeuser`, `rvtag`, + `rvcontinue`, `rvslots`, `rvsection`, `rvparse`, `rvexpandtemplates`, + `rvgeneratexml`, `rvdiffto`, `rvdifftotext`, `rvdifftotextpst`, + `rvcontentformat` and `rvcontentformat-{slot}`), since each of them pages + through revisions or reads revision content. + + Multi-value parameters are recognised in both the `|` and the + leading-`\x1f` form. Set to `false` to deny every anonymous request that + involves a protected `revisions` module. The `CrawlerProtectionShouldDeny` + hook still receives the final decision and can override it. * `$wgCrawlerProtectedRestPaths` - array of REST API path glob patterns to block for anonymous users (default: `[]`). Each pattern is tested with `fnmatch()` with the `FNM_PATHNAME` flag, so `*` matches any single path diff --git a/extension.json b/extension.json index 705599c..d4f70f6 100644 --- a/extension.json +++ b/extension.json @@ -1,7 +1,7 @@ { "name": "CrawlerProtection", "author": "[https://mywikis.com MyWikis LLC]", - "version": "1.7.0", + "version": "1.8.0", "url": "https://www.mediawiki.org/wiki/Extension:CrawlerProtection", "descriptionmsg": "crawlerprotection-desc", "type": "hook", @@ -70,6 +70,9 @@ "value": [], "merge_strategy": "provide_default" }, + "CrawlerProtectionAllowRevisionMetadata": { + "value": true + }, "CrawlerProtectionProtectRevisions": { "value": true }, diff --git a/includes/CrawlerProtectionService.php b/includes/CrawlerProtectionService.php index 0faf6ca..4701e3c 100644 --- a/includes/CrawlerProtectionService.php +++ b/includes/CrawlerProtectionService.php @@ -53,11 +53,22 @@ class CrawlerProtectionService { 'CrawlerProtectedRestPaths', 'CrawlerProtectedSpecialPages', 'CrawlerProtectionAllowedIPs', + 'CrawlerProtectionAllowRevisionMetadata', 'CrawlerProtectionProtectRevisions', 'CrawlerProtectionTreatTempUsersAsAnon', 'CrawlerProtectionTrustXForwardedFor', ]; + /** + * Values of the "rvprop" parameter that $wgCrawlerProtectionAllowRevisionMetadata + * lets through. Page Previews (Popups) asks for "timestamp" only; every other + * value exposes revision content, its author or edit summary, or more than + * the bare metadata the previews need. + * + * @var string[] + */ + private const REVISION_METADATA_PROPS = [ 'timestamp' ]; + /** @var ServiceOptions */ private ServiceOptions $options; @@ -328,6 +339,10 @@ public function checkApiModules( array $moduleNames, $user, $request = null ): b break; } } + + if ( $shouldDeny && $this->isRevisionMetadataQuery( $moduleNames, $request ) ) { + $shouldDeny = false; + } } $this->hookRunner->onCrawlerProtectionShouldDeny( @@ -352,6 +367,99 @@ public function checkApiModules( array $moduleNames, $user, $request = null ): b return true; } + /** + * Determine whether an action=query request only asks the "revisions" + * module for metadata of the latest revision of a single page, which + * $wgCrawlerProtectionAllowRevisionMetadata lets anonymous users through. + * + * Page Previews (Popups) sends action=query&prop=...|revisions&rvprop=timestamp + * for one title, so protecting "revisions" would otherwise break previews + * for anonymous readers. The rules are deliberately narrow: + * - "revisions" must be the only protected module in the request; + * - exactly one page is named by "titles" or "pageids", and neither a + * generator nor "revids" is used, so the request cannot fan out; + * - "rvprop" is given explicitly and holds only REVISION_METADATA_PROPS, + * because without it the API falls back to props that include the + * comment and user; + * - no other "rv" parameter is present, since all of them page through + * revisions or read revision content (rvlimit, rvslots, rvsection, ...). + * + * @param string[] $moduleNames Module names involved in the request + * @param WebRequest|null $request + * @return bool + */ + private function isRevisionMetadataQuery( array $moduleNames, $request ): bool { + if ( $request === null + || !$this->options->get( 'CrawlerProtectionAllowRevisionMetadata' ) + || strtolower( $moduleNames[0] ?? '' ) !== 'query' + ) { + return false; + } + + $protectedModules = array_unique( array_map( + 'strtolower', + array_filter( $moduleNames, [ $this, 'isProtectedApiModule' ] ) + ) ); + if ( array_values( $protectedModules ) !== [ 'revisions' ] ) { + return false; + } + + if ( $request->getVal( 'generator' ) !== null || $request->getVal( 'revids' ) !== null ) { + return false; + } + + $titles = $request->getVal( 'titles' ); + $pageIds = $request->getVal( 'pageids' ); + if ( $titles !== null && $pageIds !== null ) { + return false; + } + $pageTarget = $titles ?? $pageIds; + if ( $pageTarget === null ) { + return false; + } + $pages = self::splitApiMultiValue( $pageTarget ); + if ( count( $pages ) !== 1 || $pages[0] === '' ) { + return false; + } + + $revisionProps = $request->getVal( 'rvprop' ); + if ( $revisionProps === null ) { + return false; + } + foreach ( self::splitApiMultiValue( $revisionProps ) as $prop ) { + if ( !in_array( $prop, self::REVISION_METADATA_PROPS, true ) ) { + return false; + } + } + + foreach ( array_keys( $request->getValues() ) as $name ) { + $name = strtolower( (string)$name ); + if ( strncmp( $name, 'rv', 2 ) === 0 && $name !== 'rvprop' ) { + return false; + } + } + + return true; + } + + /** + * Split a multi-value Action API parameter into its values. + * + * Like ApiBase, this uses "\x1f" as the separator when the value starts + * with that character, and "|" otherwise. Values are returned as sent: + * they are neither trimmed nor filtered for empty strings. + * + * @since 1.8.0 + * @param string $value + * @return string[] + */ + public static function splitApiMultiValue( string $value ): array { + if ( substr( $value, 0, 1 ) === "\x1f" ) { + return explode( "\x1f", substr( $value, 1 ) ); + } + return explode( '|', $value ); + } + /** * Determine whether the given API module name is in the * configured list of protected modules. diff --git a/includes/Hooks.php b/includes/Hooks.php index bbbe26a..b3aafba 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -172,10 +172,7 @@ private function getApiModuleNames( $module ): array { if ( $value === null || $value === '' ) { continue; } - // MediaWiki uses "\x1f" as the separator when a multi-value - // parameter starts with that character, and "|" otherwise. - $separator = substr( $value, 0, 1 ) === "\x1f" ? "\x1f" : '|'; - foreach ( explode( $separator, $value ) as $subModule ) { + foreach ( CrawlerProtectionService::splitApiMultiValue( $value ) as $subModule ) { $subModule = trim( $subModule ); if ( $subModule !== '' ) { $names[] = $subModule; diff --git a/tests/phpunit/integration/CrawlerProtectionIntegrationTest.php b/tests/phpunit/integration/CrawlerProtectionIntegrationTest.php index b10270c..8cc852c 100644 --- a/tests/phpunit/integration/CrawlerProtectionIntegrationTest.php +++ b/tests/phpunit/integration/CrawlerProtectionIntegrationTest.php @@ -72,6 +72,7 @@ private function overrideCrawlerProtectionConfig( array $overrides = [] ): void 'CrawlerProtectedSpecialPages' => [ 'whatlinkshere', 'recentchangeslinked' ], 'CrawlerProtectedQueryParams' => [ 'target' ], 'CrawlerProtectionAllowedIPs' => [], + 'CrawlerProtectionAllowRevisionMetadata' => true, 'CrawlerProtectionProtectRevisions' => true, 'CrawlerProtectionTreatTempUsersAsAnon' => false, 'CrawlerProtectionTrustXForwardedFor' => false, @@ -763,4 +764,80 @@ public function testApiCheckCanExecuteAllowsRegisteredUserOnProtectedModule(): v 'A registered user must not be denied' ); } + + /** + * A Page Previews (Popups) style request for the timestamp of a single + * page must reach the API even though "revisions" is protected. + * + * TextExtracts and PageImages are not installed here, so only the core + * modules of the Popups "prop" list are requested. + * + * @covers \MediaWiki\Extension\CrawlerProtection\Hooks::onApiCheckCanExecute + * @covers \MediaWiki\Extension\CrawlerProtection\CrawlerProtectionService::checkApiModules + */ + public function testApiCheckCanExecuteAllowsAnonymousRevisionMetadataQuery(): void { + // Arrange + $this->overrideCrawlerProtectionConfig( [ + 'CrawlerProtectedApiModules' => [ 'revisions' ], + ] ); + $this->useWebModeServiceInContainer(); + $api = $this->makeApiMain( + [ + 'action' => 'query', + 'prop' => 'info|revisions|info', + 'formatversion' => '2', + 'redirects' => '1', + 'rvprop' => 'timestamp', + 'inprop' => 'url', + 'titles' => 'Main Page', + ], + $this->makeAnonUser() + ); + + // Act + $api->execute(); + + // Assert + $this->assertArrayHasKey( + 'query', + $api->getResult()->getResultData( [], [ 'Strip' => 'all' ] ), + 'A revision-metadata query must execute normally' + ); + } + + /** + * The same request asking for revision content must still be denied. + * + * @covers \MediaWiki\Extension\CrawlerProtection\Hooks::onApiCheckCanExecute + * @covers \MediaWiki\Extension\CrawlerProtection\CrawlerProtectionService::checkApiModules + */ + public function testApiCheckCanExecuteDeniesAnonymousRevisionContentQuery(): void { + // Arrange + $this->overrideCrawlerProtectionConfig( [ + 'CrawlerProtectedApiModules' => [ 'revisions' ], + ] ); + $this->useWebModeServiceInContainer(); + $api = $this->makeApiMain( + [ + 'action' => 'query', + 'prop' => 'info|revisions', + 'rvprop' => 'timestamp|content', + 'titles' => 'Main Page', + ], + $this->makeAnonUser() + ); + + // Act + $denied = false; + try { + $api->execute(); + } catch ( \Exception $e ) { + $denied = method_exists( $e, 'getStatusValue' ) + // @phan-suppress-next-line PhanUndeclaredMethod ApiUsageException only + && $e->getStatusValue()->hasMessage( 'crawlerprotection-accessdenied-text' ); + } + + // Assert + $this->assertTrue( $denied, 'A query for revision content must be denied' ); + } } diff --git a/tests/phpunit/namespaced-stubs.php b/tests/phpunit/namespaced-stubs.php index 072ad5c..17ed06e 100644 --- a/tests/phpunit/namespaced-stubs.php +++ b/tests/phpunit/namespaced-stubs.php @@ -178,6 +178,13 @@ public function getVal( $name, $default = null ) { return $default; } + /** + * @return array + */ + public function getValues() { + return []; + } + public function getIP(): string { return '127.0.0.1'; } diff --git a/tests/phpunit/unit/CrawlerProtectionServiceTest.php b/tests/phpunit/unit/CrawlerProtectionServiceTest.php index 6c74af5..210dd63 100644 --- a/tests/phpunit/unit/CrawlerProtectionServiceTest.php +++ b/tests/phpunit/unit/CrawlerProtectionServiceTest.php @@ -61,6 +61,7 @@ public static function setUpBeforeClass(): void { * @param bool $treatTempUsersAsAnon * @param callable[] $shouldDenyHandlers Handlers for CrawlerProtectionShouldDeny * @param bool $trustXForwardedFor + * @param bool $allowRevisionMetadata * @return CrawlerProtectionService */ private function buildService( @@ -75,7 +76,8 @@ private function buildService( array $protectedRestPaths = [], bool $treatTempUsersAsAnon = false, array $shouldDenyHandlers = [], - bool $trustXForwardedFor = false + bool $trustXForwardedFor = false, + bool $allowRevisionMetadata = true ): CrawlerProtectionService { $options = new ServiceOptions( CrawlerProtectionService::CONSTRUCTOR_OPTIONS, @@ -86,6 +88,7 @@ private function buildService( 'CrawlerProtectedRestPaths' => $protectedRestPaths, 'CrawlerProtectedSpecialPages' => $protectedPages, 'CrawlerProtectionAllowedIPs' => $allowedIPs, + 'CrawlerProtectionAllowRevisionMetadata' => $allowRevisionMetadata, 'CrawlerProtectionProtectRevisions' => $protectRevisions, 'CrawlerProtectionTreatTempUsersAsAnon' => $treatTempUsersAsAnon, 'CrawlerProtectionTrustXForwardedFor' => $trustXForwardedFor, @@ -1384,6 +1387,7 @@ public function testIsProtectedActionToleratesScalarConfig() { 'CrawlerProtectedRestPaths' => [], 'CrawlerProtectedSpecialPages' => [], 'CrawlerProtectionAllowedIPs' => [], + 'CrawlerProtectionAllowRevisionMetadata' => true, 'CrawlerProtectionProtectRevisions' => true, 'CrawlerProtectionTreatTempUsersAsAnon' => false, 'CrawlerProtectionTrustXForwardedFor' => false, @@ -1418,6 +1422,7 @@ public function testHasProtectedQueryParamToleratesScalarConfig() { 'CrawlerProtectedRestPaths' => [], 'CrawlerProtectedSpecialPages' => [], 'CrawlerProtectionAllowedIPs' => [], + 'CrawlerProtectionAllowRevisionMetadata' => true, 'CrawlerProtectionProtectRevisions' => true, 'CrawlerProtectionTreatTempUsersAsAnon' => false, 'CrawlerProtectionTrustXForwardedFor' => false, @@ -1457,6 +1462,7 @@ public function testIsProtectedSpecialPageToleratesScalarConfig() { 'CrawlerProtectedRestPaths' => [], 'CrawlerProtectedSpecialPages' => 'WhatLinksHere', 'CrawlerProtectionAllowedIPs' => [], + 'CrawlerProtectionAllowRevisionMetadata' => true, 'CrawlerProtectionProtectRevisions' => true, 'CrawlerProtectionTreatTempUsersAsAnon' => false, 'CrawlerProtectionTrustXForwardedFor' => false, @@ -1675,6 +1681,404 @@ public function testCheckApiModuleDeniesWhenNoRequestGiven() { $this->assertFalse( $service->checkApiModule( 'revisions', $user ) ); } + // --------------------------------------------------------------- + // CrawlerProtectionAllowRevisionMetadata tests + // --------------------------------------------------------------- + + /** + * Parameters of the preview request sent by the plain MediaWiki API + * gateway of Page Previews (Popups), identical on REL1_39, REL1_43 and + * master (src/gateway/mediawiki.js). + */ + private const POPUPS_PARAMS = [ + 'action' => 'query', + 'format' => 'json', + 'prop' => 'info|extracts|pageimages|revisions|info', + 'formatversion' => '2', + 'redirects' => 'true', + 'exintro' => 'true', + 'exchars' => '525', + 'explaintext' => 'true', + 'exsectionformat' => 'plain', + 'piprop' => 'thumbnail', + 'pithumbsize' => '320', + 'pilicense' => 'any', + 'rvprop' => 'timestamp', + 'inprop' => 'url', + 'titles' => 'Main_Page', + 'smaxage' => '300', + 'maxage' => '300', + 'uselang' => 'content', + ]; + + /** Module names the hook derives from POPUPS_PARAMS. */ + private const POPUPS_MODULES = [ 'query', 'info', 'extracts', 'pageimages', 'revisions', 'info' ]; + + /** + * Build an anonymous user mock. + * + * @return \PHPUnit\Framework\MockObject\MockObject + */ + private function newAnonUserMock() { + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( false ); + return $user; + } + + /** + * Build a request mock that serves the given parameters. + * + * @param array $params + * @return \PHPUnit\Framework\MockObject\MockObject + */ + private function newApiRequestMock( array $params ) { + $request = $this->createMock( self::$webRequestClassName ); + $request->method( 'getVal' )->willReturnCallback( + static function ( $name, $default = null ) use ( $params ) { + return $params[$name] ?? $default; + } + ); + $request->method( 'getValues' )->willReturn( $params ); + return $request; + } + + /** + * @covers ::checkApiModules + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryAllowsPopupsRequest() { + // Arrange + $user = $this->newAnonUserMock(); + $request = $this->newApiRequestMock( self::POPUPS_PARAMS ); + $responseFactory = $this->createMock( ResponseFactory::class ); + $responseFactory->expects( $this->never() )->method( 'markDenied' ); + $service = $this->buildService( + [], [], [], $responseFactory, true, [], false, [ 'revisions' ] + ); + + // Act + $result = $service->checkApiModules( self::POPUPS_MODULES, $user, $request ); + + // Assert + $this->assertTrue( $result ); + } + + /** + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryAllowsSinglePageId() { + // Arrange + $params = self::POPUPS_PARAMS; + unset( $params['titles'] ); + $params['pageids'] = '42'; + $service = $this->buildService( [], [], [], null, true, [], false, [ 'revisions' ] ); + + // Act + $result = $service->checkApiModules( + self::POPUPS_MODULES, $this->newAnonUserMock(), $this->newApiRequestMock( $params ) + ); + + // Assert + $this->assertTrue( $result ); + } + + /** + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryAcceptsUnitSeparatorForm() { + // Arrange + $params = [ + 'action' => 'query', + 'prop' => "\x1finfo\x1frevisions", + 'rvprop' => "\x1ftimestamp", + 'titles' => "\x1fFoo|Bar", + ]; + $service = $this->buildService( [], [], [], null, true, [], false, [ 'revisions' ] ); + + // Act + $result = $service->checkApiModules( + [ 'query', 'info', 'revisions' ], $this->newAnonUserMock(), $this->newApiRequestMock( $params ) + ); + + // Assert + $this->assertTrue( $result, 'A "|" inside a "\x1f"-separated value is part of the single title' ); + } + + /** + * Every request here deviates from the Popups request in one respect that + * must keep the protected "revisions" module denied. + * + * @covers ::isRevisionMetadataQuery + * @dataProvider provideDeniedRevisionRequests + * + * @param array $overrides Parameters to set; null removes a parameter + */ + public function testRevisionMetadataQueryDeniesNonMetadataRequest( array $overrides ) { + // Arrange + $params = array_filter( + array_merge( self::POPUPS_PARAMS, $overrides ), + static function ( $value ) { + return $value !== null; + } + ); + $service = $this->buildService( [], [], [], null, true, [], false, [ 'revisions' ] ); + + // Act + $result = $service->checkApiModules( + self::POPUPS_MODULES, $this->newAnonUserMock(), $this->newApiRequestMock( $params ) + ); + + // Assert + $this->assertFalse( $result ); + } + + /** + * @return array + */ + public static function provideDeniedRevisionRequests(): array { + $cases = []; + + // Every rvprop value other than "timestamp", in MediaWiki 1.39 and 1.43. + $disallowedProps = [ + 'ids', 'flags', 'user', 'userid', 'size', 'slotsize', 'sha1', 'slotsha1', + 'contentmodel', 'comment', 'parsedcomment', 'content', 'tags', 'roles', 'parsetree', + ]; + foreach ( $disallowedProps as $prop ) { + $cases["rvprop=$prop"] = [ [ 'rvprop' => $prop ] ]; + $cases["rvprop=timestamp|$prop"] = [ [ 'rvprop' => "timestamp|$prop" ] ]; + } + $cases['rvprop=timestamp\x1fcontent'] = [ [ 'rvprop' => "\x1ftimestamp\x1fcontent" ] ]; + $cases['empty rvprop'] = [ [ 'rvprop' => '' ] ]; + $cases['rvprop with padding'] = [ [ 'rvprop' => ' timestamp' ] ]; + $cases['missing rvprop'] = [ [ 'rvprop' => null ] ]; + + // Every other parameter of ApiQueryRevisions(Base) in MediaWiki 1.39 and 1.43. + $pagingAndContentParams = [ + 'rvslots' => 'main', + 'rvlimit' => '50', + 'rvexpandtemplates' => '1', + 'rvgeneratexml' => '1', + 'rvparse' => '1', + 'rvsection' => '0', + 'rvdiffto' => 'prev', + 'rvdifftotext' => 'text', + 'rvdifftotextpst' => '1', + 'rvcontentformat' => 'text/x-wiki', + 'rvcontentformat-main' => 'text/x-wiki', + 'rvstartid' => '1', + 'rvendid' => '2', + 'rvstart' => '2020-01-01T00:00:00Z', + 'rvend' => '2021-01-01T00:00:00Z', + 'rvdir' => 'newer', + 'rvuser' => 'Example', + 'rvexcludeuser' => 'Example', + 'rvtag' => 'mw-reverted', + 'rvcontinue' => '20200101000000|1', + 'RVLIMIT' => '50', + ]; + foreach ( $pagingAndContentParams as $name => $value ) { + $cases[$name] = [ [ $name => $value ] ]; + } + + $cases['two titles'] = [ [ 'titles' => 'Foo|Bar' ] ]; + $cases['two titles, \x1f form'] = [ [ 'titles' => "\x1fFoo\x1fBar" ] ]; + $cases['empty titles'] = [ [ 'titles' => '' ] ]; + $cases['trailing separator'] = [ [ 'titles' => 'Foo|' ] ]; + $cases['no page'] = [ [ 'titles' => null ] ]; + $cases['two pageids'] = [ [ 'titles' => null, 'pageids' => '1|2' ] ]; + $cases['titles and pageids'] = [ [ 'pageids' => '1' ] ]; + $cases['revids'] = [ [ 'revids' => '1' ] ]; + $cases['generator'] = [ [ 'generator' => 'allpages' ] ]; + + return $cases; + } + + /** + * A generator turns the request into a multi-page query, so the hook + * passes the generator module as well. + * + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryDeniesGeneratorModule() { + // Arrange + $params = [ + 'action' => 'query', + 'prop' => 'revisions', + 'rvprop' => 'timestamp', + 'generator' => 'links', + 'titles' => 'Foo', + ]; + $service = $this->buildService( [], [], [], null, true, [], false, [ 'revisions' ] ); + + // Act + $result = $service->checkApiModules( + [ 'query', 'revisions', 'links' ], $this->newAnonUserMock(), $this->newApiRequestMock( $params ) + ); + + // Assert + $this->assertFalse( $result ); + } + + /** + * @covers ::isRevisionMetadataQuery + * @dataProvider provideOtherProtectedModules + * + * @param string[] $protectedModules + */ + public function testRevisionMetadataQueryDeniesOtherProtectedModule( array $protectedModules ) { + // Arrange + $service = $this->buildService( [], [], [], null, true, [], false, $protectedModules ); + + // Act + $result = $service->checkApiModules( + self::POPUPS_MODULES, $this->newAnonUserMock(), $this->newApiRequestMock( self::POPUPS_PARAMS ) + ); + + // Assert + $this->assertFalse( $result ); + } + + /** + * @return array + */ + public static function provideOtherProtectedModules(): array { + return [ + 'second protected sub-module' => [ [ 'revisions', 'extracts' ] ], + 'protected query action' => [ [ 'revisions', 'query' ] ], + 'only another sub-module' => [ [ 'pageimages' ] ], + ]; + } + + /** + * With the flag off, the Popups request is denied as before 1.8.0. + * + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryDisabledRestoresDenial() { + // Arrange + $service = $this->buildService( + [], [], [], null, true, [], false, [ 'revisions' ], [], false, [], false, false + ); + + // Act + $result = $service->checkApiModules( + self::POPUPS_MODULES, $this->newAnonUserMock(), $this->newApiRequestMock( self::POPUPS_PARAMS ) + ); + + // Assert + $this->assertFalse( $result ); + } + + /** + * The exemption only applies to action=query, not to a direct check of + * the "revisions" module name. + * + * @covers ::isRevisionMetadataQuery + */ + public function testRevisionMetadataQueryRequiresQueryAction() { + // Arrange + $service = $this->buildService( [], [], [], null, true, [], false, [ 'revisions' ] ); + + // Act + $result = $service->checkApiModule( + 'revisions', $this->newAnonUserMock(), $this->newApiRequestMock( self::POPUPS_PARAMS ) + ); + + // Assert + $this->assertFalse( $result ); + } + + /** + * Registered users are never denied, whether or not the flag is set and + * whatever revision props they request. + * + * @covers ::checkApiModules + * @dataProvider provideRevisionMetadataFlag + * + * @param bool $allowRevisionMetadata + */ + public function testRevisionMetadataQueryLeavesRegisteredUsersAlone( bool $allowRevisionMetadata ) { + // Arrange + $user = $this->createMock( self::$userClassName ); + $user->method( 'isRegistered' )->willReturn( true ); + $params = array_merge( self::POPUPS_PARAMS, [ 'rvprop' => 'content', 'rvlimit' => '50' ] ); + $service = $this->buildService( + [], [], [], null, true, [], false, [ 'revisions' ], [], false, [], false, $allowRevisionMetadata + ); + + // Act + $result = $service->checkApiModules( self::POPUPS_MODULES, $user, $this->newApiRequestMock( $params ) ); + + // Assert + $this->assertTrue( $result ); + } + + /** + * @return array + */ + public static function provideRevisionMetadataFlag(): array { + return [ + 'flag on' => [ true ], + 'flag off' => [ false ], + ]; + } + + /** + * CrawlerProtectionShouldDeny receives the decision after the exemption, + * and can still deny the request. + * + * @covers ::checkApiModules + */ + public function testRevisionMetadataQueryPassesFinalDecisionToHook() { + // Arrange + $seen = null; + $handler = static function ( $user, $request, $entryPoint, $specialPageName, &$shouldDeny ) use ( &$seen ) { + $seen = $shouldDeny; + $shouldDeny = true; + }; + $service = $this->buildService( + [], [], [], null, true, [], false, [ 'revisions' ], [], false, [ $handler ] + ); + + // Act + $result = $service->checkApiModules( + self::POPUPS_MODULES, $this->newAnonUserMock(), $this->newApiRequestMock( self::POPUPS_PARAMS ) + ); + + // Assert + $this->assertFalse( $seen, 'The hook must see the request as allowed' ); + $this->assertFalse( $result, 'The hook must be able to deny it anyway' ); + } + + /** + * @covers ::splitApiMultiValue + * @dataProvider provideApiMultiValues + * + * @param string $value + * @param string[] $expected + */ + public function testSplitApiMultiValue( string $value, array $expected ) { + // Act + $result = CrawlerProtectionService::splitApiMultiValue( $value ); + + // Assert + $this->assertSame( $expected, $result ); + } + + /** + * @return array + */ + public static function provideApiMultiValues(): array { + return [ + 'single value' => [ 'revisions', [ 'revisions' ] ], + 'pipe separated' => [ 'info|revisions', [ 'info', 'revisions' ] ], + 'unit separator' => [ "\x1finfo\x1frevisions", [ 'info', 'revisions' ] ], + 'pipe inside unit separator form' => [ "\x1fa|b", [ 'a|b' ] ], + 'empty' => [ '', [ '' ] ], + 'values kept as sent' => [ ' a||b ', [ ' a', '', 'b ' ] ], + ]; + } + // --------------------------------------------------------------- // isProtectedRestPath tests // ---------------------------------------------------------------