Page MenuHomePhabricator

Order of operations has changed in AbuseFilters
Closed, ResolvedPublic

Event Timeline

Nirmos created this task.Oct 19 2016, 6:57 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 19 2016, 6:57 PM
Nirmos triaged this task as Unbreak Now! priority.Oct 19 2016, 6:58 PM
Restricted Application added subscribers: Jay8g, Luke081515, TerraCodes. · View Herald TranscriptOct 19 2016, 6:58 PM
Tegel added a subscriber: Tegel.Oct 19 2016, 7:19 PM

Technically speaking

cond | cond

now needs to be changed to

(cond | cond)

jhsoby added a subscriber: jhsoby.Oct 19 2016, 7:58 PM
Nirmos renamed this task from Edits trigger all AbuseFilters to Order of operations has changed in AbuseFilters.Oct 20 2016, 12:48 AM
cond | cond & cond

used to be interpreted as

( cond | cond ) & cond

but is now interpreted as

cond | ( cond & cond )

Change 316917 had a related patch set uploaded (by Legoktm):
Revert "Enable AbuseFilterCachingParser by default"

https://gerrit.wikimedia.org/r/316917

Change 316917 merged by jenkins-bot:
Revert "Enable AbuseFilterCachingParser by default"

https://gerrit.wikimedia.org/r/316917

Mentioned in SAL (#wikimedia-operations) [2016-10-20T01:17:35Z] <legoktm@mira> Synchronized wmf-config/InitialiseSettings.php: Revert Enable AbuseFilterCachingParser by default - T148673 (duration: 00m 51s)

Legoktm closed this task as Resolved.Oct 20 2016, 1:19 AM
Legoktm claimed this task.
Legoktm added subscribers: vvv, ori, Legoktm.

Should be reverted for now. CC: @ori and @vvv

Legoktm reopened this task as Open.Oct 20 2016, 1:20 AM

Oh, leaving open as someone presumably needs to fix the new AbuseFilterCachingParser.

Legoktm lowered the priority of this task from Unbreak Now! to Normal.Oct 20 2016, 1:20 AM
vvv added a comment.Oct 20 2016, 5:06 AM

The specific change which happened, as far as I understand it, is that the precedence remained the same, but apparently the new parser treats boolean operators as right-associative, whereas it used to treat them as left-associative. This is an oversight on my part. I will write a patch to fix that and add tests.

ori reassigned this task from Legoktm to vvv.Oct 20 2016, 8:15 PM
Johan moved this task from To Triage to Announce in next Tech/News on the User-notice board.
He7d3r added a subscriber: He7d3r.Oct 24 2016, 5:49 PM
ori added a comment.Nov 3 2016, 9:32 PM

@vvv, are you still planning on working on this?

matmarex closed this task as Resolved.Nov 16 2016, 3:13 PM
matmarex added a subscriber: matmarex.

This should be fixed with https://gerrit.wikimedia.org/r/#/c/320157/. @ori is re-enabling the feature in https://gerrit.wikimedia.org/r/#/c/321728/.