Page MenuHomePhabricator

Correctly validate action parameters server-side when saving a filter
Closed, ResolvedPublic

Description

Copying my comment from T203336:

  1. As specified in the patch commit message, we still need a follow-up to enforce validation on other fields, both client- and server-side. Currently, this includes:
    1. Warning (server-side and client-side): select "Other message" and leave the second field blank. This also produces a PHP notice for undefined offset 0 in ViewEdit (look for $warnMsg = $parameters[0];)
    2. Throttle (client-side): the same as we do server-side in the patch above
    3. Tag (client-side): mostly needs to clearly show that at least one tag is needed. Also, when saving a filter with tag enabled but without touching the tag input field correctly returns an error but an empty tag is added in the field topbar; this shouldn't probably happen, but it's a minor problem.

In short, we need to properly validate (both server- and client-side) action parameters to avoid errors like that. Client-side validation has its own task, T196984.

Event Timeline

Change 457394 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Reject empty warning messages when validating a filter

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

The "warning" part is the only one to do in this task. However, as I was saying in the commit message, this opens a new follow-up: currently, when executing the "warn" action, we check if the specified message is a string with length > 0. If it isn't, then the "abusefilter-warning" is used. The patch above prevents empty messages, so as soon as every invalid filter will be modified adding a message, above check won't be necessary. This means that, as for T203359, the code could be simplified, but it can't be done right now. Again, should we add a wfDeprecated for those cases?

Change 457424 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Overhaul tag selector

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

Change 457394 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Reject empty warning and disallow messages when validating a filter

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

Change 457424 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Overhaul tag selector

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