Page MenuHomePhabricator
Create Task
Maniphest T212730

Change the syntax for non-decimal numbers
Closed, ResolvedPublic
Actions

Description

The current syntax for non-decimal numbers is awful, undocumented and weak. We should devise a deprecation process and replace it with, for instance, the one used in PHP. I think there won't be many use cases, but we still need to be careful in order to avoid breaking existing filters.

Plan

  • Check production use with info logging
  • Introduce new syntax as experimental
  • Ensure that no filter in production is already unintendedly using the new syntax
  • Bump logging for old syntax to deprecated, in 1.34
  • Remove old path, leave only the new one, in 1.35

Details

SubjectRepoBranchLines +/-
Remove old number syntaxmediawiki/extensions/AbuseFiltermaster+8 -61
Also parse numbers with the new syntax and hard-deprecate the old onemediawiki/extensions/AbuseFilterREL1_34+38 -16
Also parse numbers with the new syntax and hard-deprecate the old onemediawiki/extensions/AbuseFiltermaster+38 -16
Add new number syntax as experimentalmediawiki/extensions/AbuseFiltermaster+19 -2
Move non-decimal numbers deprecation loggingmediawiki/extensions/AbuseFiltermaster+8 -8
Customize query in gerrit

Related Objects
Search...

Event Timeline

Daimona created this task.Dec 31 2018, 4:46 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 31 2018, 4:46 PM
Daimona added a subtask: T212726: "ex" evaluates to 14 in AbuseFilterParser.Dec 31 2018, 4:47 PM
Daimona moved this task from Backlog to Future on the User-Daimona board.
Liuxinyu970226 awarded a token.Jan 1 2019, 12:48 AM
Liuxinyu970226 subscribed.
Daimona mentioned this in T214674: Short circuit fails with assignments.Jan 25 2019, 10:15 AM
Daimona closed subtask T212726: "ex" evaluates to 14 in AbuseFilterParser as Resolved.Jul 16 2019, 12:28 PM
Daimona added a comment.Jul 16 2019, 4:36 PM

Some data has started to flow to logstash for group0, and we have another example: testwiki filter 176, using the variable "db", evaluated to the number "b" in base 2. So well, we really need this change.

Daimona added a comment.Aug 3 2019, 4:56 PM

I see there are quite a lot of notices on logstash. However, I have to say that not all logged cases are actually recognized as numbers, because we check if the digits are OK. So for instance, "b" won't be considered valid in base 2 (while it would be valid hex).
Now I'm going to move the logging to a place where we already know if what we found is a valid number.

gerritbot added a comment.Aug 3 2019, 4:56 PM

Change 527849 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Move non-decimal numbers deprecation logging

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

gerritbot added a project: Patch-For-Review.Aug 3 2019, 4:56 PM
Daimona claimed this task.Aug 3 2019, 4:58 PM
gerritbot added a comment.Aug 3 2019, 6:18 PM

Change 527849 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Move non-decimal numbers deprecation logging

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

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.17; 2019-08-06).Aug 3 2019, 7:00 PM
Maintenance_bot removed a project: Patch-For-Review.Aug 3 2019, 7:10 PM
Huji subscribed.Aug 3 2019, 8:29 PM

Obviously, keeping this open despite the merged patch, as the patch only helps with better logging of events and the problem persists.

Daimona added a comment.Aug 4 2019, 10:50 PM

Indeed. I'll examine logs again next week, hoping to find some useful data without all of those false positives. I still don't know how to proceed at that point, but anyway I'd like to reimplement non-decimal numbers like in PHP.
I'll probably add an alternate code path that will try to parse a number using this new syntax, report whether it succeeds or not, and again check the logs. If no filter was using non-decimal numbers (intendedly) and no filter is already using the new syntax for something else, then we may raise the logging to WARNING level, formally deprecating the old syntax. And then we'll leave the new syntax only in the next MW version.

Daimona added a parent task: T230263: Tasks requiring manual edits of existing abuse filters in production (tracking).Aug 10 2019, 2:35 PM
Daimona added a comment.Aug 10 2019, 2:59 PM

