jenkins-bot has submitted this change and it was merged.
Change subject: Use custom error code for all edit and upload API responses
......................................................................
Use custom error code for all edit and upload API responses
The error code 'abusefilter-disallowed' or 'abusefilter-warning' is
used, depending on whether the filter only warns (and will allow the
action when retried) or prevents the user from performing the action.
The API response has been extended with some additional properties.
* For simple filters with no custom messages where the only action is
'disallow' or 'warn', the error code is the same as before.
* For filters with different actions, different error codes would
previously be returned; 'abusefilter-disallowed' will be returned
for them all, with the actions taken listed in the
.abusefilter.actions property of the API response.
* For filters with custom messages, the message key would previously
be used as the error code; now 'abusefilter-disallowed' or
'abusefilter-warning' is used, with the message available in the
.message property of the API response.
Also cleaned up some dead "forwards-compatibility" code and made a
recently introduced public method private.
The new functionality depends on Ifac8995a4d16d11840cee814177fc2808bc2072c
in MediaWiki core, older MediaWiki versions behave mostly as before.
The new .message property contains both the key and the parameters
duplicated from .abusefilter, so that the client doesn't have to know
what AbuseFilter is - it'll be able to just display the given
message with the given parameters. My specific use case is the upload
dialog in core (core shouldn't have to know about any extensions).
See also TitleBlacklist change I97c1f5c6bbbdfc0b8ea9914bb075d5299c14df8f.
Bug: T137961
Change-Id: I5780eae96930211191ecd874aacf53fdacb58f89
---
M AbuseFilter.class.php
M AbuseFilter.hooks.php
2 files changed, 52 insertions(+), 38 deletions(-)
Approvals:
Anomie: Looks good to me, but someone else must approve
MarkTraceur: Looks good to me, approved
jenkins-bot: Verified
diff --git a/AbuseFilter.class.php b/AbuseFilter.class.php
index 35d5958..b8f14a7 100644
--- a/AbuseFilter.class.php
+++ b/AbuseFilter.class.php
@@ -852,17 +852,12 @@
* @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 );
}
diff --git a/AbuseFilter.hooks.php b/AbuseFilter.hooks.php
index 122f1aa..b16139a 100644
--- a/AbuseFilter.hooks.php
+++ b/AbuseFilter.hooks.php
@@ -43,22 +43,21 @@
* @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
+ * @return bool Always true
*/
public static function onEditFilterMergedContent( IContextSource
$context, Content $content,
Status $status, $summary, User $user, $minoredit ) {
$text = AbuseFilter::contentToString( $content );
- $continue = self::filterEdit( $context, $content, $text,
$status, $summary, $minoredit );
+ $filterStatus = self::filterEdit( $context, $content, $text,
$status, $summary, $minoredit );
- if ( !$status->isOK() ) {
+ if ( !$filterStatus->isOK() ) {
// Produce a useful error message for API edits
- $status->apiHookResult = self::getEditApiResult(
$status );
+ $status->apiHookResult = self::getApiResult(
$filterStatus );
}
- return $continue;
+ return true;
}
/**
@@ -70,8 +69,7 @@
* @param Status $status Error message to return
* @param string $summary Edit summary for page
* @param bool $minoredit whether this is a minor edit according to the
user.
- *
- * @return bool
+ * @return Status
*/
public static function filterEdit( IContextSource $context, $content,
$text,
Status $status, $summary, $minoredit ) {
@@ -90,7 +88,7 @@
$page = $context->getWikiPage();
$revision = $page->getRevision();
if ( !$revision ) {
- return true;
+ return Status::newGood();
}
$oldcontent = $revision->getContent( Revision::RAW );
@@ -103,10 +101,10 @@
// Don't trigger for null edits.
if ( $content && isset( $oldcontent ) &&
$content->equals( $oldcontent ) ) {
// Compare Content objects if available
- return true;
+ return Status::newGood();
} elseif ( strcmp( $oldtext, $text ) == 0 ) {
// Otherwise, compare strings
- return true;
+ return Status::newGood();
}
} else {
$page = null;
@@ -121,13 +119,13 @@
if ( !$filter_result->isOK() ) {
$status->merge( $filter_result );
- return true; // re-show edit form
+ return $filter_result;
}
self::$successful_action_vars = $vars;
self::$last_edit_page = $page;
- return true;
+ return Status::newGood();
}
/**
@@ -159,31 +157,46 @@
}
/**
- * Implementation for EditFilterMergedContent hook.
- *
* @param Status $status Error message details
* @return array API result
*/
- public static function getEditApiResult( Status $status ) {
- $msg = $status->getErrorsArray()[0];
+ private static function getApiResult( Status $status ) {
+ global $wgFullyInitialised;
- // Use the error message key name as error code, the first
parameter is the filter description.
- if ( $msg instanceof Message ) {
- // For forward compatibility: In case we switch over
towards using Message objects someday.
- // (see the todo for AbuseFilter::buildStatus)
- $code = $msg->getKey();
- $filterDescription = $msg->getParams()[0];
- $warning = $msg->parse();
- } else {
- $code = array_shift( $msg );
- $filterDescription = $msg[0];
- $warning = wfMessage( $code )->params( $msg )->parse();
+ $params = $status->getErrorsArray()[0];
+ $key = array_shift( $params );
+
+ $warning = wfMessage( $key )->params( $params );
+ if ( !$wgFullyInitialised ) {
+ // This could happen for account autocreation checks
+ $warning = $warning->inContentLanguage();
}
+ $filterDescription = $params[0];
+ $filter = $params[1];
+
+ // The value is a nested structure keyed by filter id, which
doesn't make sense when we only
+ // return the result from one filter. Flatten it to a plain
array of actions.
+ $actionsTaken = array_values( array_unique(
+ call_user_func_array( 'array_merge', array_values(
$status->getValue() ) )
+ ) );
+ $code = ( $actionsTaken === [ 'warn' ] ) ?
'abusefilter-warning' : 'abusefilter-disallowed';
+
+ ApiResult::setIndexedTagName( $params, 'param' );
return array(
'code' => $code,
+ 'message' => array(
+ 'key' => $key,
+ 'params' => $params,
+ ),
+ 'abusefilter' => array(
+ 'id' => $filter,
+ 'description' => $filterDescription,
+ 'actions' => $actionsTaken,
+ ),
+ // For backwards-compatibility
'info' => 'Hit AbuseFilter: ' . $filterDescription,
- 'warning' => $warning
+ 'warning' => $warning->parse(),
);
}
@@ -692,7 +705,7 @@
* @param array $props
* @param string $comment
* @param string $pageText
- * @param array &$error
+ * @param array|ApiMessage &$error
* @return bool
*/
public static function onUploadVerifyUpload( UploadBase $upload, User
$user,
@@ -704,7 +717,7 @@
/**
* @param UploadBase $upload
* @param string $mime
- * @param array &$error
+ * @param array|ApiMessage &$error
* @return bool
*/
public static function onUploadVerifyFile( $upload, $mime, &$error ) {
@@ -732,7 +745,7 @@
* @param array $props File properties, as returned by
FSFile::getPropsFromPath()
* @param string|null $summary Upload log comment (also used as edit
summary)
* @param string|null $text File description page text (only used for
new uploads)
- * @param array &$error
+ * @param array|ApiMessage &$error
* @return bool
*/
public static function filterUpload( $action, UploadBase $upload, User
$user,
@@ -796,7 +809,13 @@
$filter_result = AbuseFilter::filterAction( $vars, $title );
if ( !$filter_result->isOK() ) {
- $error = $filter_result->getErrorsArray()[0];
+ $messageAndParams = $filter_result->getErrorsArray()[0];
+ $apiResult = self::getApiResult( $filter_result );
+ $error = ApiMessage::create(
+ $messageAndParams,
+ $apiResult['code'],
+ $apiResult
+ );
}
return $filter_result->isOK();
--
To view, visit https://gerrit.wikimedia.org/r/295314
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5780eae96930211191ecd874aacf53fdacb58f89
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/AbuseFilter
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Jackmcbarn <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: MarkTraceur <[email protected]>
Gerrit-Reviewer: Se4598 <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
