Page MenuHomePhabricator
Create Task
Maniphest T228677

use of get_matches function returns "Requesting array item of non-array"
Closed, ResolvedPublic
Actions

Description

The get_matches function returns an array, but as of very recently I'm unable to use it like an array.

Here are long-standing filters that now throw syntax errors:

Basically we're doing match := get_matches("regex", added_lines)[1].

get_matches can return false if there are no matches, in which case it's of course invalid to treat it like an array. But up until now I think AbuseFilter gracefully failed, which is more than fine in my case (since the filter would evaluate to false anyway).

Details

SubjectRepoBranchLines +/-
Revert "Better handling of DNONE"mediawiki/extensions/AbuseFilterwmf/1.34.0-wmf.16+66 -273
Better handling of DNONEmediawiki/extensions/AbuseFilterwmf/1.34.0-wmf.16+273 -66
Better handling of DNONEmediawiki/extensions/AbuseFiltermaster+273 -66
Revert "Better handling of DNONE"mediawiki/extensions/AbuseFilterwmf/1.34-wmf.16+66 -273
Better handling of DNONEmediawiki/extensions/AbuseFilterwmf/1.34-wmf.16+273 -66
Customize query in gerrit

Related Objects

Event Timeline

MusikAnimal created this task.Jul 22 2019, 3:52 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 22 2019, 3:52 PM
Daimona subscribed.Jul 22 2019, 3:58 PM

Thanks for reporting, I do see some "Requesting array item of non-array" errors on logstash for get_matches... I believe a potential culprit is https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/478424/. I'll investigate a bit further, although it should be easy to test locally.

Daimona added a comment.Jul 22 2019, 4:03 PM

Yeah, that's probably the culprit. Some of the fixes done in https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/522763/ should have been included there, but I only discovered them later. And maybe, there are other places where we should add a check for DNONE. @MusikAnimal Could you please test locally if applying the patch above fixes the error? I'm unable to do so right now :-/

MusikAnimal triaged this task as High priority.Jul 22 2019, 8:35 PM

I can try testing your patch, sure. I'm going to bump this to High, hope that's okay. https://en.wikipedia.org/wiki/Special:AbuseFilter/944 is an important filter that we need to update every day. I see the last modification was today at 00:08, so I guess this bug was introduced this morning, or perhaps that user somehow bypassed the syntax checker? I've definitely made changes to this filter since July 16, when https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/478424/ was apparently deployed.

Daimona added a comment.Jul 22 2019, 10:05 PM

Yeah, fine. The syntax checker is bypassed because it's poorly implemented. What it does is disable shortcircuit and try to run the filter. This way, shortcircuit-only bugs will only be discovered at runtime. Other examples are T214674, T214642 and T218906, and T214643 is the wider task about syntax checking.

Also, given the nature of this bug, if the shortcircuit causing the bug is for consecutive &s, the filter wouldn't match anyway. I haven't checked the specific case, though.

Anyway, I believe that the patch above should fix the error. Otherwise I'll create another one, if there's another edge-case that was left out.

MusikAnimal added a comment.Jul 22 2019, 11:02 PM

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/522763/ indeed fixes the bug :) I'm not familiar with the parser code so I'm afraid I'll have to leave the +2'ing to someone else, though.

Daimona claimed this task.Jul 23 2019, 10:14 AM
In T228677#5355735, @MusikAnimal wrote:

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/522763/ indeed fixes the bug :) I'm not familiar with the parser code so I'm afraid I'll have to leave the +2'ing to someone else, though.

Cool, thanks :-)

MusikAnimal mentioned this in T214674: Short circuit fails with assignments.Jul 23 2019, 3:52 PM
Daimona added a comment.Aug 1 2019, 9:15 PM

Meh, I just realized that the "errors" this task is about happen when trying to update the filter, or check the syntax, and not at runtime (i.e. reported on logstash). And other filters have this problem too, e.g. some filters on itwiki using get_matches. Solving T228677 will solve this one as well, and I'm totally on it. I'm keeping this task open, though, as it may be non-obvious that these tasks are related.

