-
Notifications
You must be signed in to change notification settings - Fork 7
Configurable protected special pages list and optional quick request halting #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…roves fast deny logic
| "CrawlerProtectedSpecialPages": { | ||
| "value": [ | ||
| "recentchangeslinked", | ||
| "whatlinkshere" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add "mobilediff" too
| || in_array( 'Special:' . $special->getName(), $protectedSpecialPages, true ) | ||
| ) { | ||
| $out = $special->getContext()->getOutput(); | ||
| if ( $denyFast ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add unit tests to test this branch
| // allow forgiving entries in the setting array for Special pages names | ||
| in_array( $special->getName(), $protectedSpecialPages, true ) | ||
| || in_array( $name, $protectedSpecialPages, true ) | ||
| || in_array( 'Special:' . $special->getName(), $protectedSpecialPages, true ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refactor magic word "Special:" to a constant variable at the top of the file
| $config = MediaWikiServices::getInstance()->getMainConfig(); | ||
| $protectedSpecialPages = $config->get( 'CrawlerProtectedSpecialPages' ); | ||
| $denyFast = $config->get( 'CrawlerProtectedSpecialPages' ); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than having multiple checks, please add a line to get a version of $protectedSpecialPages which has been tolowercaseed and had Special: stripped from it.
For example:
$result = array_map(
fn($p) => ($p = strtolower($p)) && strpos($p, NS_SPECIAL_NAME) === 0
? substr($p, 8)
: $p,
$protectedSpecialPages
);| if ( in_array( $name, [ 'recentchangeslinked', 'whatlinkshere' ], true ) ) { | ||
| if ( | ||
| // allow forgiving entries in the setting array for Special pages names | ||
| in_array( $special->getName(), $protectedSpecialPages, true ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 3 lines will be redundant once the transformation is applied. Please remove the two extra lines
| * @return void | ||
| * @suppress PhanPluginNeverReturnMethod | ||
| */ | ||
| protected function denyAccessFast() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Deny access fast" is a subjective name which I don't think properly addresses why one might choose to use this. Naming is a hard problem to solve, so I do empathize. How about we change the 403 vs. 418 preference variable to $wgCrawlerProtectionUse418, and this function's name to denyAccessWith418()?
|
Also, could we use this PR to incorporate the per-feature blocking options in #6? |
|
This PR has been superseded by #12 |
This pull request introduces the following improvements:
$wgCrawlerProtectedSpecialPages$wgCrawlerProtectionDenyFasttoggle (turned off by default)