Commit graph

5 commits

Author SHA1 Message Date
Bartosz Dziewoński e79b45b71f 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
2016-04-09 16:25:52 +02:00
Bartosz Dziewoński 3b32cf00e9 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
2016-04-09 16:16:27 +02:00
Ori Livneh bab9832415 Move rule tokenization to new AbuseFilterTokenizer class
* Move AbuseFilterParser::nextToken() and the various AbuseFilterParser
  properties that accompanied it to a new class, AbuseFilterTokenizer.
* Tokenize rules eagerly and cache the result in APC.

Change-Id: I15f5b5b65e8c4ec4fba3000d7c9fd78b98967d1d
2015-08-25 14:00:10 -07:00
Ori Livneh b388dfab1b Clean-up of AbuseFilterParser::nextToken()
No functional changes.

* Don't include $code as part of the return value; it is ignored anyway.
* Removed AbuseFilterParser::lastHandledToken and AFPParserState::lastInput,
  because AbuseFilterParser::nextToken() no longer calls itself recursively.
* The regular expression that matches operators is no longer constructed
  dynamically, but hard-coded into the class. To make sure it does not drift
  apart from the more legible AbuseFilterParser::$mOps, add a unit test that
  constructs the regex dynamically as before and compares it to
  AbuseFilterParser::OPERATOR_RE.
* AbuseFilterParser::RADIX_RE ditto.

Change-Id: I9c23b60759ed2f4c73a9b480243b16bbce5a208f
2015-08-25 10:50:31 -07:00
Marius Hoch 35747761fb Allow running the AbuseFilter parser tests via phpunit
I've also added myself to the credits file as I'm the only
maintainer of this extension for a while now.

Change-Id: Id998172ea2abd70b8243de9db1a96cc2cfa47a64
2013-07-08 19:22:43 +02:00