Merge changes I72e1a6dd,Ibb9d4c9a

* changes:
  Use Status object to report filter results.
  (bug 42064) AbuseFilter + EditFilterMergedContent
This commit is contained in:
Hoo man 2013-01-17 17:43:32 +00:00 committed by Gerrit Code Review
commit 24427e6d76
5 changed files with 236 additions and 79 deletions

View file

@ -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() . "<br />\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() . "<br />\n";
$rule_desc
);
}
break;
@ -1176,10 +1213,10 @@ class AbuseFilter {
$logParams, self::getFilterUser()
);
$display .= wfMessage(
$message = array(
'abusefilter-blocked-display',
array( $rule_desc )
)->parse() . "<br />\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() . "<br />\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() . "<br />\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() . "<br />\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() . "<br />\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;
}
}

View file

@ -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();
}
/**

View file

@ -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';

View file

@ -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'];

View file

@ -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