Skip to content

Commit bce77ed

Browse files
skizzerzskizzerz
authored
1.43 compatibility (#3)
* 1.43 compatibility - Use new HookHandler interface - Set up forwards-compat class aliases for older MW versions * Add CI * Address CI issues - Use tabs instead of spaces - Make unit tests use the updated hook interfaces * Fix tests phpunit mock methods configured with with() will disallow any parameters not specified in the with() call, which caused tests to fail when WebRequest::getVal was called multiple times with different parameters. Switch to willReturnMap() to properly map certain values to return values, while returning null for unmapped values. * Migrate to proper unit test This requires moving to a unit/ subdirectory to indicate that we do not need to load MW when running these tests. Additionally, instead of checking that we call various methods on OutputPage when denying access, refactor denyAccess to a protected method and mock it during testing, so that we can ensure denyAccess is called without running any of its code. The code itself requires the services container to be instantiated in order for wfMessage()->plain() to work, which we cannot do during pure unit tests. We should probably also add some integration tests to verify that denyAccess() behaves as expected, in addition to tests that exercise other aspects of the extension.
1 parent f0e57d0 commit bce77ed

File tree

6 files changed

+424
-154
lines changed

6 files changed

+424
-154
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
name: Continuous Integration
2+
3+
permissions:
4+
contents: read
5+
6+
on:
7+
push:
8+
branches:
9+
- main
10+
pull_request:
11+
12+
env:
13+
EXTNAME: CrawlerProtection
14+
MW_INSTALL_PATH: ${{ github.workspace }}
15+
16+
jobs:
17+
style:
18+
name: Code Style
19+
runs-on: ${{ matrix.os }}
20+
strategy:
21+
fail-fast: false
22+
matrix:
23+
os: [ ubuntu-latest ]
24+
php: [ '8.2', '8.3', '8.4' ]
25+
mediawiki: [ REL1_43 ]
26+
include:
27+
- os: ubuntu-latest
28+
php: '7.4'
29+
mediawiki: REL1_35
30+
- os: ubuntu-latest
31+
php: '7.4'
32+
mediawiki: REL1_39
33+
- os: ubuntu-latest
34+
php: '8.1'
35+
mediawiki: REL1_39
36+
steps:
37+
- name: Setup PHP
38+
uses: shivammathur/setup-php@v2
39+
with:
40+
php-version: ${{ matrix.php }}
41+
extensions: mbstring, intl
42+
coverage: none
43+
tools: composer
44+
- name: Setup MediaWiki
45+
uses: actions/checkout@v4
46+
with:
47+
repository: wikimedia/mediawiki
48+
ref: ${{ matrix.mediawiki }}
49+
- name: Setup Extension
50+
uses: actions/checkout@v4
51+
with:
52+
path: extensions/${{ env.EXTNAME }}
53+
- name: Setup Composer
54+
run: |
55+
echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
56+
composer update
57+
composer update
58+
- name: Lint
59+
run: ./vendor/bin/parallel-lint --exclude node_modules --exclude vendor extensions/${{ env.EXTNAME }}
60+
- name: PHP Code Sniffer
61+
run: ./vendor/bin/phpcs -sp --standard=vendor/mediawiki/mediawiki-codesniffer/MediaWiki extensions/${{ env.EXTNAME }}
62+
63+
security:
64+
name: Static Analysis
65+
runs-on: ${{ matrix.os }}
66+
strategy:
67+
fail-fast: false
68+
matrix:
69+
os: [ ubuntu-latest ]
70+
# 1.43 phan is broken on php 8.4
71+
php: [ '8.2', '8.3' ]
72+
mediawiki: [ REL1_43 ]
73+
include:
74+
- os: ubuntu-latest
75+
php: '7.4'
76+
mediawiki: REL1_35
77+
- os: ubuntu-latest
78+
php: '7.4'
79+
mediawiki: REL1_39
80+
- os: ubuntu-latest
81+
php: '8.1'
82+
mediawiki: REL1_39
83+
steps:
84+
- name: Setup PHP
85+
uses: shivammathur/setup-php@v2
86+
with:
87+
php-version: ${{ matrix.php }}
88+
extensions: mbstring, intl, ast
89+
coverage: none
90+
tools: composer
91+
- name: Setup MediaWiki
92+
uses: actions/checkout@v4
93+
with:
94+
repository: wikimedia/mediawiki
95+
ref: ${{ matrix.mediawiki }}
96+
- name: Setup Extension
97+
uses: actions/checkout@v4
98+
with:
99+
path: extensions/${{ env.EXTNAME }}
100+
- name: Setup Composer
101+
run: |
102+
echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
103+
composer update
104+
composer update
105+
- name: Phan
106+
run: ./vendor/bin/phan -d extensions/${{ env.EXTNAME }} --minimum-target-php-version=7.4 --long-progress-bar
107+
phpunit:
108+
name: Unit Tests
109+
runs-on: ${{ matrix.os }}
110+
strategy:
111+
fail-fast: false
112+
matrix:
113+
os: [ ubuntu-latest ]
114+
php: [ '8.2', '8.3', '8.4' ]
115+
mediawiki: [ REL1_43 ]
116+
include:
117+
- os: ubuntu-latest
118+
php: '7.4'
119+
mediawiki: REL1_35
120+
- os: ubuntu-latest
121+
php: '7.4'
122+
mediawiki: REL1_39
123+
- os: ubuntu-latest
124+
php: '8.2'
125+
mediawiki: REL1_39
126+
steps:
127+
- name: Setup PHP
128+
uses: shivammathur/setup-php@v2
129+
with:
130+
php-version: ${{ matrix.php }}
131+
extensions: mbstring, intl, ast
132+
coverage: none
133+
tools: composer
134+
- name: Setup MediaWiki
135+
uses: actions/checkout@v4
136+
with:
137+
repository: wikimedia/mediawiki
138+
ref: ${{ matrix.mediawiki }}
139+
- name: Setup Extension
140+
uses: actions/checkout@v4
141+
with:
142+
path: extensions/${{ env.EXTNAME }}
143+
- name: Setup Composer
144+
run: |
145+
echo '{"extra":{"merge-plugin":{"include":["extensions/*/composer.json","skins/*/composer.json"]}}}' > composer.local.json
146+
composer update
147+
composer update
148+
- name: Install MediaWiki
149+
run: php maintenance/install.php --dbtype=sqlite --with-extensions --pass=UnitTestingAdminPassword519 UnitTesting WikiAdmin
150+
- name: Phpunit
151+
run: ./vendor/bin/phpunit -- extensions/${{ env.EXTNAME }}/tests/phpunit

‎.phan/config.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
// use phan config shipped with mediawiki core
4+
$cfg = require __DIR__ . '/../../../vendor/mediawiki/mediawiki-phan-config/src/config.php';
5+
6+
// we suppress things for backwards compat reasons, so suppressions may not apply to all phan test runs
7+
// as part of dropping support for old versions, old suppressions will be removed
8+
$cfg['suppress_issue_types'][] = 'UnusedSuppression';
9+
$cfg['suppress_issue_types'][] = 'UnusedPluginSuppression';
10+
11+
// we make use of class aliases for backwards compat, but phan doesn't honor version checks surrounding them
12+
$cfg['suppress_issue_types'][] = 'PhanUndeclaredClassAliasOriginal';
13+
$cfg['suppress_issue_types'][] = 'PhanRedefineClassAlias';
14+
15+
return $cfg;

‎extension.json‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,15 @@
1010
"AutoloadNamespaces": {
1111
"MediaWiki\\Extension\\CrawlerProtection\\": "includes/"
1212
},
13+
"HookHandlers": {
14+
"main": {
15+
"class": "MediaWiki\\Extension\\CrawlerProtection\\Hooks",
16+
"services": []
17+
}
18+
},
1319
"Hooks": {
14-
"MediaWikiPerformAction": "MediaWiki\\Extension\\CrawlerProtection\\Hooks::onMediaWikiPerformAction",
15-
"SpecialPageBeforeExecute": "MediaWiki\\Extension\\CrawlerProtection\\Hooks::onSpecialPageBeforeExecute"
20+
"MediaWikiPerformAction": "main",
21+
"SpecialPageBeforeExecute": "main"
1622
},
1723
"license-name": "MIT",
1824
"Tests": {

‎includes/Hooks.php‎

Lines changed: 114 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -2,95 +2,125 @@
22

33
namespace MediaWiki\Extension\CrawlerProtection;
44

5-
use OutputPage;
6-
use Article;
7-
use Title;
8-
use User;
9-
use WebRequest;
10-
use MediaWiki;
11-
use SpecialPage;
12-
use RequestContext;
5+
// Class aliases for multi-version compatibility.
6+
// These need to be in global scope so phan can pick up on them,
7+
// and before any use statements that make use of the namespaced names.
8+
if ( version_compare( MW_VERSION, '1.39.4', '<' ) ) {
9+
class_alias( '\Title', '\MediaWiki\Title\Title' );
10+
}
11+
12+
if ( version_compare( MW_VERSION, '1.41', '<' ) ) {
13+
class_alias( '\OutputPage', '\MediaWiki\Output\OutputPage' );
14+
class_alias( '\SpecialPage', '\MediaWiki\SpecialPage\SpecialPage' );
15+
class_alias( '\User', '\MediaWiki\User\User' );
16+
class_alias( '\WebRequest', '\MediaWiki\Request\WebRequest' );
17+
}
18+
19+
if ( version_compare( MW_VERSION, '1.42', '<' ) ) {
20+
class_alias( '\MediaWiki', '\MediaWiki\Actions\ActionEntryPoint' );
21+
}
22+
23+
if ( version_compare( MW_VERSION, '1.44', '<' ) ) {
24+
class_alias( '\Article', '\MediaWiki\Page\Article' );
25+
}
26+
27+
use MediaWiki\Actions\ActionEntryPoint;
28+
use MediaWiki\Hook\MediaWikiPerformActionHook;
29+
use MediaWiki\Output\OutputPage;
30+
use MediaWiki\Page\Article;
31+
use MediaWiki\Request\WebRequest;
32+
use MediaWiki\SpecialPage\Hook\SpecialPageBeforeExecuteHook;
33+
use MediaWiki\SpecialPage\SpecialPage;
34+
use MediaWiki\Title\Title;
35+
use MediaWiki\User\User;
36+
37+
class Hooks implements MediaWikiPerformActionHook, SpecialPageBeforeExecuteHook {
38+
/**
39+
* Block sensitive page views for anonymous users via MediaWikiPerformAction.
40+
* Handles:
41+
* - ?type=revision
42+
* - ?action=history
43+
* - ?diff=1234
44+
* - ?oldid=1234
45+
*
46+
* Special pages (e.g. Special:WhatLinksHere) are handled separately.
47+
*
48+
* @param OutputPage $output
49+
* @param Article $article
50+
* @param Title $title
51+
* @param User $user
52+
* @param WebRequest $request
53+
* @param ActionEntryPoint $mediaWiki
54+
* @return bool False to abort further action
55+
*/
56+
public function onMediaWikiPerformAction(
57+
$output,
58+
$article,
59+
$title,
60+
$user,
61+
$request,
62+
$mediaWiki
63+
) {
64+
$type = $request->getVal( 'type' );
65+
$action = $request->getVal( 'action' );
66+
$diffId = (int)$request->getVal( 'diff' );
67+
$oldId = (int)$request->getVal( 'oldid' );
1368

14-
class Hooks {
15-
/**
16-
* Block sensitive page views for anonymous users via MediaWikiPerformAction.
17-
* Handles:
18-
* - ?type=revision
19-
* - ?action=history
20-
* - ?diff=1234
21-
* - ?oldid=1234
22-
*
23-
* Special pages (e.g. Special:WhatLinksHere) are handled separately.
24-
*
25-
* @param OutputPage $output
26-
* @param Article $article
27-
* @param Title $title
28-
* @param User $user
29-
* @param WebRequest $request
30-
* @param MediaWiki $wiki
31-
* @return bool False to abort further action
32-
*/
33-
public static function onMediaWikiPerformAction(
34-
OutputPage $output,
35-
Article $article,
36-
Title $title,
37-
User $user,
38-
WebRequest $request,
39-
MediaWiki $wiki
40-
) {
41-
$type = $request->getVal( 'type' );
42-
$action = $request->getVal( 'action' );
43-
$diffId = (int)$request->getVal( 'diff' );
44-
$oldId = (int)$request->getVal( 'oldid' );
69+
if (
70+
!$user->isRegistered()
71+
&& (
72+
$type === 'revision'
73+
|| $action === 'history'
74+
|| $diffId > 0
75+
|| $oldId > 0
76+
)
77+
) {
78+
$this->denyAccess( $output );
79+
return false;
80+
}
4581

46-
if (
47-
!$user->isRegistered()
48-
&& (
49-
$type === 'revision'
50-
|| $action === 'history'
51-
|| $diffId > 0
52-
|| $oldId > 0
53-
)
54-
) {
55-
self::denyAccess( $output );
56-
return false;
57-
}
82+
return true;
83+
}
5884

59-
return true;
60-
}
85+
/**
86+
* Block Special:RecentChangesLinked and Special:WhatLinksHere for anonymous users.
87+
*
88+
* @param SpecialPage $special
89+
* @param string|null $subPage
90+
* @return bool False to abort execution
91+
*/
92+
public function onSpecialPageBeforeExecute( $special, $subPage ) {
93+
$user = $special->getContext()->getUser();
94+
if ( $user->isRegistered() ) {
95+
// logged-in users: allow
96+
return true;
97+
}
6198

62-
/**
63-
* Block Special:RecentChangesLinked and Special:WhatLinksHere for anonymous users.
64-
*
65-
* @param SpecialPage $specialPage
66-
* @param string $subPage
67-
* @return bool False to abort execution
68-
*/
69-
public static function onSpecialPageBeforeExecute( SpecialPage $specialPage, $subPage ) {
70-
$user = $specialPage->getContext()->getUser();
71-
if ( $user->isRegistered() ) {
72-
return true; // logged-in users: allow
73-
}
99+
$name = strtolower( $special->getName() );
100+
if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) {
101+
$out = $special->getContext()->getOutput();
102+
$this->denyAccess( $out );
103+
return false;
104+
}
74105

75-
$name = strtolower( $specialPage->getName() );
76-
if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) {
77-
$out = $specialPage->getContext()->getOutput();
78-
self::denyAccess( $out );
79-
return false;
80-
}
106+
return true;
107+
}
81108

82-
return true;
83-
}
109+
/**
110+
* Helper: output 403 Access Denied page using i18n messages.
111+
*
112+
* @param OutputPage $output
113+
* @return void
114+
*/
115+
protected function denyAccess( OutputPage $output ): void {
116+
$output->setStatusCode( 403 );
117+
$output->addWikiTextAsInterface( wfMessage( 'crawlerprotection-accessdenied-text' )->plain() );
84118

85-
/**
86-
* Helper: output 403 Access Denied page using i18n messages.
87-
*
88-
* @param OutputPage $output
89-
* @return void
90-
*/
91-
private static function denyAccess( OutputPage $output ): void {
92-
$output->setStatusCode( 403 );
93-
$output->setPageTitle( wfMessage( 'crawlerprotection-accessdenied-title' )->text() );
94-
$output->addWikiTextAsInterface( wfMessage( 'crawlerprotection-accessdenied-text' )->text() );
95-
}
119+
if ( version_compare( MW_VERSION, '1.41', '<' ) ) {
120+
$output->setPageTitle( wfMessage( 'crawlerprotection-accessdenied-title' ) );
121+
} else {
122+
// @phan-suppress-next-line PhanUndeclaredMethod Exists in 1.41+
123+
$output->setPageTitleMsg( wfMessage( 'crawlerprotection-accessdenied-title' ) );
124+
}
125+
}
96126
}

0 commit comments

Comments
 (0)