jenkins-bot has submitted this change and it was merged. 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(-) Approvals: Legoktm: Looks good to me, approved jenkins-bot: Verified 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: merged Gerrit-Change-Id: I897769db4c2ceac802e3ae5d6fa8e9c9926ef246 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/AbuseFilter Gerrit-Branch: master Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com> Gerrit-Reviewer: Alex Monk <kren...@gmail.com> Gerrit-Reviewer: Bartosz Dziewoński <matma....@gmail.com> Gerrit-Reviewer: Dragons flight <ro...@robertrohde.com> Gerrit-Reviewer: Hoo man <h...@online.de> Gerrit-Reviewer: Jackmcbarn <jackmcb...@gmail.com> Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com> Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org> Gerrit-Reviewer: Se4598 <se4...@gmx.de> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits