And what's even more weird, I cannot reproduce the problem locally. Sure, I had to change the affected filters a little bit in order to make them match, but yet, they match my edits correctly. I'll investigate further.

OK, so, additionally, we have some weird caching-related problem or the like. After having unsuccessfully tried testing locally and on itwiki itself, I came up with the following test: make the broken filter match my edit with a different IP address, copy its content to a brand new filter, and edit a page. Surprisingly, despite having the very same content, the new filter matched, while the old one did not.
So I suspect we have either caching problems (e.g. with tokenization or the like), or something like that. Note that editing the broken filter is not enough to "refresh something" or make it start working again.

In T230256#5407119, @Daimona wrote:

OK, so, additionally, we have some weird caching-related problem or the like. After having unsuccessfully tried testing locally and on itwiki itself, I came up with the following test: make the broken filter match my edit with a different IP address, copy its content to a brand new filter, and edit a page. Surprisingly, despite having the very same content, the new filter matched, while the old one did not.
So I suspect we have either caching problems (e.g. with tokenization or the like), or something like that. Note that editing the broken filter is not enough to "refresh something" or make it start working again.

Oh my goodness... I forgot to enable the "broken" filter after fixing it... So the actual explanation for that problem is me being stoopid, no more and no less.

Note that the problem in the task description is still a thing, and I still don't know what the affected filters are. Instead, the part saying that you should go to Special:AbuseFilter/test to verify the bug could be wrong. I learned that in T228526, and it was the same for the itwiki filter, but for the latter it's just because the filter was disabled, and /test only checks the pattern regardless of whether the filter is enabled.

Change 529598 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add a 'strict' option to VariableHolder::getVar

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

Change 529601 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Add a new 'is_set' function

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

@Daimona So I tried to write a summary in Tech News, but there's enough information that I think people actually need to read the ticket. But something like this?

"Some abusefilters stopped working because of a change. Only variables for the current action will work. Variables defined inside a branch may not work outside of that branch. You can [[phab:T230256|read more]].

In T230256#5415643, @Johan wrote:

@Daimona So I tried to write a summary in Tech News, but there's enough information that I think people actually need to read the ticket.

Thanks! Unfortunately yes, there's too much information to be able to summarize it in the announcement only.

"Some abusefilters stopped working because of a change. Only variables for the current action will work. Variables defined inside a branch may not work outside of that branch. You can [[phab:T230256|read more]].

Looks good. Maybe specify "stopped working because of a backend change", so that maybe it's clearer that it's not an on-wiki change made by some user? And then I could clean up the task description to provide a short summary for people who click the link.

A list would truly be useful.

In T230256#5423934, @Bencemac wrote:

A list would truly be useful.

Yes, I know, but at the moment there's no way to do that. I'll add some logging to the parser in order to do that.

Ah, right, I already sent T230256#5408092 to get a list and T230256#5408110 as helper. Note that the only filter known to be affected because of custom variables used out of scope is arwiki's 130, for which I'm adding the parent task.

@Daimona I see now that user_rights for instance is no longer available for the createaccount action. This makes sense, based on your description, but the issue is we now have to handle createaccount and autocreateaccount separately, leading to much more complicated filters with more conditions.

Before:

action contains "createaccount" &
!('override-antispoof' in user_rights) &
accountname contains "Foobar"

After:

(
    action == "createaccount" |
    (
        action == "autocreateaccount" &
        !('override-antispoof' in user_rights)
    )
) &
accountname contains "Foobar"

Having to use a new function to check for the existence of a variable seems less ideal, too. Couldn't we make it so that the parser just returns a blank or null value when an unavailable variable is encountered? So in the above case, user_rights is blank, so !('override-antispoof' in user_rights) returns true and the filter doesn't break.

On enwiki, nearly all of the account creation-related filters appear to have broken.

In T230256#5439932, @MusikAnimal wrote:

On enwiki, nearly all of the account creation-related filters appear to have broken.

A quick scan shows the following filters that may need to be changed:

• https://en.wikipedia.org/wiki/Special:AbuseFilter/51
• https://en.wikipedia.org/wiki/Special:AbuseFilter/527
• https://en.wikipedia.org/wiki/Special:AbuseFilter/579
• https://en.wikipedia.org/wiki/Special:AbuseFilter/874
• https://en.wikipedia.org/wiki/Special:AbuseFilter/887
• https://en.wikipedia.org/wiki/Special:AbuseFilter/890
• https://en.wikipedia.org/wiki/Special:AbuseFilter/978
• https://en.wikipedia.org/wiki/Special:AbuseFilter/998

