Page MenuHomePhabricator

Deprecate and then remove the old AbuseFilterParser
Open, HighPublic

Description

The new AbuseFilterCachingParser supports a saner syntax, and it has improved performance [1]. However, it also stopped supporting various features that the old parser used to support; notably:

  • Empty operands (T156096)
  • Dangling commas in non-variadic function calls (T153251)

Whenever a filter tries to use one of the deprecated features, we already emit a deprecation warning (in 1.34). However, in 1.35 we should start emitting notices whenever we find the old parser to be in use (note: this is the default, as specified in extension.json). Then, in 1.36 we should make the new parser be the default, and in 1.37 we should drop the old parser code altogether.

This is because, right now, we have to maintain two different parsers. Some features cannot be reasonably implemented in both parsers unless we introduce more and more hacks. Hence, we should get rid of the old parser as soon as possible. Proposing 1.37 per above, because fixing incompatible filters may take up a decent amount of time and work for wiki users.

[1] - At least for WMF wikis, the increase has been fairly small. T234427 tracks possible improvements in this area.

Event Timeline

Change 555487 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Emit a deprecation warning if the old parser is being used

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

Change 577538 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] parser: Set explicit deprecation version to 1.36 for empty operands

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

Legoktm added a subscriber: Legoktm.

AbuseFilter isn't bundled, removing as a release blocker.

Daimona triaged this task as High priority.Sep 16 2020, 2:34 PM

High priority because the 1.35 release is near, so we must backport the deprecation notice quickly.

Change 627958 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_35] parser: Set explicit deprecation version to 1.36 for empty operands

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

Change 627958 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_35] parser: Set explicit deprecation version to 1.36 for empty operands

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

Change 577538 abandoned by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@master] parser: Set explicit deprecation version to 1.36 for empty operands

Reason:
1.36 is now! We can make this the default (patch coming)

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

Change 555487 abandoned by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@master] Emit a deprecation warning if the old parser is being used

Reason:
Going to send another commit

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

I decided to anticipate the removal to 1.36 (and the deprecation of the old parser to 1.35), given that 1.35 is an LTS release, and that the old parser is too expensive to keep it around for a long time.

Change 627961 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_35] Set the CachingParser as the default parser

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

Change 628133 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Remove the old parser

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

Change 627961 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_35] Set the CachingParser as the default parser

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

I'd like to keep the removal (which is the only thing left to do) outside of the overhaul, because it would have a huge impact on code coverage that was not accounted for when outlining the grant goals, and I don't want to recompute everything.

Change 628133 abandoned by Daimona Eaytoy:

[mediawiki/extensions/AbuseFilter@master] Remove the old parser

Reason:

There are so many merge conflicts that redoing from scratch is certainly easier.

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

Change 678240 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/AbuseFilter@master] Remove the old parser

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

Change 678240 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Remove the old parser

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

Change 680753 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[operations/mediawiki-config@master] Stop setting $wgAbuseFilterParserClass

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