wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-23 21:53:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve how the number of conditions is counted

Browse source 
With the new behavior, the number of conditions in incremented when:
* Evaluating a function
* Evaluating a comparison operator (== === != !== < > <= >= =)
* Evaluating a keyword (in like matches contains rlike irlike regex)

Previously, the number of conditions was incremented when:
* Evaluating a function
* Entering the comparison operator evaluation mode

This resulted in a number of surprising behaviors. In particular:
* '(((a == b)))' counted as 4 conditions, not 1
* 'contains_any(a, b, c)' counted as 5 conditions, not 1
* 'a == b == c' counted as 1 condition, not 2
* 'a in b + c in d + e in f' counted as 1 condition, not 3
* 'true' counted as 1 condition, not 0

It is still possible to easily cheat the count by rewriting comparisons
as arithmetic operations. I believe this is meant to advise users of
the complexity of their rules and not really enforce strict limits.

Bug: T132190
Change-Id: I897769db4c2ceac802e3ae5d6fa8e9c9926ef246
This commit is contained in:
Bartosz Dziewoński 2016-04-09 15:00:02 +02:00
parent ca0b0c081d
commit 3b32cf00e9
2 changed files with 33 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
AbuseFilter.parser.php
View file

@ -1016,7 +1016,6 @@ class AbuseFilterParser {
* @param $result
*/
protected function doLevelCompares( &$result ) {
AbuseFilter::triggerLimiter();
$this->doLevelSumRels( $result );
$ops = array( '==', '===', '!=', '!==', '<', '>', '<=', '>=', '=' );
while ( $this->mCur->type == AFPToken::TOP && in_array( $this->mCur->value, $ops ) ) {
@ -1024,6 +1023,7 @@ class AbuseFilterParser {
$this->move();
$r2 = new AFPData();
$this->doLevelSumRels( $r2 );
AbuseFilter::triggerLimiter();
$result = AFPData::compareOp( $result, $r2, $op );
}
}
@ -1114,6 +1114,7 @@ class AbuseFilterParser {
return; // The result doesn't matter.
}
AbuseFilter::triggerLimiter();
wfProfileIn( __METHOD__ . "-$func" );
$result = AFPData::$func( $result, $r2, $this->mCur->pos );
wfProfileOut( __METHOD__ . "-$func" );

33
tests/phpunit/parserTest.php
View file

@ -77,7 +77,7 @@ class AbuseFilterParserTest extends MediaWikiTestCase {
}
/**
* Ensure that AbsueFilterTokenizer::OPERATOR_RE matches the contents
* Ensure that AbuseFilterTokenizer::OPERATOR_RE matches the contents
* and order of AbuseFilterTokenizer::$operators.
*/
public function testOperatorRe() {
@ -88,7 +88,7 @@ class AbuseFilterParserTest extends MediaWikiTestCase {
}
/**
* Ensure that AbsueFilterTokenizer::RADIX_RE matches the contents
* Ensure that AbuseFilterTokenizer::RADIX_RE matches the contents
* and order of AbuseFilterTokenizer::$bases.
*/
public function testRadixRe() {
@ -96,4 +96,33 @@ class AbuseFilterParserTest extends MediaWikiTestCase {
$radixRe = "/([0-9A-Fa-f]+(?:\.\d*)?|\.\d+)([$baseClass])?/Au";
$this->assertEquals( $radixRe, AbuseFilterTokenizer::RADIX_RE );
}
/**
* Ensure the number of conditions counted for given expressions is right.
*
* @dataProvider condCountCases
*/
public function testCondCount( $rule, $expected ) {
$parser = self::getParser();
// Set some variables for convenience writing test cases
$parser->setVars( array_combine( range( 'a', 'f' ), range( 'a', 'f' ) ) );
$countBefore = AbuseFilter::$condCount;
$parser->parse( $rule );
$countAfter = AbuseFilter::$condCount;
$actual = $countAfter - $countBefore;
$this->assertEquals( $expected, $actual, 'Condition count for ' . $rule );
}
/**
* @return array
*/
public function condCountCases() {
return array(
array( '(((a == b)))', 1 ),
array( 'contains_any(a, b, c)', 1 ),
array( 'a == b == c', 2 ),
array( 'a in b + c in d + e in f', 3 ),
array( 'true', 0 ),
);
}
}