Bartosz Dziewoński has uploaded a new change for review.
https://gerrit.wikimedia.org/r/282477
Change subject: Improve how the number of conditions is counted
......................................................................
Improve how the number of conditions is counted
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
---
M AbuseFilter.parser.php
M tests/phpunit/parserTest.php
2 files changed, 33 insertions(+), 3 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/AbuseFilter
refs/changes/77/282477/1
diff --git a/AbuseFilter.parser.php b/AbuseFilter.parser.php
index 1ef3a1e..cc9d7a8 100644
--- a/AbuseFilter.parser.php
+++ b/AbuseFilter.parser.php
@@ -1016,7 +1016,6 @@
* @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 @@
$this->move();
$r2 = new AFPData();
$this->doLevelSumRels( $r2 );
+ AbuseFilter::triggerLimiter();
$result = AFPData::compareOp( $result, $r2, $op );
}
}
@@ -1114,6 +1114,7 @@
return; // The result doesn't matter.
}
+ AbuseFilter::triggerLimiter();
wfProfileIn( __METHOD__ . "-$func" );
$result = AFPData::$func( $result, $r2,
$this->mCur->pos );
wfProfileOut( __METHOD__ . "-$func" );
diff --git a/tests/phpunit/parserTest.php b/tests/phpunit/parserTest.php
index 34c6c9c..0a114f1 100644
--- a/tests/phpunit/parserTest.php
+++ b/tests/phpunit/parserTest.php
@@ -77,7 +77,7 @@
}
/**
- * 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 @@
}
/**
- * 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 @@
$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 ),
+ );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/282477
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I897769db4c2ceac802e3ae5d6fa8e9c9926ef246
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/AbuseFilter
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
