Improve ignoring short-circuited operations

Previously, 'false & a == b' would actually execute the comparison and
count it against the condition limit, while 'false & (a == b)' wouldn't.
They behave the same now.

mShortCircuit was only checked for the most potentially expensive
operations (computing functions and getting variables), all the other
operations on bogus values generated by this would be executed and the
results ignored later.

This probably doesn't noticeably improve performance, but it corrects
how the condition limit is counted.

Bug: T43693
Change-Id: Id1d5f577b14b6ae6d987ded12689788eb7922474
This commit is contained in:
Bartosz Dziewoński 2016-04-09 15:53:16 +02:00
parent 3b32cf00e9
commit e79b45b71f
4 changed files with 23 additions and 0 deletions

View file

@ -1023,6 +1023,9 @@ class AbuseFilterParser {
$this->move();
$r2 = new AFPData();
$this->doLevelSumRels( $r2 );
if ( $this->mShortCircuit ) {
break; // The result doesn't matter.
}
AbuseFilter::triggerLimiter();
$result = AFPData::compareOp( $result, $r2, $op );
}
@ -1039,6 +1042,9 @@ class AbuseFilterParser {
$this->move();
$r2 = new AFPData();
$this->doLevelMulRels( $r2 );
if ( $this->mShortCircuit ) {
break; // The result doesn't matter.
}
if ( $op == '+' ) {
$result = AFPData::sum( $result, $r2 );
}
@ -1059,6 +1065,9 @@ class AbuseFilterParser {
$this->move();
$r2 = new AFPData();
$this->doLevelPow( $r2 );
if ( $this->mShortCircuit ) {
break; // The result doesn't matter.
}
$result = AFPData::mulRel( $result, $r2, $op, $this->mCur->pos );
}
}
@ -1072,6 +1081,9 @@ class AbuseFilterParser {
$this->move();
$expanent = new AFPData();
$this->doLevelBoolInvert( $expanent );
if ( $this->mShortCircuit ) {
break; // The result doesn't matter.
}
$result = AFPData::pow( $result, $expanent );
}
}
@ -1083,6 +1095,9 @@ class AbuseFilterParser {
if ( $this->mCur->type == AFPToken::TOP && $this->mCur->value == '!' ) {
$this->move();
$this->doLevelSpecialWords( $result );
if ( $this->mShortCircuit ) {
return; // The result doesn't matter.
}
$result = AFPData::boolInvert( $result );
} else {
$this->doLevelSpecialWords( $result );
@ -1129,6 +1144,9 @@ class AbuseFilterParser {
if ( $this->mCur->type == AFPToken::TOP && ( $op == "+" || $op == "-" ) ) {
$this->move();
$this->doLevelListElements( $result );
if ( $this->mShortCircuit ) {
return; // The result doesn't matter.
}
if ( $op == '-' ) {
$result = AFPData::unaryMinus( $result );
}

View file

@ -0,0 +1 @@
MATCH

View file

@ -0,0 +1,2 @@
/* The division by zero should not be executed and not crash the filter */
true | 1/0

View file

@ -123,6 +123,8 @@ class AbuseFilterParserTest extends MediaWikiTestCase {
array( 'a == b == c', 2 ),
array( 'a in b + c in d + e in f', 3 ),
array( 'true', 0 ),
array( 'a == a | c == d', 1 ),
array( 'a == b & c == d', 1 ),
);
}
}