In T230256#5439932, @MusikAnimal wrote:

@Daimona I see now that user_rights for instance is no longer available for the createaccount action.

Yes, this is correct.

This makes sense, based on your description, but the issue is we now have to handle createaccount and autocreateaccount separately, leading to much more complicated filters with more conditions.

Unfortunately, this is also true in some cases, i.e. when the filter is also using variables that aren't guaranteed to be set (e.g. user_groups).

Having to use a new function to check for the existence of a variable seems less ideal, too.

That's not really supposed to make things easier, it's just for cases where checking the action is not practical: for instance, instead of

( action === 'createaccount' | action === 'autocreateaccount' ) & accountname === 'xxx'

(yes, this could be written as action contains 'createaccount', but that wasn't intended, so let's pretend it doesn't work)

you could write

is_set( 'accountname' ) & accountname === 'xxx'

Couldn't we make it so that the parser just returns a blank or null value when an unavailable variable is encountered?

It used to return NULL. The problem with returning null is that, while filters could be simplified, they could behave unexpectedly. For instance (something similar has happened to me several times), suppose that you want to catch a serial page blanker. You may write something as

!"confirmed" in user_groups &
edit_delta < -5000

with throttle by page at a given rate, and enable the block. That could seem perfect. But it would start blocking *all* account creations, because you'd have

!"confirmed" in "" &
null < -5000 /* Yes, this is true, and it's the same in PHP */

the page being always Special:CreateAccount. That could be catastrophic if no-one notices it on time.
And I'm sure there are other cases of ambiguity. So basically, it's a trade-off between verbosity and unexpected behaviours.

On enwiki, nearly all of the account creation-related filters appear to have broken.

Sad to hear that. We currently don't have a list of involved filters. https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/529598/ would provide one, and then we could also understand how many filters are affected, and what to do. Which could be either keep returning null for back-compat until all filters are fixed, or even take a step back and start returning null again for unset variables. Would you mind reviewing the patch, please? It's not very complicated. Although I'm unsure whether we're on time to get it included in this week's train.
As a side note, I'd be happy to help in fixing broken filters, but this is yet another case where the new global group would be super-helpful.

Change 529598 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Add a 'strict' option to VariableHolder::getVar

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

Change 536166 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Better logging for unset variables

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

Change 536166 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Better logging for unset variables

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

OK, so, there are lots of affected filters. Searching channel:AbuseFilter AND "unset variable" for the last 15 minutes yield 150000 results. Hence, I think we should take a step back, and keep returning NULL in strict mode.

Most of the log entries could be resolved by checking the action (e.g. action==='move'), but I see that it's subpar to do that for all filters.

I'm not happy with NULL, due to the ambiguities it could cause. But again, I think it's our only option given the amount of "broken" filters.

Change 538598 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Return a DNULL in VariableHolder::getVar with GET_STRICT

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

Summing up my comments on gerrit:

I don't like this change because it can cause unexpected behaviour at runtime

(e.g. edit_delta < -5000 will match any non-edit action)

However, this is necessary due to the high amount of affected filters.

For other examples of stuff that can match at runtime when the expected variables are unset, see this data provider; specifically, test sets 2, 3, 4, 7, 14, 15, 17, and 18 (0-based).

I understand that having action === 'xxx' checks in all filters may seem unnecessary. Although some communities do that for all filters, most people do not (myself included, I should say). However, I also think that returning NULL is too false-positive-prone. Hence, ideally we should find a way to update all the existing filters to check for unset variables.

Change 628322 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] parser: Add a BC option to get DNULL for unset variables

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

Change 538598 abandoned by Daimona Eaytoy:
[mediawiki/extensions/AbuseFilter@master] Return a DNULL in VariableHolder::getVar with GET_STRICT

Reason:
This was conceptually wrong. I've resubmitted this as I4d06303047397674c1edbfc32628f1bc83ac3340, which adds a new mode rather than hacking GET_STRICT.

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

Change 628554 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_35] parser: Add a BC option to get DNULL for unset variables

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

Change 628554 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_35] parser: Add a BC option to get DNULL for unset variables

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

Change 628322 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] parser: Add a BC option to get DNULL for unset variables

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

Message idea for Tech News:

[[Tech/News/2019/34|Last year]] a backend change caused some abusefilters to stop working. Using variables not available for the current action would make the filter fail unexpectedly. This was now resolved. See [[:phab:T230256|]]