Page MenuHomePhabricator

AbuseFilter and SpamBlacklist return a "successful" action=edit API response when they prevent edits
Closed, ResolvedPublic

Description

AbuseFilter and SpamBlacklist return a "successful" action=edit API response when they prevent edits. Here's an example for AbuseFilter:

{
    "edit": {
        "code": "abusefilter-disallowed",
        "message": {
            "key": "abusefilter-disallowed",
            "params": [ ... ]
        },
        "abusefilter": { ... },
        "info": "Hit AbuseFilter: Test filter disallow",
        "warning": "This action has been automatically identified ...",
        "result": "Failure"
    }
}

Other than being annoyingly inconsistent, this output doesn't support the errorformat parameter.

This is because they use 'apiHookResult' property to define the error message, and they should instead use ApiMessage.

For comparison, a 'readonly' error generated by MediaWiki (with errorformat=html):

{
    "errors": [
        {
            "code": "readonly",
            "data": {
                "readonlyreason": "foo bar"
            },
            "module": "main",
            "*": "The wiki is currently in read-only mode."
        }
    ],
    "*": "See http://localhost:3080/w/api.php for API usage. ..."
}

For another comparison, an error generated by TitleBlacklist (with errorformat=html):

{
    "errors": [
        {
            "code": "titleblacklist-forbidden",
            "data": {
                "message": {
                    "key": "titleblacklist-forbidden-edit",
                    "params": [ ...]
                },
                "line": ...,
                "info": "TitleBlacklist prevents this title from being created"
            },
            "module": "edit",
            "*": "The title ..."
        }
    ],
    "*": "See http://localhost:3080/w/api.php for API usage. ..."
}

Related Objects

Event Timeline

matmarex created this task.Aug 1 2019, 3:20 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 1 2019, 3:20 AM

Change 526828 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/AbuseFilter@master] [WIP] Actually return errors for action=edit API

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

Change 526829 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/SpamBlacklist@master] [WIP] Actually return errors for action=edit API

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

Restricted Application added a project: Platform Engineering. · View Herald TranscriptAug 1 2019, 3:34 AM
mobrovac added a subscriber: mobrovac.

Moving back to CPT's backlog since the PS' are WIPs. @matmarex feel free to ping us when you feel the patches are ready to be reviewed.

WDoranWMF triaged this task as Medium priority.Aug 14 2019, 1:58 PM

@mobrovac Turns out that we need to make some changes in VE first, since it depends on the current API output. However, I would already appreciate review on two smaller commits that are dependencies for this work: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SpamBlacklist/+/531302 https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SpamBlacklist/+/526816

(This depends on T229532, not the other way round. To fix this bug, we must fix T229532 first, otherwise VE will not be able to display pretty error messages for these extensions.)

@mobrovac All patches here are ready for review. Thanks!

While working on them, I also discovered an existing bug in MW core, with another patch that needs review: T231253.

matmarex claimed this task.Aug 26 2019, 9:59 PM
matmarex moved this task from Incoming to Code review on the VisualEditor (Current work) board.

Change 526828 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Actually return errors for action=edit API

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

Change 526829 merged by jenkins-bot:
[mediawiki/extensions/SpamBlacklist@master] Actually return errors for action=edit API

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

matmarex added a project: Skipped QA.

(QA from VisualEditor's perspective is covered by T229532)

ppelberg closed this task as Resolved.Oct 22 2019, 12:44 AM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:41 PM