Merge "Change throttle selector to restore old functionality, overall improvement"

This commit is contained in:
jenkins-bot 2018-11-15 00:58:11 +00:00 committed by Gerrit Code Review
commit 213c2aa011
9 changed files with 700 additions and 135 deletions

View file

@ -150,7 +150,8 @@
"ApiAbuseFilterCheckSyntax": "includes/api/ApiAbuseFilterCheckSyntax.php",
"ApiAbuseFilterEvalExpression": "includes/api/ApiAbuseFilterEvalExpression.php",
"ApiAbuseFilterUnblockAutopromote": "includes/api/ApiAbuseFilterUnblockAutopromote.php",
"ApiAbuseFilterCheckMatch": "includes/api/ApiAbuseFilterCheckMatch.php"
"ApiAbuseFilterCheckMatch": "includes/api/ApiAbuseFilterCheckMatch.php",
"NormalizeThrottleParameters": "maintenance/normalizeThrottleParameters.php"
},
"ResourceModules": {
"ext.abuseFilter": {
@ -162,6 +163,8 @@
"abusefilter-edit-syntaxok",
"abusefilter-edit-syntaxerr",
"abusefilter-http-error",
"abusefilter-edit-throttle-placeholder",
"abusefilter-edit-tag-placeholder",
"unknown-error"
],
"dependencies": [

View file

@ -201,13 +201,15 @@
"abusefilter-edit-throttle-count": "Number of actions to allow:",
"abusefilter-edit-throttle-period": "Period of time (in seconds):",
"abusefilter-edit-throttle-groups": "Group throttle by:",
"abusefilter-edit-throttle-ip": "IP address",
"abusefilter-edit-throttle-user": "User account",
"abusefilter-edit-throttle-range": "/16 range",
"abusefilter-edit-throttle-creationdate": "Server time of account creation",
"abusefilter-edit-throttle-editcount": "Edit count",
"abusefilter-edit-throttle-site": "The whole site",
"abusefilter-edit-throttle-page": "Page",
"abusefilter-edit-throttle-hidden-placeholder": "Split with commas to join with AND, and with linebreaks to join with OR",
"abusefilter-edit-throttle-placeholder": "Split with commas to join with AND, and insert one by one to join with OR",
"abusefilter-throttle-ip": "IP address",
"abusefilter-throttle-user": "user account",
"abusefilter-throttle-range": "/16 range",
"abusefilter-throttle-creationdate": "account creation date",
"abusefilter-throttle-editcount": "edit count",
"abusefilter-throttle-site": "whole site",
"abusefilter-throttle-page": "page",
"abusefilter-throttle-details": "Allow $1 {{PLURAL:$1|action|actions}} every $2 {{PLURAL:$2|second|seconds}}, group throttle by: $3",
"abusefilter-edit-warn-message": "System message to use for warning:",
"abusefilter-edit-warn-other": "Other message",
@ -251,6 +253,11 @@
"abusefilter-edit-notallowed": "You are not permitted to create or edit abuse filters",
"abusefilter-edit-notallowed-global": "You are not permitted to create or edit global abuse filters",
"abusefilter-edit-notallowed-global-custom-msg": "Custom warning messages are not supported for global filters",
"abusefilter-edit-invalid-throttlecount": "The throttle action count must be a positive integer.",
"abusefilter-edit-invalid-throttleperiod": "The throttle period must be a positive integer.",
"abusefilter-edit-empty-throttlegroups": "At least one throttle group must be selected.",
"abusefilter-edit-duplicated-throttlegroups": "Throttle groups cannot have duplicates.",
"abusefilter-edit-invalid-throttlegroups": "The specified throttle groups are not valid.",
"abusefilter-edit-builder-select": "Select an option to add it at the cursor",
"abusefilter-edit-builder-group-op-arithmetic": "Arithmetic operators",
"abusefilter-edit-builder-op-arithmetic-addition": "Addition (+)",

View file

@ -234,13 +234,15 @@
"abusefilter-edit-throttle-count": "Field label for entering the number of allowed hits before triggering the filter consequences.",
"abusefilter-edit-throttle-period": "Field label for entering a time period in seconds.\n{{Identical|Second}}",
"abusefilter-edit-throttle-groups": "Field label for properties to group throttle counts by (for example IP address and username). Throttling is the concept of limiting occurrences of a certain action in a given time frame.",
"abusefilter-edit-throttle-ip": "Checkbox with a throttle group",
"abusefilter-edit-throttle-user": "Checkbox with a throttle group",
"abusefilter-edit-throttle-range": "Checkbox with a throttle group",
"abusefilter-edit-throttle-creationdate": "Checkbox with a throttle group",
"abusefilter-edit-throttle-editcount": "Checkbox with a throttle group",
"abusefilter-edit-throttle-site": "Checkbox with a throttle group",
"abusefilter-edit-throttle-page": "Checkbox with a throttle group",
"abusefilter-edit-throttle-hidden-placeholder": "Label for a textarea where users may insert throttling criteria.",
"abusefilter-edit-throttle-placeholder": "Label for an input field where users may insert throttling criteria.",
"abusefilter-throttle-ip": "Throttle option.",
"abusefilter-throttle-user": "Throttle option.",
"abusefilter-throttle-range": "Throttle option.",
"abusefilter-throttle-creationdate": "Throttle option.",
"abusefilter-throttle-editcount": "Throttle option.",
"abusefilter-throttle-site": "Throttle option.",
"abusefilter-throttle-page": "Throttle option.",
"abusefilter-throttle-details": "Description for Special:AbuseFilter/history with a detailed description for throttle action. Parameters:\n* $1 is the number of allowed actions, $2 is the time in seconds to use before resetting the action count, $3 is a list of throttled groups",
"abusefilter-edit-warn-message": "Field label for dropdown list with system messages.",
"abusefilter-edit-warn-other": "Option in dropdown menu to specify no item from the list should be used.\n\nSee also:\n* {{msg-mw|abusefilter-edit-disallow-other}}",
@ -284,6 +286,11 @@
"abusefilter-edit-notallowed": "Error message when trying to modify a filter while not allowed.",
"abusefilter-edit-notallowed-global": "Error message when trying to modify a global filter while not allowed.",
"abusefilter-edit-notallowed-global-custom-msg": "Error message when trying to add a custom warning message to a global filter, which is not allowed.",
"abusefilter-edit-invalid-throttlecount": "Error message when trying to provide an invalid action count for \"throttle\" action.",
"abusefilter-edit-invalid-throttleperiod": "Error message when trying to provide an invalid time period for \"throttle\" action.",
"abusefilter-edit-empty-throttlegroups": "Error message when trying to save a filter with \"throttle\" action enabled but no throttle groups selected.",
"abusefilter-edit-duplicated-throttlegroups": "Error message when trying to save a filter with \"throttle\" action enabled and duplicated throttle groups.",
"abusefilter-edit-invalid-throttlegroups": "Error message when trying to save a filter with \"throttle\" action enabled and invalid throttle groups.",
"abusefilter-edit-builder-select": "Default value for dropdown menu that allows inserting abuse filter syntax in the filter definition field.",
"abusefilter-edit-builder-group-op-arithmetic": "Group entry in dropdown menu.",
"abusefilter-edit-builder-op-arithmetic-addition": "Abuse filter syntax option in a dropdown from the group {{msg-mw|abusefilter-edit-builder-group-op-arithmetic}}.",

View file

@ -2214,6 +2214,76 @@ class AbuseFilter {
return $finalStatus;
}
/**
* Validate throttle parameters
*
* @param array $params Throttle parameters
* @return null|string Null on success, a string with the error message on failure
*/
public static function checkThrottleParameters( $params ) {
$throttleRate = explode( ',', $params[1] );
$throttleCount = $throttleRate[0];
$throttlePeriod = $throttleRate[1];
$throttleGroups = array_slice( $params, 2 );
$validGroups = [
'ip',
'user',
'range',
'creationdate',
'editcount',
'site',
'page'
];
$error = null;
if ( preg_match( '/^[1-9][0-9]*$/', $throttleCount ) === 0 ) {
$error = 'abusefilter-edit-invalid-throttlecount';
} elseif ( preg_match( '/^[1-9][0-9]*$/', $throttlePeriod ) === 0 ) {
$error = 'abusefilter-edit-invalid-throttleperiod';
} elseif ( !$throttleGroups ) {
$error = 'abusefilter-edit-empty-throttlegroups';
} else {
$valid = true;
// Groups should be unique in three ways: no direct duplicates like 'user' and 'user',
// no duplicated subgroups, not even shuffled ('ip,user' and 'user,ip') and no duplicates
// within subgroups ('user,ip,user')
$uniqueGroups = [];
$uniqueSubGroups = true;
// Every group should be valid, and subgroups should have valid groups inside
foreach ( $throttleGroups as $group ) {
if ( strpos( $group, ',' ) !== false ) {
$subGroups = explode( ',', $group );
if ( $subGroups !== array_unique( $subGroups ) ) {
$uniqueSubGroups = false;
break;
}
foreach ( $subGroups as $subGroup ) {
if ( !in_array( $subGroup, $validGroups ) ) {
$valid = false;
break 2;
}
}
sort( $subGroups );
$uniqueGroups[] = implode( ',', $subGroups );
} else {
if ( !in_array( $group, $validGroups ) ) {
$valid = false;
break;
}
$uniqueGroups[] = $group;
}
}
if ( !$valid ) {
$error = 'abusefilter-edit-invalid-throttlegroups';
} elseif ( !$uniqueSubGroups || $uniqueGroups !== array_unique( $uniqueGroups ) ) {
$error = 'abusefilter-edit-duplicated-throttlegroups';
}
}
return $error;
}
/**
* Checks whether user input for the filter editing form is valid and if so saves the filter
*
@ -2269,6 +2339,15 @@ class AbuseFilter {
}
}
// If 'throttle' is selected, check its parameters
if ( !empty( $actions['throttle'] ) ) {
$throttleCheck = self::checkThrottleParameters( $actions['throttle']['parameters'] );
if ( $throttleCheck !== null ) {
$validationStatus->error( $throttleCheck );
return $validationStatus;
}
}
$differences = self::compareVersions(
[ $newRow, $actions ],
[ $newRow->mOriginalRow, $newRow->mOriginalActions ]
@ -2915,7 +2994,40 @@ class AbuseFilter {
} elseif ( $action === 'throttle' ) {
array_shift( $parameters );
list( $actions, $time ) = explode( ',', array_shift( $parameters ) );
$groups = $wgLang->commaList( $parameters );
if ( $parameters === [ '' ] ) {
// Having empty groups won't happen for new filters due to validation upon saving,
// but old entries may have it. We'd better not show a broken message. Also,
// the array has an empty string inside because we haven't been passing an empty array
// as the default when retrieving wpFilterThrottleGroups with getArray (when it was
// a CheckboxMultiselect).
$groups = '';
} else {
// Old entries may not have unique values.
$throttleGroups = array_unique( $parameters );
// Join comma-separated groups in a commaList with a final "and", and convert to messages.
// Messages used here: abusefilter-throttle-ip, abusefilter-throttle-user,
// abusefilter-throttle-site, abusefilter-throttle-creationdate, abusefilter-throttle-editcount
// abusefilter-throttle-range, abusefilter-throttle-page
foreach ( $throttleGroups as &$val ) {
if ( strpos( $val, ',' ) !== false ) {
$subGroups = explode( ',', $val );
foreach ( $subGroups as &$group ) {
$msg = wfMessage( "abusefilter-throttle-$group" );
// We previously accepted literally everything in this field, so old entries
// may have weird stuff.
$group = $msg->exists() ? $msg->text() : $group;
}
unset( $group );
$val = $wgLang->listToText( $subGroups );
} else {
$msg = wfMessage( "abusefilter-throttle-$val" );
$val = $msg->exists() ? $msg->text() : $val;
}
}
unset( $val );
$groups = $wgLang->semicolonList( $throttleGroups );
}
$displayAction = self::getActionDisplay( $action ) .
wfMessage( 'colon-separator' )->escaped() .
wfMessage( 'abusefilter-throttle-details' )->params( $actions, $time, $groups )->escaped();

View file

@ -566,47 +566,36 @@ class AbuseFilterViewEdit extends AbuseFilterView {
'label' => $this->msg( 'abusefilter-edit-throttle-period' )->text()
]
);
$throttleFields['abusefilter-edit-throttle-groups'] =
$throttleConfig = [
'values' => $throttleGroups,
'label' => $this->msg( 'abusefilter-edit-throttle-groups' )->parse(),
'disabled' => $readOnlyAttrib
];
$this->getOutput()->addJsConfigVars( 'throttleConfig', $throttleConfig );
$hiddenGroups =
new OOUI\FieldLayout(
new OOUI\CheckboxMultiselectInputWidget( [
'name' => 'wpFilterThrottleGroups[]',
'value' => $throttleGroups,
'options' => [
[
'data' => 'ip',
'label' => $this->msg( 'abusefilter-edit-throttle-ip' )->text()
],
[
'data' => 'user',
'label' => $this->msg( 'abusefilter-edit-throttle-user' )->text()
],
[
'data' => 'range',
'label' => $this->msg( 'abusefilter-edit-throttle-range' )->text()
],
[
'data' => 'creationdate',
'label' => $this->msg( 'abusefilter-edit-throttle-creationdate' )->text()
],
[
'data' => 'editcount',
'label' => $this->msg( 'abusefilter-edit-throttle-editcount' )->text()
],
[
'data' => 'site',
'label' => $this->msg( 'abusefilter-edit-throttle-site' )->text()
],
[
'data' => 'page',
'label' => $this->msg( 'abusefilter-edit-throttle-page' )->text()
]
]
] + $readOnlyAttrib ),
new OOUI\MultilineTextInputWidget( [
'name' => 'wpFilterThrottleGroups',
'value' => implode( "\n", $throttleGroups ),
'rows' => 5,
'placeholder' => $this->msg( 'abusefilter-edit-throttle-hidden-placeholder' )->text(),
'infusable' => true,
'id' => 'mw-abusefilter-hidden-throttle-field'
] + $readOnlyAttrib
),
[
'label' => $this->msg( 'abusefilter-edit-throttle-groups' )->text(),
'align' => 'right'
'label' => new OOUI\HtmlSnippet(
$this->msg( 'abusefilter-edit-throttle-groups' )->parse()
),
'align' => 'top',
'id' => 'mw-abusefilter-hidden-throttle'
]
);
$throttleFields['abusefilter-edit-throttle-groups'] = $hiddenGroups;
$throttleSettings .=
Xml::tags(
'div',
@ -753,10 +742,9 @@ class AbuseFilterViewEdit extends AbuseFilterView {
$output .= $checkbox;
$tagConfig = [
'tagUsed' => $tags,
'tagLabel' => $this->msg( 'abusefilter-edit-tag-tag' )->parse(),
'tagPlaceholder' => $this->msg( 'abusefilter-edit-tag-placeholder' )->text(),
'tagDisabled' => $readOnlyAttrib
'values' => $tags,
'label' => $this->msg( 'abusefilter-edit-tag-tag' )->parse(),
'disabled' => $readOnlyAttrib
];
$this->getOutput()->addJsConfigVars( 'tagConfig', $tagConfig );
@ -768,7 +756,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
'rows' => 5,
'placeholder' => $this->msg( 'abusefilter-edit-tag-hidden-placeholder' )->text(),
'infusable' => true,
'id' => 'mw-abusefilter-hidden-tags-field'
'id' => 'mw-abusefilter-hidden-tag-field'
] + $readOnlyAttrib
),
[
@ -776,7 +764,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
$this->msg( 'abusefilter-edit-tag-tag' )->parse()
),
'align' => 'top',
'id' => 'mw-abusefilter-hidden-tags'
'id' => 'mw-abusefilter-hidden-tag'
]
);
$output .=
@ -1192,7 +1180,18 @@ class AbuseFilterViewEdit extends AbuseFilterView {
// We need to load the parameters
$throttleCount = $request->getIntOrNull( 'wpFilterThrottleCount' );
$throttlePeriod = $request->getIntOrNull( 'wpFilterThrottlePeriod' );
$throttleGroups = $request->getArray( 'wpFilterThrottleGroups' );
// First explode with \n, which is the delimiter used in the textarea
$rawGroups = explode( "\n", $request->getText( 'wpFilterThrottleGroups' ) );
// Trim any space, both as an actual group and inside subgroups
$throttleGroups = [];
foreach ( $rawGroups as $group ) {
if ( strpos( $group, ',' ) !== false ) {
$subGroups = explode( ',', $group );
$throttleGroups[] = implode( ',', array_map( 'trim', $subGroups ) );
} elseif ( trim( $group ) !== '' ) {
$throttleGroups[] = trim( $group );
}
}
$parameters[0] = $this->mFilter;
$parameters[1] = "$throttleCount,$throttlePeriod";

View file

@ -0,0 +1,356 @@
<?php
/**
* Normalizes throttle parameters as part of the overhaul described in T203587
*
* Tasks performed by this script:
* - Remove duplicated throttle groups (T203584)
* - Remove unrecognized stuff from throttle groups (T203584)
* - Checks if throttle count or period have extra commas inside. If this leads to the filter acting
* like it would with throttle disabled, we just disable it. Otherwise, since we don't know what
* the filter is meant to do, we just ask users to evaluate and fix every case by hand. This is
* highly unlikely to happen anyway. (T203585)
* - Disables the throttle action for filters where throttle groups are empty (or only contain
* unknown keywords). Per T203336#4568819, in those case throttle already doesn't work. (T203584)
*
* @ingroup Maintenance
*/
if ( getenv( 'MW_INSTALL_PATH' ) ) {
$IP = getenv( 'MW_INSTALL_PATH' );
} else {
$IP = __DIR__ . '/../../..';
}
require_once "$IP/maintenance/Maintenance.php";
/**
* Normalizes throttle parameters, see T203587
*/
class NormalizeThrottleParameters extends LoggedUpdateMaintenance {
public function __construct() {
parent::__construct();
$this->mDescription = 'Normalize AbuseFilter throttle parameters - T203587';
$this->addOption( 'dry-run', 'Perform a dry run' );
$this->requireExtension( 'Abuse Filter' );
}
/**
* @see Maintenance::getUpdateKey()
* @return string
*/
public function getUpdateKey() {
return __CLASS__;
}
/** @var \Wikimedia\Rdbms\IDatabase $db The master database */
private $dbw;
/**
* Rollback the current transaction and emit a fatal error
*
* @param string $msg The message of the error
*/
protected function fail( $msg ) {
$this->rollbackTransaction( $this->dbw, __METHOD__ );
$this->fatalError( $msg );
}
/**
* Get normalized throttle groups
*
* @param array $params Throttle parameters
* @return array[] The first element is the array of old throttle groups, the second
* is an array of formatted throttle groups
*/
private function getNewGroups( $params ) {
$validGroups = [
'ip',
'user',
'range',
'creationdate',
'editcount',
'site',
'page'
];
$rawGroups = array_slice( $params, 2 );
$newGroups = [];
// We use a standard order to check for duplicates. This variable is not used as the actual
// array of groups to avoid silly changes like 'ip,user' => 'user,ip'
$normalizedGroups = [];
// Every group should be valid, and subgroups should have valid groups inside. Only keep
// valid (sub)groups.
foreach ( $rawGroups as $group ) {
if ( strpos( $group, ',' ) !== false ) {
// No duplicates in subgroups
$subGroups = array_unique( explode( ',', $group ) );
$uniqueGroup = implode( ',', $subGroups );
$valid = true;
foreach ( $subGroups as $subGroup ) {
if ( !in_array( $subGroup, $validGroups ) ) {
$valid = false;
break;
}
}
sort( $subGroups );
if ( $valid && !in_array( $subGroups, $normalizedGroups ) ) {
$newGroups[] = $uniqueGroup;
$normalizedGroups[] = $subGroups;
}
} elseif ( in_array( $group, $validGroups ) ) {
$newGroups[] = $group;
$normalizedGroups[] = $group;
}
}
// Remove duplicates
$newGroups = array_unique( $newGroups );
return [ $rawGroups, $newGroups ];
}
/**
* Check if throttle rate is malformed, i.e. if it has extra commas
*
* @param string $rate The throttle rate as saved in the DB ("count,period")
* @return string|null String with error type or null if the rate is valid
*/
private function checkThrottleRate( $rate ) {
if ( preg_match( '/^,/', $rate ) === 1 ) {
// The comma was inserted at least in throttle count. This behaves like if
// throttling isn't enabled, so just disable it
return 'disable';
} elseif ( preg_match( '/^\d+,$/', $rate ) === 1 || preg_match( '/^\d+,\d+$/', $rate ) === 0 ) {
// First condition is for comma only inside throttle period. The behaviour in this case
// is unclear, ask users to fix this by hand. Second condition is for every other case;
// since it's unpredictable what the problem is, we just ask to fix it by hand.
return 'hand';
} else {
return null;
}
}
/**
* @see Maintenance::doDBUpdates
* @return bool
*/
public function doDBUpdates() {
$user = User::newSystemUser( 'AbuseFilter updater', [ 'steal' => true ] );
$this->dbw = wfGetDB( DB_MASTER );
$dryRun = $this->hasOption( 'dry-run' );
$this->beginTransaction( $this->dbw, __METHOD__ );
// Only select throttle actions
$actionRows = $this->dbw->select(
'abuse_filter_action',
[ 'afa_filter', 'afa_parameters' ],
[ 'afa_consequence' => 'throttle' ],
__METHOD__,
[ 'LOCK IN SHARE MODE' ]
);
$newActionRows = [];
$deleteActionIDs = [];
$changeActionIDs = [];
foreach ( $actionRows as $actRow ) {
$filter = $actRow->afa_filter;
$params = explode( "\n", $actRow->afa_parameters );
$rateCheck = $this->checkThrottleRate( $params[1] );
if ( $rateCheck === 'hand' ) {
$this->fail(
"Throttle count and period for filter $filter are malformed, probably due to extra commas. " .
"Please fix them by hand in the way they're meant to be, then launch the script again."
);
}
list( $oldGroups, $newGroups ) = $this->getNewGroups( $params );
if ( $rateCheck === 'disable' || count( $newGroups ) === 0 ) {
// Empty throttle groups, disable throttle for the filter
$deleteActionIDs[] = $actRow->afa_filter;
} elseif ( $oldGroups !== $newGroups ) {
$newParams = array_merge( array_slice( $params, 0, 2 ), $newGroups );
$newActionRows[] = [
'afa_filter' => $actRow->afa_filter,
'afa_consequence' => 'throttle',
'afa_parameters' => implode( "\n", $newParams )
];
$changeActionIDs[] = $actRow->afa_filter;
}
}
$changeActionCount = count( $changeActionIDs );
if ( $changeActionCount ) {
if ( $dryRun ) {
$this->output(
"normalizeThrottleParameter has found $changeActionCount rows to change in " .
"abuse_filter_action for the following IDs: " . implode( ', ', $changeActionIDs ) . "\n"
);
} else {
$this->dbw->replace(
'abuse_filter_action',
[ [ 'afa_filter', 'afa_consequence' ] ],
$newActionRows,
__METHOD__
);
}
}
$deleteActionCount = count( $deleteActionIDs );
// Use the same timestamps in abuse_filter and abuse_filter_history, since this is
// what we do in the actual code.
$timestamps = [];
if ( $deleteActionCount ) {
if ( $dryRun ) {
$this->output(
"normalizeThrottleParameter has found $deleteActionCount rows to delete in " .
"abuse_filter_action and update in abuse_filter for the following IDs: " .
implode( ', ', $deleteActionIDs ) . "\n"
);
} else {
// Delete rows in abuse_filter_action
$this->dbw->delete(
'abuse_filter_action',
[
'afa_consequence' => 'throttle',
'afa_filter' => $deleteActionIDs
],
__METHOD__
);
// Update abuse_filter. abuse_filter_history done later
$rows = $this->dbw->select(
'abuse_filter',
'*',
[ 'af_id' => $deleteActionIDs ],
__METHOD__
);
foreach ( $rows as $row ) {
$oldActions = explode( ',', $row->af_actions );
$actions = array_diff( $oldActions, [ 'throttle' ] );
$timestamps[ $row->af_id ] = $this->dbw->timestamp( wfTimestampNow() );
$newRow = [
'af_user' => $user->getId(),
'af_user_text' => $user->getName(),
'af_timestamp' => $timestamps[ $row->af_id ],
'af_actions' => implode( ',', $actions ),
] + $row;
$this->dbw->replace(
'abuse_filter',
[ 'af_id' ],
$newRow,
__METHOD__
);
}
}
}
$affectedActionRows = $changeActionCount + $deleteActionCount;
$touchedIDs = array_merge( $changeActionIDs, $deleteActionIDs );
if ( count( $touchedIDs ) === 0 ) {
$this->output( "No throttle parameters to normalize.\n" );
$this->commitTransaction( $this->dbw, __METHOD__ );
return !$dryRun;
}
// Create new history rows for every changed filter
$historyIDs = $this->dbw->selectFieldValues(
'abuse_filter_history',
'MAX(afh_id)',
[ 'afh_filter' => $touchedIDs ],
__METHOD__,
[ 'GROUP BY' => 'afh_filter', 'LOCK IN SHARE MODE' ]
);
$lastHistoryRows = $this->dbw->select(
'abuse_filter_history',
[
'afh_filter',
'afh_user',
'afh_user_text',
'afh_timestamp',
'afh_pattern',
'afh_comments',
'afh_flags',
'afh_public_comments',
'afh_actions',
'afh_deleted',
'afh_changed_fields',
'afh_group'
],
[ 'afh_id' => $historyIDs ],
__METHOD__,
[ 'LOCK IN SHARE MODE' ]
);
$newHistoryRows = [];
$changeHistoryFilters = [];
foreach ( $lastHistoryRows as $histRow ) {
$actions = unserialize( $histRow->afh_actions );
$filter = $histRow->afh_filter;
$rateCheck = $this->checkThrottleRate( $actions['throttle'][1] );
if ( $rateCheck === 'hand' ) {
$this->fail(
"Throttle count and period for filter $filter are malformed, probably due to extra commas. " .
"Please fix them by hand in the way they're meant to be, then launch the script again."
);
}
list( $oldGroups, $newGroups ) = $this->getNewGroups( $actions['throttle'] );
$fixedRowSection = [
'afh_id' => $this->dbw->nextSequenceValue( 'abuse_filter_af_id_seq' ),
'afh_user' => $user->getId(),
'afh_user_text' => $user->getName(),
'afh_timestamp' => $timestamps[ $filter ] ?? $this->dbw->timestamp( wfTimestampNow() ),
'afh_changed_fields' => 'actions',
] + get_object_vars( $histRow );
if ( $rateCheck === 'disable' || count( $newGroups ) === 0 ) {
// Empty throttle groups, disable throttle for the filter
unset( $actions['throttle'] );
$newHistoryRows[] = [
'afh_actions' => serialize( $actions )
] + $fixedRowSection;
$changeHistoryFilters[] = $filter;
} elseif ( $oldGroups !== $newGroups ) {
$actions['throttle'] = array_merge( array_slice( $actions['throttle'], 0, 2 ), $newGroups );
$newHistoryRows[] = [
'afh_actions' => serialize( $actions )
] + $fixedRowSection;
$changeHistoryFilters[] = $filter;
}
}
$historyCount = count( $changeHistoryFilters );
$sanityCheck = $historyCount === $affectedActionRows;
if ( !$sanityCheck ) {
// Something went wrong.
$this->fail(
"The amount of affected rows isn't equal for abuse_filter_action and abuse_filter history. " .
"Found $affectedActionRows for the former and $historyCount for the latter."
);
}
if ( count( $newHistoryRows ) ) {
if ( $dryRun ) {
$this->output(
"normalizeThrottleParameter would insert $historyCount rows in abuse_filter_history" .
" for the following filters: " . implode( ', ', $changeHistoryFilters ) . "\n"
);
} else {
$this->dbw->insert(
'abuse_filter_history',
$newHistoryRows,
__METHOD__
);
}
}
$this->commitTransaction( $this->dbw, __METHOD__ );
$changed = $affectedActionRows + $historyCount;
$resultMsg = $dryRun ?
"Throttle parameter normalization would change a total of $changed rows.\n" :
"Throttle parameters successfully normalized. Changed $changed rows.\n";
$this->output( $resultMsg );
return !$dryRun;
}
}
$maintClass = 'NormalizeThrottleParameters';
require_once RUN_MAINTENANCE_IF_MAIN;

View file

@ -358,47 +358,68 @@
}
}
/**
* Builds a TagMultiselectWidget, to be used both for throttle groups and change tags
*
* @param {string} action Either 'throttle' or 'tag', will be used to build element IDs
* @param {Array} config The array with configuration passed from PHP code
*/
function buildSelector( action, config ) {
var disabled = config.disabled.length !== 0,
// mw-abusefilter-throttle-parameters, mw-abusefilter-tag-parameters
$container = $( '#mw-abusefilter-' + action + '-parameters' ),
// Character used to separate elements in the textarea.
separator = action === 'throttle' ? '\n' : ',',
selector, field, hiddenField;
selector =
new OO.ui.TagMultiselectWidget( {
inputPosition: 'outline',
allowArbitrary: true,
allowEditTags: true,
selected: config.values,
// abusefilter-edit-throttle-placeholder, abusefilter-edit-tag-placeholder
placeholder: OO.ui.msg( 'abusefilter-edit-' + action + '-placeholder' ),
disabled: disabled
} );
field =
new OO.ui.FieldLayout(
selector,
{
label: $( $.parseHTML( config.label ) ),
align: 'top'
}
);
// mw-abusefilter-hidden-throttle-field, mw-abusefilter-hidden-tag-field
hiddenField = OO.ui.infuse( $( '#mw-abusefilter-hidden-' + action + '-field' ) );
selector.on( 'change', function () {
hiddenField.setValue( selector.getValue().join( separator ) );
} );
// mw-abusefilter-hidden-throttle, mw-abusefilter-hidden-tag
$( '#mw-abusefilter-hidden-' + action ).hide();
$container.append( field.$element );
}
// On ready initialization
$( document ).ready( function () {
var basePath, readOnly,
$exportBox = $( '#mw-abusefilter-export' ),
isFilterEditor = mw.config.get( 'isFilterEditor' ),
tagConfig = mw.config.get( 'tagConfig' ),
$tagContainer, tagUsed, tagDisabled, tagSelector, tagField,
tagHiddenField, cbEnabled, cbDeleted;
throttleConfig = mw.config.get( 'throttleConfig' ),
cbEnabled, cbDeleted;
if ( isFilterEditor ) {
// Configure the actual editing interface
if ( tagConfig ) {
// Build the tag selector
$tagContainer = $( '#mw-abusefilter-tag-parameters' );
tagUsed = tagConfig.tagUsed;
tagDisabled = tagConfig.tagDisabled.length !== 0;
// Input field for tags
tagSelector =
new OO.ui.TagMultiselectWidget( {
inputPosition: 'outline',
allowArbitrary: true,
allowEditTags: true,
selected: tagUsed,
placeholder: tagConfig.tagPlaceholder,
disabled: tagDisabled
} );
tagField =
new OO.ui.FieldLayout(
tagSelector,
{
label: $( $.parseHTML( tagConfig.tagLabel ) ),
align: 'top'
}
);
tagHiddenField = OO.ui.infuse( $( '#mw-abusefilter-hidden-tags-field' ) );
tagSelector.on( 'change', function () {
tagHiddenField.setValue( tagSelector.getValue() );
} );
$( '#mw-abusefilter-hidden-tags' ).hide();
$tagContainer.append( tagField.$element );
buildSelector( 'tag', tagConfig );
}
if ( throttleConfig ) {
// Build the throttle groups selector
buildSelector( 'throttle', throttleConfig );
}
toggleWarnPreviewButton = OO.ui.infuse( $( '#mw-abusefilter-warn-preview-button' ) );

View file

@ -485,7 +485,112 @@ class AbuseFilterSaveTest extends MediaWikiTestCase {
'needsOtherFilters' => 1
]
]
],
[
[
'filterParameters' => [
'rules' => '1==1',
'description' => 'Invalid throttle groups',
'notes' => 'Throttle... Again',
'throttleEnabled' => true,
'throttleCount' => 11,
'throttlePeriod' => 111,
'throttleGroups' => 'user\nfoo'
],
'testData' => [
'doingWhat' => 'Trying to save a filter with invalid throttle groups',
'expectedResult' => 'an error message saying that throttle groups are invalid',
'expectedMessage' => 'abusefilter-edit-invalid-throttlegroups',
'shouldFail' => true,
'shouldBeSaved' => false,
'customUserGroup' => '',
'needsOtherFilters' => false
]
]
]
];
}
/**
* Check that our tag validation is working properly. Note that we only need one test
* for each called function. Consistency within ChangeTags functions should be
* assured by tests in core. The test for canAddTagsAccompanyingChange and canCreateTag
* are missing because they won't actually fail, never. Resolving T173917 would
* greatly improve the situation and could help writing better tests.
*
* @param string $tag The tag to validate
* @param string|null $error The expected error message. Null if validations should pass
* @covers AbuseFilter::isAllowedTag
* @dataProvider provideTags
*/
public function testIsAllowedTag( $tag, $error ) {
$status = AbuseFilter::isAllowedTag( $tag );
if ( !$status->isGood() ) {
$actualError = $status->getErrors();
$actualError = $actualError[0]['message'];
} else {
$actualError = null;
if ( $error !== null ) {
$this->fail( "Tag validation returned a valid status instead of the expected '$error' error." );
}
}
$this->assertSame(
$error,
$actualError,
"Expected message '$error', got '$actualError' while validating the tag '$tag'."
);
}
/**
* Data provider for testIsAllowedTag
* @return array
*/
public function provideTags() {
return [
[ 'a|b', 'tags-create-invalid-chars' ],
[ 'mw-undo', 'abusefilter-edit-bad-tags' ],
[ 'abusefilter-condition-limit', 'abusefilter-tag-reserved' ],
[ 'my_tag', null ],
];
}
/**
* Check that throttle parameters validation works fine
*
* @param array $params Throttle parameters
* @param string|null $error The expected error message. Null if validations should pass
* @covers AbuseFilter::checkThrottleParameters
* @dataProvider provideThrottleParameters
*/
public function testCheckThrottleParameters( $params, $error ) {
$result = AbuseFilter::checkThrottleParameters( $params );
$this->assertSame( $error, $result, 'Throttle parameter validation does not work as expected.' );
}
/**
* Data provider for testCheckThrottleParameters
* @return array
*/
public function provideThrottleParameters() {
return [
[ [ '1', '5,23', 'user', 'ip', 'page,range', 'ip,user', 'range,ip' ], null ],
[ [ '1', '5.3,23', 'user', 'ip' ], 'abusefilter-edit-invalid-throttlecount' ],
[ [ '1', '-3,23', 'user', 'ip' ], 'abusefilter-edit-invalid-throttlecount' ],
[ [ '1', '5,2.3', 'user', 'ip' ], 'abusefilter-edit-invalid-throttleperiod' ],
[ [ '1', '4,-14', 'user', 'ip' ], 'abusefilter-edit-invalid-throttleperiod' ],
[ [ '1', '3,33' ], 'abusefilter-edit-empty-throttlegroups' ],
[ [ '1', '3,33', 'user', 'ip,foo,user' ], 'abusefilter-edit-invalid-throttlegroups' ],
[ [ '1', '3,33', 'foo', 'ip,user' ], 'abusefilter-edit-invalid-throttlegroups' ],
[ [ '1', '3,33', 'foo', 'ip,user,bar' ], 'abusefilter-edit-invalid-throttlegroups' ],
[ [ '1', '3,33', 'user', 'ip,page,user' ], null ],
[
[ '1', '3,33', 'ip', 'user','user,ip', 'ip,user', 'user,ip,user', 'user', 'ip,ip,user' ],
'abusefilter-edit-duplicated-throttlegroups'
],
[ [ '1', '3,33', 'ip,ip,user' ], 'abusefilter-edit-duplicated-throttlegroups' ],
[ [ '1', '3,33', 'user,ip', 'ip,user' ], 'abusefilter-edit-duplicated-throttlegroups' ],
];
}
}