No occurrences on logstash since wmf.17 reached production. I'll proceed as described in T212730#5391054.

gerritbot added a comment.Aug 10 2019, 4:56 PM

Change 529475 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add new number syntax as experimental

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

gerritbot added a project: Patch-For-Review.Aug 10 2019, 4:56 PM
Daimona moved this task from Future to Under review on the User-Daimona board.Aug 13 2019, 3:20 PM
Daimona moved this task from Backlog to Requiring manual fix of filters in production [SUPERSEDED BY #wikimedia-abusefilter-maintainers] on the AbuseFilter board.Aug 21 2019, 12:18 PM
Daimona removed a parent task: T230263: Tasks requiring manual edits of existing abuse filters in production (tracking).
gerritbot added a comment.Aug 31 2019, 5:12 PM

Change 529475 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add new number syntax as experimental

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

ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.21; 2019-09-03); removed MW-1.34-notes (1.34.0-wmf.17; 2019-08-06).Aug 31 2019, 6:00 PM
Maintenance_bot removed a project: Patch-For-Review.Aug 31 2019, 6:10 PM
Daimona updated the task description. (Show Details)Aug 31 2019, 6:38 PM
Daimona updated the task description. (Show Details)Sep 7 2019, 11:03 AM
Daimona moved this task from Requiring manual fix of filters in production [SUPERSEDED BY #wikimedia-abusefilter-maintainers] to Backlog on the AbuseFilter board.
gerritbot added a comment.Sep 7 2019, 11:37 AM

Change 534903 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Parse numbers with the new syntax, too

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

gerritbot added a project: Patch-For-Review.Sep 7 2019, 11:37 AM
Daimona added a project: MW-1.34-release.Sep 17 2019, 1:34 PM
Daimona moved this task from Blocker to Deprecate/Remove on the MW-1.34-release board.

The deprecation should happen in 1.34.

Daimona updated the task description. (Show Details)Sep 23 2019, 12:18 PM
Daimona mentioned this in T234058: Review new AbuseFilter hard deprecations ahead of 1.34 release.Sep 27 2019, 4:15 PM
Krinkle removed a project: MW-1.34-release.Nov 6 2019, 12:37 AM
gerritbot added a comment.Nov 12 2019, 12:02 PM

Change 534903 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Also parse numbers with the new syntax and hard-deprecate the old one

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

Maintenance_bot removed a project: Patch-For-Review.Nov 12 2019, 12:11 PM
ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.8; 2019-11-26).Nov 12 2019, 1:00 PM
Nullzero closed this task as Resolved.Nov 12 2019, 6:20 PM
gerritbot added a comment.Nov 12 2019, 6:32 PM

Change 550529 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_34] Also parse numbers with the new syntax and hard-deprecate the old one

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

gerritbot added a project: Patch-For-Review.Nov 12 2019, 6:32 PM
Daimona reopened this task as Open.Nov 12 2019, 6:35 PM

Not yet, I'm sorry. For now it's only experimental.

Daimona updated the task description. (Show Details)Nov 12 2019, 6:35 PM
gerritbot added a comment.Nov 13 2019, 11:36 AM

Change 550529 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_34] Also parse numbers with the new syntax and hard-deprecate the old one

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

Maintenance_bot removed a project: Patch-For-Review.Nov 13 2019, 12:10 PM
gerritbot added a comment.Nov 13 2019, 1:27 PM

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

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

gerritbot added a project: Patch-For-Review.Nov 13 2019, 1:27 PM
gerritbot added a comment.Feb 25 2020, 11:54 PM

Change 550677 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Remove old number syntax

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

Jdforrester-WMF closed this task as Resolved.Feb 25 2020, 11:56 PM
Jdforrester-WMF updated the task description. (Show Details)
Maintenance_bot removed a project: Patch-For-Review.Feb 26 2020, 12:10 AM
ReleaseTaggerBot edited projects, added MW-1.35-notes (1.35.0-wmf.22; 2020-03-03); removed MW-1.35-notes (1.35.0-wmf.8; 2019-11-26).Feb 26 2020, 4:02 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL