Cenarium has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/338508 )
Change subject: Fix display of descriptions with wikitext, avoid use of globals
......................................................................
Fix display of descriptions with wikitext, avoid use of globals
This fixes the display of filter descriptions containing wikitext,
which is useful in logs and such, but is broken at Special:AbuseFilter
and in warnings.
At Special:AbuseFilter, the wikitext is parsed then stripped of html
(this is how Echo outputs comments in notifications).
In warnings, a message is made and directly passed to the status.
This makes it no longer necessary to do the parse here with $wgOut.
So $wgTitle doesn't have to be set if empty anymore, though after
I53c82921595a9014b555e4ec468e5ba10454cd3a this should never be the
case.
Other uses of $wgUser and $wgOut are avoided where possible.
This also bumps the MW requirements to 1.27 since this uses
MediaWikiServices, and removes the AbortMove hook handler since
that hook was removed in 1.25.
Change-Id: I93ede02e5e6928604ff588cad0a45cba0bee261b
---
M AbuseFilter.hooks.php
M Views/AbuseFilterViewEdit.php
M Views/AbuseFilterViewExamine.php
M Views/AbuseFilterViewList.php
M Views/AbuseFilterViewTestBatch.php
M Views/AbuseFilterViewTools.php
M extension.json
M includes/AbuseFilter.class.php
8 files changed, 70 insertions(+), 113 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/AbuseFilter
refs/changes/08/338508/1
diff --git a/AbuseFilter.hooks.php b/AbuseFilter.hooks.php
index 2254f18..a10552c 100644
--- a/AbuseFilter.hooks.php
+++ b/AbuseFilter.hooks.php
@@ -118,7 +118,7 @@
$user, $title, $page, $summary, $content, $oldcontent,
$text
);
- $filter_result = AbuseFilter::filterAction( $vars, $title );
+ $filter_result = AbuseFilter::filterAction( $vars, $title,
'default', $user );
if ( !$filter_result->isOK() ) {
$status->merge( $filter_result );
@@ -337,37 +337,10 @@
$vars->setVar( 'SUMMARY', $reason );
$vars->setVar( 'ACTION', 'move' );
- $result = AbuseFilter::filterAction( $vars, $oldTitle );
+ $result = AbuseFilter::filterAction( $vars, $oldTitle,
'default', $user );
$status->merge( $result );
return $result->isOK();
- }
-
- /**
- * @param $oldTitle Title
- * @param $newTitle Title
- * @param $user User
- * @param $error
- * @param $reason
- * @return bool
- */
- public static function onAbortMove( $oldTitle, $newTitle, $user,
&$error, $reason ) {
- global $wgUser;
- // HACK: This is a secret userright so system actions
- // can bypass AbuseFilter. Should not be assigned to
- // normal users. This should be turned into a proper
- // userright in bug 67936.
- if ( $wgUser->isAllowed( 'abusefilter-bypass' ) ) {
- return true;
- }
-
- $status = new Status();
- self::onMovePageCheckPermissions( $oldTitle, $newTitle,
$wgUser, $reason, $status );
- if ( !$status->isOK() ) {
- $error = $status->getHTML();
- }
-
- return $status->isOK();
}
/**
@@ -389,7 +362,7 @@
$vars->setVar( 'SUMMARY', $reason );
$vars->setVar( 'ACTION', 'delete' );
- $filter_result = AbuseFilter::filterAction( $vars,
$article->getTitle() );
+ $filter_result = AbuseFilter::filterAction( $vars,
$article->getTitle(), $user );
$status->merge( $filter_result );
$error = $filter_result->isOK() ? '' :
$filter_result->getHTML();
@@ -424,7 +397,7 @@
$vars->setVar( 'ACCOUNTNAME', $user->getName() );
$filter_result = AbuseFilter::filterAction(
- $vars, SpecialPage::getTitleFor( 'Userlogin' ) );
+ $vars, SpecialPage::getTitleFor( 'Userlogin' ), $user );
$message = $filter_result->isOK() ? '' :
$filter_result->getWikiText();
@@ -753,6 +726,7 @@
* @param string $mime
* @param array|ApiMessage &$error
* @return bool
+ * @deprecated only used for 1.27 and older
*/
public static function onUploadVerifyFile( $upload, $mime, &$error ) {
global $wgUser, $wgVersion;
diff --git a/Views/AbuseFilterViewEdit.php b/Views/AbuseFilterViewEdit.php
index a8eddec..d10fbf9 100644
--- a/Views/AbuseFilterViewEdit.php
+++ b/Views/AbuseFilterViewEdit.php
@@ -411,7 +411,8 @@
$row->af_pattern,
'wpFilterRules',
true,
- $this->canEditFilter( $row )
+ $this->canEditFilter( $row ),
+ $this
);
$fields['abusefilter-edit-notes'] = Xml::textarea(
'wpFilterNotes',
diff --git a/Views/AbuseFilterViewExamine.php b/Views/AbuseFilterViewExamine.php
index e9b18cf..626032a 100644
--- a/Views/AbuseFilterViewExamine.php
+++ b/Views/AbuseFilterViewExamine.php
@@ -140,7 +140,8 @@
// Add test bit
if ( $this->getUser()->isAllowed( 'abusefilter-modify' ) ) {
$tester = Xml::tags( 'h2', null, $this->msg(
'abusefilter-examine-test' )->parse() );
- $tester .= AbuseFilter::buildEditBox(
$this->mTestFilter, 'wpTestFilter', false );
+ $tester .= AbuseFilter::buildEditBox(
$this->mTestFilter, 'wpTestFilter', false,
+ true, $this );
$tester .=
"\n" .
Xml::inputLabel(
diff --git a/Views/AbuseFilterViewList.php b/Views/AbuseFilterViewList.php
index ca4e519..f73c51f 100644
--- a/Views/AbuseFilterViewList.php
+++ b/Views/AbuseFilterViewList.php
@@ -275,9 +275,11 @@
$lang->formatNum( intval( $value ) )
);
case 'af_public_comments':
+ // parse the value then use the stripped-out
html
+ $html = $this->getOutput()->parseInline( $value
);
return $this->linkRenderer->makeLink(
SpecialPage::getTitleFor(
'AbuseFilter', intval( $row->af_id ) ),
- $value
+ trim( html_entity_decode( strip_tags(
$html ), ENT_QUOTES ) )
);
case 'af_actions':
$actions = explode( ',', $value );
diff --git a/Views/AbuseFilterViewTestBatch.php
b/Views/AbuseFilterViewTestBatch.php
index c7e271d..402c6ba 100644
--- a/Views/AbuseFilterViewTestBatch.php
+++ b/Views/AbuseFilterViewTestBatch.php
@@ -23,7 +23,8 @@
$out->addWikiMsg( 'abusefilter-test-intro', self::$mChangeLimit
);
$output = '';
- $output .= AbuseFilter::buildEditBox( $this->mFilter,
'wpTestFilter' ) . "\n";
+ $output .= AbuseFilter::buildEditBox( $this->mFilter,
'wpTestFilter', true, true, $this )
+ . "\n";
$output .=
Xml::inputLabel(
$this->msg( 'abusefilter-test-load-filter'
)->text(),
diff --git a/Views/AbuseFilterViewTools.php b/Views/AbuseFilterViewTools.php
index f07d508..8be4363 100644
--- a/Views/AbuseFilterViewTools.php
+++ b/Views/AbuseFilterViewTools.php
@@ -10,7 +10,7 @@
// Expression evaluator
$eval = '';
- $eval .= AbuseFilter::buildEditBox( '', 'wpTestExpr' );
+ $eval .= AbuseFilter::buildEditBox( '', 'wpTestExpr', true,
true, $this );
// Only let users with permission actually test it
if ( $user->isAllowed( 'abusefilter-modify' ) ) {
diff --git a/extension.json b/extension.json
index 0af7683..b99cf21 100644
--- a/extension.json
+++ b/extension.json
@@ -11,7 +11,7 @@
"license-name": "GPL-2.0+",
"type": "antispam",
"requires": {
- "MediaWiki": ">= 1.25.0"
+ "MediaWiki": ">= 1.27.0"
},
"AvailableRights": [
"abusefilter-modify",
diff --git a/includes/AbuseFilter.class.php b/includes/AbuseFilter.class.php
index 3686744..daf2ae8 100644
--- a/includes/AbuseFilter.class.php
+++ b/includes/AbuseFilter.class.php
@@ -739,12 +739,13 @@
* @param $filters array
* @param $title Title
* @param $vars AbuseFilterVariableHolder
+ * @param $user User
* @return Status returns the operation's status. $status->isOK() will
return true if
* there were no actions taken, false otherwise.
$status->getValue() will return
* an array listing the actions taken. $status-getErrors(),
etc, will provide
* the errors and warnings to be shown to the user to explain
the actions.
*/
- public static function executeFilterActions( $filters, $title, $vars ) {
+ private static function executeFilterActions( $filters, $title, $vars,
$user ) {
global $wgMainCacheType;
$actionsByFilter = self::getConsequencesForFilters( $filters );
@@ -752,14 +753,10 @@
$messages = array();
- global $wgOut, $wgAbuseFilterDisallowGlobalLocalBlocks,
$wgAbuseFilterRestrictions;
+ global $wgAbuseFilterDisallowGlobalLocalBlocks,
$wgAbuseFilterRestrictions;
foreach ( $actionsByFilter as $filter => $actions ) {
- // Special-case handling for warnings.
- $parsed_public_comments = $wgOut->parseInline(
- self::getFilter( $filter )->af_public_comments
- );
-
$global_filter = self::decodeGlobalName( $filter ) !==
false;
+ $desc = self::getFilter( $filter )->af_public_comments;
// If the filter is throttled and throttling is
available via object
// caching, check to see if the user has hit the
throttle.
@@ -773,7 +770,9 @@
// The rest are throttle-types.
foreach ( $parameters as $throttleType ) {
$hitThrottle = $hitThrottle ||
self::isThrottled(
- $throttleId,
$throttleType, $title, $rateCount, $ratePeriod, $global_filter );
+ $throttleId, $throttleType,
$title, $rateCount, $ratePeriod,
+ $global_filter, $user
+ );
}
unset( $actions['throttle'] );
@@ -805,7 +804,8 @@
} else {
$msg = 'abusefilter-warning';
}
- $messages[] = array( $msg,
$parsed_public_comments, $filter );
+ // pass filter description and id as
msg params
+ $messages[] = array( $msg, $desc,
$filter );
$actionsTaken[$filter][] = 'warn';
@@ -832,8 +832,9 @@
$info['parameters'],
$title,
$vars,
- self::getFilter( $filter
)->af_public_comments,
- $filter
+ $desc,
+ $filter,
+ $user
);
if ( $newMsg !== null ) {
@@ -861,7 +862,8 @@
$status = Status::newGood( $actionsTaken );
foreach ( $messages as $msg ) {
- call_user_func_array( array( $status, 'fatal' ), $msg );
+ $msg = call_user_func_array( 'wfMessage', $msg );
+ $status->fatal( $msg );
}
return $status;
@@ -878,18 +880,8 @@
public static function filterAction(
$vars, $title, $group = 'default', $user = null, $mode =
'execute'
) {
- global $wgUser, $wgTitle, $wgRequest;
-
- $context = RequestContext::getMain();
- $oldContextTitle = $context->getTitle();
-
- $oldWgTitle = $wgTitle;
-
- if ( !$wgTitle ) {
- $wgTitle = SpecialPage::getTitleFor( 'AbuseFilter' );
- }
-
- if ( !$user ) {
+ if ( $user === null ) {
+ global $wgUser;
$user = $wgUser;
}
@@ -945,13 +937,13 @@
if ( count( $matched_filters ) == 0 ) {
$status = Status::newGood();
} else {
- $status = self::executeFilterActions( $matched_filters,
$title, $vars );
+ $status = self::executeFilterActions( $matched_filters,
$title, $vars, $user );
$actions_taken = $status->value; // getValue() was
introduced only in 1.20
$action = $vars->getVar( 'ACTION' )->toString();
- // If $wgUser isn't safe to load (e.g. a failure during
+ // If $user isn't safe to load (e.g. a failure during
// AbortAutoAccount), create a dummy anonymous user
instead.
$user = $user->isSafeToLoad() ? $user : new User;
@@ -962,7 +954,7 @@
'afl_timestamp' => wfGetDB( DB_SLAVE
)->timestamp( wfTimestampNow() ),
'afl_namespace' => $title->getNamespace(),
'afl_title' => $title->getDBkey(),
- 'afl_ip' => $wgRequest->getIP()
+ 'afl_ip' => $user->getRequest()->getIP()
);
// Hack to avoid revealing IPs of people creating
accounts
@@ -971,17 +963,6 @@
}
self::addLogEntries( $actions_taken, $log_template,
$action, $vars, $group );
- }
-
- // Bug 53498: If we screwed around with $wgTitle, reset it so
the title
- // is correctly picked up from the request later. Do the same
for the
- // main RequestContext, because that might have picked up the
bogus
- // title from $wgTitle.
- if ( $wgTitle !== $oldWgTitle ) {
- $wgTitle = $oldWgTitle;
- }
- if ( $context->getTitle() !== $oldContextTitle &&
$oldContextTitle instanceof Title ) {
- $context->setTitle( $oldContextTitle );
}
return $status;
@@ -1054,7 +1035,7 @@
* @param string $group The filter's group (as defined in
$wgAbuseFilterValidGroups)
* @return mixed
*/
- public static function addLogEntries( $actions_taken, $log_template,
$action,
+ private static function addLogEntries( $actions_taken, $log_template,
$action,
$vars, $group = 'default'
) {
$dbw = wfGetDB( DB_MASTER );
@@ -1344,6 +1325,7 @@
* @param $vars AbuseFilterVariableHolder
* @param $rule_desc
* @param $rule_number int|string
+ * @param $user User
*
* @return array|null a message describing the action that was taken,
* or null if no action was taken. The message is given as an
array
@@ -1354,9 +1336,9 @@
* Status object from these messages, and before 1.21, Status did
* not accept Message objects to be added directly.
*/
- public static function takeConsequenceAction( $action, $parameters,
$title,
- $vars, $rule_desc, $rule_number ) {
- global $wgAbuseFilterCustomActionsHandlers, $wgRequest;
+ private static function takeConsequenceAction( $action, $parameters,
$title,
+ $vars, $rule_desc, $rule_number, $user ) {
+ global $wgAbuseFilterCustomActionsHandlers;
$message = null;
@@ -1375,8 +1357,8 @@
break;
case 'block':
- global $wgAbuseFilterBlockDuration,
$wgAbuseFilterAnonBlockDuration, $wgUser;
- if ( $wgUser->isAnon() &&
$wgAbuseFilterAnonBlockDuration !== null ) {
+ global $wgAbuseFilterBlockDuration,
$wgAbuseFilterAnonBlockDuration;
+ if ( $user->isAnon() &&
$wgAbuseFilterAnonBlockDuration !== null ) {
// The user isn't logged in and the
anon block duration
// doesn't default to
$wgAbuseFilterBlockDuration.
$expiry =
$wgAbuseFilterAnonBlockDuration;
@@ -1389,7 +1371,7 @@
'desc' => $rule_desc,
'number' => $rule_number
),
- $wgUser->getName(),
+ $user->getName(),
$expiry,
true
);
@@ -1406,7 +1388,7 @@
'desc' => $rule_desc,
'number' => $rule_number
),
- IP::sanitizeRange( $wgRequest->getIP()
. '/16' ),
+ IP::sanitizeRange(
$user->getRequest()->getIP() . '/16' ),
'1 week',
false
);
@@ -1418,13 +1400,12 @@
);
break;
case 'degroup':
- global $wgUser;
- if ( !$wgUser->isAnon() ) {
+ if ( !$user->isAnon() ) {
// Remove all groups from the user.
Ouch.
- $groups = $wgUser->getGroups();
+ $groups = $user->getGroups();
foreach ( $groups as $group ) {
- $wgUser->removeGroup( $group );
+ $user->removeGroup( $group );
}
$message = array(
@@ -1442,7 +1423,7 @@
$log = new LogPage( 'rights' );
$log->addEntry( 'rights',
- $wgUser->getUserPage(),
+ $user->getUserPage(),
wfMessage(
'abusefilter-degroupreason',
$rule_desc,
@@ -1458,11 +1439,10 @@
break;
case 'blockautopromote':
- global $wgUser;
- if ( !$wgUser->isAnon() ) {
+ if ( !$user->isAnon() ) {
$blockPeriod = (int)mt_rand( 3 * 86400,
7 * 86400 ); // Block for 3-7 days.
ObjectCache::getMainStashInstance()->set(
- self::autoPromoteBlockKey(
$wgUser ), true, $blockPeriod
+ self::autoPromoteBlockKey(
$user ), true, $blockPeriod
);
$message = array(
@@ -1479,10 +1459,8 @@
case 'tag':
// Mark with a tag on recentchanges.
- global $wgUser;
-
$actionID = implode( '-', array(
- $title->getPrefixedText(),
$wgUser->getName(),
+ $title->getPrefixedText(),
$user->getName(),
$vars->getVar( 'ACTION' )->toString()
) );
@@ -1583,13 +1561,14 @@
* @param $rateCount
* @param $ratePeriod
* @param $global bool
+ * @param $user User
* @return bool
*/
- public static function isThrottled( $throttleId, $types, $title,
$rateCount,
- $ratePeriod, $global = false
+ private static function isThrottled( $throttleId, $types, $title,
$rateCount,
+ $ratePeriod, $global = false, $user
) {
$stash = ObjectCache::getMainStashInstance();
- $key = self::throttleKey( $throttleId, $types, $title, $global
);
+ $key = self::throttleKey( $throttleId, $types, $title, $global,
$user );
$count = intval( $stash->get( $key ) );
wfDebugLog( 'AbuseFilter', "Got value $count for throttle key
$key\n" );
@@ -1618,28 +1597,27 @@
/**
* @param $type
* @param $title Title
+ * @param $user User
* @return int|string
*/
- public static function throttleIdentifier( $type, $title ) {
- global $wgUser, $wgRequest;
-
+ private static function throttleIdentifier( $type, $title, $user ) {
switch ( $type ) {
case 'ip':
- $identifier = $wgRequest->getIP();
+ $identifier = $user->getRequest()->getIP();
break;
case 'user':
- $identifier = $wgUser->getId();
+ $identifier = $user->getId();
break;
case 'range':
- $identifier = substr( IP::toHex(
$wgRequest->getIP() ), 0, 4 );
+ $identifier = substr( IP::toHex(
$user->getRequest()->getIP() ), 0, 4 );
break;
case 'creationdate':
- $reg = $wgUser->getRegistration();
+ $reg = $user->getRegistration();
$identifier = $reg - ( $reg % 86400 );
break;
case 'editcount':
// Hack for detecting different single-purpose
accounts.
- $identifier = $wgUser->getEditCount();
+ $identifier = $user->getEditCount();
break;
case 'site':
$identifier = 1;
@@ -1659,15 +1637,16 @@
* @param $type
* @param $title Title
* @param $global bool
+ * @param $user User
* @return String
*/
- public static function throttleKey( $throttleId, $type, $title, $global
= false ) {
+ private static function throttleKey( $throttleId, $type, $title,
$global = false, $user ) {
$types = explode( ',', $type );
$identifiers = array();
foreach ( $types as $subtype ) {
- $identifiers[] = self::throttleIdentifier( $subtype,
$title );
+ $identifiers[] = self::throttleIdentifier( $subtype,
$title, $user );
}
$identifier = sha1( implode( ':', $identifiers ) );
@@ -1886,20 +1865,19 @@
* @param $textName String
* @param $addResultDiv Boolean
* @param $canEdit Boolean
+ * @param $context IContextSource
* @return string
*/
- static function buildEditBox( $rules, $textName = 'wpFilterRules',
$addResultDiv = true,
- $canEdit = true ) {
- global $wgOut;
+ public static function buildEditBox( $rules, $textName =
'wpFilterRules', $addResultDiv = true,
+ $canEdit = true, $context ) {
$textareaAttrib = array( 'dir' => 'ltr' ); # Rules are in
English
if ( !$canEdit ) {
$textareaAttrib['readonly'] = 'readonly';
}
- global $wgUser;
$noTestAttrib = array();
- if ( !$wgUser->isAllowed( 'abusefilter-modify' ) ) {
+ if ( !$context->getUser()->isAllowed( 'abusefilter-modify' ) ) {
$noTestAttrib['disabled'] = 'disabled';
$addResultDiv = false;
}
@@ -1959,7 +1937,7 @@
}
// Add script
- $wgOut->addModules( 'ext.abuseFilter.edit' );
+ $context->getOutput()->addModules( 'ext.abuseFilter.edit' );
self::$editboxName = $textName;
return $rules;
--
To view, visit https://gerrit.wikimedia.org/r/338508
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I93ede02e5e6928604ff588cad0a45cba0bee261b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/AbuseFilter
Gerrit-Branch: master
Gerrit-Owner: Cenarium <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
