diff --git a/AbuseFilter.class.php b/AbuseFilter.class.php
index 74a512ac9..def6e57c7 100644
--- a/AbuseFilter.class.php
+++ b/AbuseFilter.class.php
@@ -676,12 +676,15 @@ class AbuseFilter {
}
/**
- * Returns an array [ list of actions taken by filter, error message to display, if any ]
+ * Executes a list of actions.
*
* @param $filters array
* @param $title Title
* @param $vars array
- * @return array
+ * @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 ) {
wfProfileIn( __METHOD__ );
@@ -743,10 +746,7 @@ class AbuseFilter {
} else {
$msg = 'abusefilter-warning';
}
- $messages[] = wfMessage(
- $msg,
- array( $parsed_public_comments, $filter )
- )->parse() . "
\n";
+ $messages[] = array( $msg, $parsed_public_comments, $filter );
$actionsTaken[$filter][] = 'warn';
@@ -772,22 +772,50 @@ class AbuseFilter {
self::$filters[$filter]->af_public_comments
);
- if ( $newMsg ) {
+ if ( $newMsg !== null ) {
$messages[] = $newMsg;
}
$actionsTaken[$filter][] = $action;
}
}
+ $status = self::buildStatus( $actionsTaken, $messages );
+
wfProfileOut( __METHOD__ );
- return array( $actionsTaken, implode( "\n", $messages ) );
+ return $status;
+ }
+
+ /**
+ * Constructs a Status object as returned by executeFilterActions() from the list of
+ * actions taken and the corresponding list of messages.
+ *
+ * @param array[] $actionsTaken associative array mapping each filter to the list if
+ * actions taken because of that filter.
+ * @param array[] $messages a list if arrays, where each array contains a message key
+ * followed by any message parameters.
+ *
+ * @todo: change this to accept Message objects. This is only possible from 1.21 onward,
+ * because before that, Status::fatal does not accept Message objects.
+ *
+ * @return Status
+ */
+ protected static function buildStatus( array $actionsTaken, array $messages ) {
+ $status = Status::newGood( $actionsTaken );
+
+ foreach ( $messages as $msg ) {
+ // Since MW 1.21, we could just pass Message objects, but in 1.20,
+ // we still have to rely on arrays.
+ call_user_func_array( array( $status, 'fatal' ), $msg );
+ }
+
+ return $status;
}
/**
* @param $vars AbuseFilterVariableHolder
* @param $title Title
* @param $group string
- * @return bool
+ * @return Status
*/
public static function filterAction( $vars, $title, $group = 'default' ) {
global $wgUser, $wgTitle, $wgRequest;
@@ -814,14 +842,16 @@ class AbuseFilter {
// Short-cut any remaining code if no filters were hit.
if ( count( $matched_filters ) == 0 ) {
wfProfileOut( __METHOD__ );
- return true;
+ return Status::newGood();
}
wfProfileIn( __METHOD__ . '-block' );
- list( $actions_taken, $error_msg ) = self::executeFilterActions(
+ $status = self::executeFilterActions(
$matched_filters, $title, $vars );
+ $actions_taken = $status->value; // getValue() was introduced only in 1.20
+
$action = $vars->getVar( 'ACTION' )->toString();
// Create a template
@@ -841,13 +871,11 @@ class AbuseFilter {
self::addLogEntries( $actions_taken, $log_template, $action, $vars, $group );
- $error_msg = $error_msg == '' ? true : $error_msg;
-
wfProfileOut( __METHOD__ . '-block' );
wfProfileOut( __METHOD__ );
- return $error_msg;
+ return $status;
}
/**
@@ -1118,24 +1146,33 @@ class AbuseFilter {
* @param $title Title
* @param $vars AbuseFilterVariableHolder
* @param $rule_desc
- * @return string
+ *
+ * @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
+ * containing the message key followed by any message parameters.
+ *
+ * @note: Returning the message as an array instead of a Message object is
+ * needed for compatibility with MW 1.20: we will be constructing a
+ * 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 )
{
global $wgAbuseFilterCustomActionsHandlers, $wgRequest;
- $display = '';
+ $message = null;
+
switch ( $action ) {
case 'disallow':
if ( strlen( $parameters[0] ) ) {
- $display .= wfMessage( $parameters[0], array( $rule_desc ) )->parse() . "\n";
+ $message = array( $parameters[0], $rule_desc );
} else {
// Generic message.
- $display .= wfMessage(
+ $message = array(
'abusefilter-disallowed',
- array( $rule_desc )
- )->parse() . "
\n";
+ $rule_desc
+ );
}
break;
@@ -1176,10 +1213,10 @@ class AbuseFilter {
$logParams, self::getFilterUser()
);
- $display .= wfMessage(
+ $message = array(
'abusefilter-blocked-display',
- array( $rule_desc )
- )->parse() . "
\n";
+ $rule_desc
+ );
break;
case 'rangeblock':
$filterUser = AbuseFilter::getFilterUser();
@@ -1213,10 +1250,10 @@ class AbuseFilter {
$logParams, self::getFilterUser()
);
- $display .= wfMessage(
+ $message = array(
'abusefilter-blocked-display',
$rule_desc
- )->parse() . "
\n";
+ );
break;
case 'degroup':
global $wgUser;
@@ -1228,10 +1265,10 @@ class AbuseFilter {
$wgUser->removeGroup( $group );
}
- $display .= wfMessage(
+ $message = array(
'abusefilter-degrouped',
- array( $rule_desc )
- )->parse() . "
\n";
+ $rule_desc
+ );
// Don't log it if there aren't any groups being removed!
if ( !count( $groups ) ) {
@@ -1259,10 +1296,10 @@ class AbuseFilter {
$blockPeriod = (int)mt_rand( 3 * 86400, 7 * 86400 ); // Block for 3-7 days.
$wgMemc->set( self::autoPromoteBlockKey( $wgUser ), true, $blockPeriod );
- $display .= wfMessage(
+ $message = array(
'abusefilter-autopromote-blocked',
- array( $rule_desc )
- )->parse() . "
\n";
+ $rule_desc
+ );
}
break;
@@ -1288,14 +1325,14 @@ class AbuseFilter {
$msg = call_user_func( $custom_function, $action, $parameters, $title, $vars, $rule_desc );
}
if( isset( $msg ) ) {
- $display .= wfMessage( $msg )->text() . "
\n";
+ $message = array( $msg );
}
} else {
wfDebugLog( 'AbuseFilter', "Unrecognised action $action" );
}
}
- return $display;
+ return $message;
}
/**
@@ -1875,12 +1912,17 @@ class AbuseFilter {
/**
* @param $title Title
- * @param $article Array|Article
+ * @param $article Page|null
* @return AbuseFilterVariableHolder
*/
- public static function getEditVars( $title, $article = null ) {
+ public static function getEditVars( $title, Page $page = null ) {
$vars = new AbuseFilterVariableHolder;
+ // NOTE: $page may end up remaining null, e.g. if $title points to a special page.
+ if ( !$page && $title->canExist() && $title->exists() ) {
+ $page = WikiPage::factory( $title );
+ }
+
$vars->setLazyLoadVar( 'edit_diff', 'diff',
array( 'oldtext-var' => 'old_wikitext', 'newtext-var' => 'new_wikitext' ) );
$vars->setLazyLoadVar( 'new_size', 'length', array( 'length-var' => 'new_wikitext' ) );
@@ -1900,7 +1942,7 @@ class AbuseFilter {
'namespace' => $title->getNamespace(),
'title' => $title->getText(),
'text-var' => 'new_wikitext',
- 'article' => $article
+ 'article' => $page
) );
$vars->setLazyLoadVar( 'old_links', 'links-from-wikitext-or-database',
array(
@@ -1918,7 +1960,7 @@ class AbuseFilter {
'namespace' => $title->getNamespace(),
'title' => $title->getText(),
'wikitext-var' => 'new_wikitext',
- 'article' => $article
+ 'article' => $page
) );
$vars->setLazyLoadVar( 'new_text', 'strip-html',
array( 'html-var' => 'new_html' ) );
@@ -2108,13 +2150,16 @@ class AbuseFilter {
* @return string|null the content of the revision as some kind of string,
* or an empty string if it can not be found
*/
- static function revisionToString( $revision, $audience = Revision::FOR_PUBLIC ) {
+ static function revisionToString( $revision, $audience = Revision::FOR_THIS_USER ) {
if ( !$revision instanceof Revision ) {
return '';
}
if ( defined( 'MW_SUPPORTS_CONTENTHANDLER' ) ) {
$content = $revision->getContent( $audience );
- $result = $content instanceof TextContent ? $content->getNativeData() : $content->getTextForSearchIndex();
+ if ( $content === null ) {
+ return '';
+ }
+ $result = self::contentToString( $content );
} else {
// For MediaWiki without contenthandler support (< 1.21)
$result = $revision->getText();
@@ -2122,4 +2167,37 @@ class AbuseFilter {
return $result;
}
+ /**
+ * Converts the given Content object to a string.
+ *
+ * This uses Content::getNativeData() if $content is an instance of TextContent,
+ * or Content::getTextForSearchIndex() otherwise.
+ *
+ * The hook 'AbuseFilter::contentToString' can be used to override this
+ * behavior.
+ *
+ * @param Content $content
+ *
+ * @return string a suitable string representation of the content.
+ */
+ static function contentToString( Content $content ) {
+ $text = null;
+
+ if ( wfRunHooks( 'AbuseFilter-contentToString', array( $content, &$text ) ) ) {
+ $text = $content instanceof TextContent
+ ? $content->getNativeData()
+ : $content->getTextForSearchIndex();
+ }
+
+ if ( is_string( $text ) ) {
+ // bug 20310
+ // XXX: Is this really needed? Should we rather apply PST?
+ $text = str_replace( "\r\n", "\n", $text );
+ } else {
+ $text = '';
+ }
+
+ return $text;
+ }
+
}
diff --git a/AbuseFilter.hooks.php b/AbuseFilter.hooks.php
index a50366d82..47bcf9c55 100644
--- a/AbuseFilter.hooks.php
+++ b/AbuseFilter.hooks.php
@@ -8,7 +8,7 @@ class AbuseFilterHooks {
// Hooray!
/**
- * Entry points for MediaWiki hook 'EditFilterMerged'
+ * Entry points for MediaWiki hook 'EditFilterMerged' (MW 1.20 and earlier)
*
* @param $editor EditPage instance (object)
* @param $text string Content of the edit box
@@ -17,63 +17,126 @@ class AbuseFilterHooks {
* @return bool
*/
public static function onEditFilterMerged( $editor, $text, &$error, $summary ) {
+ global $wgUser;
+
+ $context = $editor->mArticle->getContext();
+
+ $status = Status::newGood();
+ $minoredit = $editor->minoredit;
+
+ // poor man's PST, see bug 20310
+ $text = str_replace( "\r\n", "\n", $text );
+
+ $continue = self::filterEdit( $context, null, $text, $status, $summary, $wgUser, $minoredit );
+
+ if ( !$status->isOK() ) {
+ $error = $status->getWikiText();
+ }
+
+ return $continue;
+ }
+
+ /**
+ * Entry points for MediaWiki hook 'EditFilterMergedContent' (MW 1.21 and later)
+ *
+ * @param IContextSource $context the context of the edit
+ * @param Content $content the new Content generated by the edit
+ * @param Status $status Error message to return
+ * @param string $summary Edit summary for page
+ * @param User $user the user performing the edit
+ * @param bool $minoredit whether this is a minor edit according to the user.
+ *
+ * @return bool
+ */
+ public static function onEditFilterMergedContent( IContextSource $context, Content $content,
+ Status $status, $summary, User $user, $minoredit ) {
+
+ $text = AbuseFilter::contentToString( $content, Revision::RAW );
+
+ $continue = self::filterEdit( $context, $content, $text, $status, $summary, $user, $minoredit );
+ return $continue;
+ }
+
+ /**
+ * Common implementation for EditFilterMerged and EditFilterMergedContent hooks.
+ *
+ * @param IContextSource $context the context of the edit
+ * @param Content|null $content the new Content generated by the edit
+ * @param string $text new page content (subject of filtering)
+ * @param Status $status Error message to return
+ * @param string $summary Edit summary for page
+ * @param User $user the user performing the edit
+ * @param bool $minoredit whether this is a minor edit according to the user.
+ *
+ * @return bool
+ */
+ public static function filterEdit( IContextSource $context, $content, $text,
+ Status $status, $summary, User $user, $minoredit ) {
// Load vars
$vars = new AbuseFilterVariableHolder;
- # Replace line endings so the filter won't get confused as $text
- # was not processed by Parser::preSaveTransform (bug 20310)
- $text = str_replace( "\r\n", "\n", $text );
-
self::$successful_action_vars = false;
self::$last_edit_page = false;
// Check for null edits.
$oldtext = '';
+ $oldcontent = null;
- $article = $editor->getArticle();
- if ( $article->exists() ) {
+ $title = $context->getTitle();
+ if ( $title->canExist() && $title->exists() ) {
// Make sure we load the latest text saved in database (bug 31656)
- $revision = $article->getRevision();
+ $page = $context->getWikiPage();
+ $revision = $page->getRevision();
if ( !$revision ) {
return true;
}
- $oldtext = AbuseFilter::revisionToString( $revision, Revision::RAW );
+
+ if ( defined( 'MW_SUPPORTS_CONTENTHANDLER' ) ) {
+ $oldcontent = $revision->getContent( Revision::RAW );
+ $oldtext = AbuseFilter::contentToString( $oldcontent );
+ } else {
+ $oldtext = AbuseFilter::revisionToString( $revision, Revision::RAW );
+ }
+
+ // Cache article object so we can share a parse operation
+ $articleCacheKey = $title->getNamespace() . ':' . $title->getText();
+ AFComputedVariable::$articleCache[$articleCacheKey] = $page;
+ } else {
+ $page = null;
}
- // Cache article object so we can share a parse operation
- $title = $editor->mTitle;
- $articleCacheKey = $title->getNamespace() . ':' . $title->getText();
- AFComputedVariable::$articleCache[$articleCacheKey] = $article;
-
- if ( strcmp( $oldtext, $text ) == 0 ) {
- // Don't trigger for null edits.
+ // Don't trigger for null edits.
+ if ( $content && $oldcontent && $oldcontent->equals( $content ) ) {
+ // Compare Content objects if available
+ return true;
+ } else if ( strcmp( $oldtext, $text ) == 0 ) {
+ // Otherwise, compare strings
return true;
}
- global $wgUser;
- $vars->addHolder( AbuseFilter::generateUserVars( $wgUser ) );
+ $vars->addHolder( AbuseFilter::generateUserVars( $user ) );
$vars->addHolder( AbuseFilter::generateTitleVars( $title , 'article' ) );
$vars->setVar( 'action', 'edit' );
$vars->setVar( 'summary', $summary );
- $vars->setVar( 'minor_edit', $editor->minoredit );
+ $vars->setVar( 'minor_edit', $minoredit );
$vars->setVar( 'old_wikitext', $oldtext );
$vars->setVar( 'new_wikitext', $text );
- $vars->addHolder( AbuseFilter::getEditVars( $title, $article ) );
+ // TODO: set old_content and new_content vars, use them
+
+ $vars->addHolder( AbuseFilter::getEditVars( $title, $page ) );
$filter_result = AbuseFilter::filterAction( $vars, $title );
- if ( $filter_result !== true ) {
- global $wgOut;
- $wgOut->addHTML( $filter_result );
- $editor->showEditForm();
- return false;
+ if ( !$filter_result->isOK() ) {
+ $status->merge( $filter_result );
+ return true; // re-show edit form
}
self::$successful_action_vars = $vars;
- self::$last_edit_page = $editor->mArticle;
+ self::$last_edit_page = $page;
return true;
}
@@ -192,9 +255,8 @@ class AbuseFilterHooks {
$filter_result = AbuseFilter::filterAction( $vars, $oldTitle );
- $error = $filter_result;
-
- return $filter_result == '' || $filter_result === true;
+ $error = $filter_result->isOK() ? '' : $filter_result->getWikiText();
+ return $filter_result->isOK();
}
/**
@@ -202,9 +264,10 @@ class AbuseFilterHooks {
* @param $user User
* @param $reason string
* @param $error
+ * @param $status
* @return bool
*/
- public static function onArticleDelete( &$article, &$user, &$reason, &$error ) {
+ public static function onArticleDelete( &$article, &$user, &$reason, &$error, &$status ) {
$vars = new AbuseFilterVariableHolder;
global $wgUser;
@@ -215,9 +278,10 @@ class AbuseFilterHooks {
$filter_result = AbuseFilter::filterAction( $vars, $article->getTitle() );
- $error = $filter_result;
+ $status->merge( $filter_result );
+ $error = $filter_result->isOK() ? '' : $filter_result->getWikiText();
- return $filter_result == '' || $filter_result === true;
+ return $filter_result->isOK();
}
/**
@@ -248,9 +312,8 @@ class AbuseFilterHooks {
$filter_result = AbuseFilter::filterAction(
$vars, SpecialPage::getTitleFor( 'Userlogin' ) );
- $message = $filter_result;
-
- return $filter_result == '' || $filter_result === true;
+ $message = $filter_result->isOK() ? '' : $filter_result->getWikiText();
+ return $filter_result->isOK();
}
/**
@@ -450,11 +513,14 @@ class AbuseFilterHooks {
$filter_result = AbuseFilter::filterAction( $vars, $title );
- if ( is_string( $filter_result ) ) {
- $error = $filter_result;
- }
+ // XXX: HACK: return the first error, as an array containing the message key and
+ // any parameters.
+ // XXX: $errors may in the future also contain Message objects. Make sure UploadBase can
+ // deal with us returning that.
+ $errors = array_values( (array)$filter_result->getErrorsArray() );
+ $error = empty( $errors ) ? '' : $errors[0];
- return $filter_result == '' || $filter_result === true;
+ return $filter_result->isOK();
}
/**
diff --git a/AbuseFilter.php b/AbuseFilter.php
index 548240d9f..d5f0a76fc 100644
--- a/AbuseFilter.php
+++ b/AbuseFilter.php
@@ -68,7 +68,13 @@ $wgAPIModules['abusefilterunblockautopromote'] = 'ApiAbuseFilterUnblockAutopromo
$wgAutoloadClasses['ApiAbuseFilterCheckMatch'] = "$dir/api/ApiAbuseFilterCheckMatch.php";
$wgAPIModules['abusefiltercheckmatch'] = 'ApiAbuseFilterCheckMatch';
-$wgHooks['EditFilterMerged'][] = 'AbuseFilterHooks::onEditFilterMerged';
+
+if ( defined( 'MW_SUPPORTS_CONTENTHANDLER' ) ) {
+ $wgHooks['EditFilterMergedContent'][] = 'AbuseFilterHooks::onEditFilterMergedContent';
+} else {
+ $wgHooks['EditFilterMerged'][] = 'AbuseFilterHooks::onEditFilterMerged';
+}
+
$wgHooks['GetAutoPromoteGroups'][] = 'AbuseFilterHooks::onGetAutoPromoteGroups';
$wgHooks['AbortMove'][] = 'AbuseFilterHooks::onAbortMove';
$wgHooks['AbortNewAccount'][] = 'AbuseFilterHooks::onAbortNewAccount';
diff --git a/AbuseFilterVariableHolder.php b/AbuseFilterVariableHolder.php
index 7f3c4891b..9b897db47 100644
--- a/AbuseFilterVariableHolder.php
+++ b/AbuseFilterVariableHolder.php
@@ -347,6 +347,7 @@ class AFComputedVariable {
}
// Otherwise fall back to database
case 'parse-wikitext-nonedit':
+ // TODO: use Content object instead, if available! In any case, use WikiPage, not Article.
$article = self::articleFromTitle( $parameters['namespace'], $parameters['title'] );
$textVar = $parameters['wikitext-var'];
diff --git a/hooks.txt b/hooks.txt
index b465cb1e7..e3cd9c957 100644
--- a/hooks.txt
+++ b/hooks.txt
@@ -17,6 +17,12 @@ $vars: AbuseFilterVariableHolder
$parameters: Parameters with data to compute the value
&$result: Result of the computation
+'AbuseFilter-contentToString': Called when converting a Content object to a string to which
+filters can be applied. If the hook function returns true, Content::getTextForSearchIndex()
+will be used for non-text content.
+$content: The Content object
+&$text: Set this to the desired text.
+
'AbuseFilter-filterAction': Allows overwriting of abusefilter variables in AbuseFilter::filterAction just
before they're checked against filters.
$vars: AbuseFilterVariableHolder with variables