View file

@ -525,51 +525,6 @@ class AbuseFilterTest extends MediaWikiTestCase {
];
}
/**
* Check that our tag validation is working properly. Note that we only need one test
* for each called function. Consistency within ChangeTags functions should be
* assured by tests in core. The test for canAddTagsAccompanyingChange and canCreateTag
* are missing because they won't actually fail, never. Resolving T173917 would
* greatly improve the situation and could help writing better tests.
*
* @param string $tag The tag to validate
* @param string|null $error The expected error message. Null if validations should pass
* @covers AbuseFilter::isAllowedTag
* @dataProvider provideTags
*/
public function testIsAllowedTag( $tag, $error ) {
$status = AbuseFilter::isAllowedTag( $tag );
if ( !$status->isGood() ) {
$actualError = $status->getErrors();
$actualError = $actualError[0]['message'];
} else {
$actualError = null;
if ( $error !== null ) {
$this->fail( "Tag validation returned a valid status instead of the expected '$error' error." );
}
}
$this->assertSame(
$error,
$actualError,
"Expected message '$error', got '$actualError' while validating the tag '$tag'."
);
}
/**
* Data provider for testIsAllowedTag
* @return array
*/
public function provideTags() {
return [
[ 'a|b', 'tags-create-invalid-chars' ],
[ 'mw-undo', 'abusefilter-edit-bad-tags' ],
[ 'abusefilter-condition-limit', 'abusefilter-tag-reserved' ],
[ 'my_tag', null ],
];
}
/**
* Check that version comparing works well
*