jenkins-bot has submitted this change and it was merged.
Change subject: Improve ignoring short-circuited operations
..
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
---
M AbuseFilter.parser.php
A tests/parserTests/shortcircuit.r
A tests/parserTests/shortcircuit.t
M tests/phpunit/parserTest.php
4 files changed, 23 insertions(+), 0 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/AbuseFilter.parser.php b/AbuseFilter.parser.php
index cc9d7a8..6cbefe6 100644
--- a/AbuseFilter.parser.php
+++ b/AbuseFilter.parser.php
@@ -1023,6 +1023,9 @@
$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 @@
$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 @@
$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 @@
$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 @@
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 @@
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 );
}
diff --git a/tests/parserTests/shortcircuit.r b/tests/parserTests/shortcircuit.r
new file mode 100644
index 000..4736e08
--- /dev/null
+++ b/tests/parserTests/shortcircuit.r
@@ -0,0 +1 @@
+MATCH
diff --git a/tests/parserTests/shortcircuit.t b/tests/parserTests/shortcircuit.t
new file mode 100644
index 000..bec088f
--- /dev/null
+++ b/tests/parserTests/shortcircuit.t
@@ -0,0 +1,2 @@
+/* The division by zero should not be executed and not crash the filter */
+true | 1/0
diff --git a/tests/phpunit/parserTest.php b/tests/phpunit/parserTest.php
index 0a114f1..6bb8ff9 100644
--- a/tests/phpunit/parserTest.php
+++ b/tests/phpunit/parserTest.php
@@ -123,6 +123,8 @@
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 ),
);
}
}
--
To view, visit https