Of note, this one is a different issue which I coincidentally tried to fix together with the other task. The reason I'm not doing this one separately is that, even if we fix this bug, we may face worse troubles, like filters matching for no reason and potentially blocking lots of users like on itwp.

gerritbot added a comment.Aug 2 2019, 11:49 AM

Change 527520 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] [WIP] Better handling of DNONE

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

gerritbot added a project: Patch-For-Review.Aug 2 2019, 11:49 AM
gerritbot added a comment.Aug 2 2019, 10:55 PM

Change 527520 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Better handling of DNONE

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

Daimona removed a project: Patch-For-Review.Aug 2 2019, 10:57 PM

Leaving open until production deployment, just to be sure.

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.17; 2019-08-06).Aug 2 2019, 11:00 PM
gerritbot added a comment.Aug 5 2019, 11:06 AM

Change 528108 had a related patch set uploaded (by Urbanecm; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.34-wmf.16] Better handling of DNONE

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

gerritbot added a project: Patch-For-Review.Aug 5 2019, 11:06 AM
gerritbot added a comment.Aug 5 2019, 11:44 AM

Change 528108 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@wmf/1.34-wmf.16] Better handling of DNONE

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

gerritbot added a comment.Aug 5 2019, 11:59 AM

Change 528111 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/extensions/AbuseFilter@wmf/1.34-wmf.16] Revert "Better handling of DNONE"

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

gerritbot added a comment.Aug 5 2019, 11:59 AM

Change 528111 merged by Urbanecm:
[mediawiki/extensions/AbuseFilter@wmf/1.34-wmf.16] Revert "Better handling of DNONE"

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

Maintenance_bot removed a project: Patch-For-Review.Aug 5 2019, 12:10 PM
gerritbot added a comment.Aug 5 2019, 6:25 PM

Change 528211 had a related patch set uploaded (by Urbanecm; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.34.0-wmf.16] Better handling of DNONE

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

gerritbot added a project: Patch-For-Review.Aug 5 2019, 6:25 PM

Change 528211 merged by Urbanecm:
[mediawiki/extensions/AbuseFilter@wmf/1.34.0-wmf.16] Better handling of DNONE

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

Stashbot added a comment.Aug 5 2019, 6:32 PM

Mentioned in SAL (#wikimedia-operations) [2019-08-05T18:32:22Z] <urbanecm@deploy1001> Synchronized php-1.34.0-wmf.16/extensions/AbuseFilter/: SWAT: rEABF936a462daa0d: Better handling of DNONE (T214674, T228677) (duration: 00m 47s)

gerritbot added a comment.Aug 5 2019, 6:37 PM

Change 528212 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/extensions/AbuseFilter@wmf/1.34.0-wmf.16] Revert "Better handling of DNONE"

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

gerritbot added a comment.Aug 5 2019, 6:38 PM

Change 528212 merged by Urbanecm:
[mediawiki/extensions/AbuseFilter@wmf/1.34.0-wmf.16] Revert "Better handling of DNONE"

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

Stashbot added a comment.Aug 5 2019, 6:39 PM

Mentioned in SAL (#wikimedia-operations) [2019-08-05T18:39:44Z] <urbanecm@deploy1001> Synchronized php-1.34.0-wmf.16/extensions/AbuseFilter/: SWAT: rEABFd358f17dd615: Revert "Better handling of DNONE" (T214674, T228677) (duration: 00m 47s)

Maintenance_bot removed a project: Patch-For-Review.Aug 5 2019, 7:10 PM
Stashbot added a comment.Aug 6 2019, 11:21 AM

Mentioned in SAL (#wikimedia-operations) [2019-08-06T11:21:38Z] <urbanecm@deploy1001> Synchronized php-1.34.0-wmf.16/extensions/AbuseFilter/: SWAT: rEABF8cc96db45b64: Better handling of DNONE (T214674, T228677) (duration: 00m 48s)

Daimona closed this task as Resolved.Aug 6 2019, 11:36 AM
valhallasw mentioned this in T230698: ReleaseTaggerBot hasn't tagged anything since 8 August 2019.Aug 19 2019, 6:50 PM
ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.16; 2019-07-30); removed MW-1.34-notes (1.34.0-wmf.17; 2019-08-06).Aug 20 2019, 10:05 AM
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