Page MenuHomePhabricator

Content for AbuseFilter was transformed into JSON
Closed, ResolvedPublic

Description

In these logs, new_wikitext and added_lines have JSON instead of the entity serialization in old_wikitext:

I cannot say under what conditions this happens though KasparBot triggered a filter many times as a result of this bug.

Details

Related Gerrit Patches:
mediawiki/extensions/AbuseFilter : masterDon't filter undos coming in via the "APIEditBeforeSave" hook

Event Timeline

matej_suchanek raised the priority of this task from to Needs Triage.
matej_suchanek updated the task description. (Show Details)
matej_suchanek added a subscriber: matej_suchanek.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 13 2016, 11:26 AM
matej_suchanek set Security to None.
Lydia_Pintscher added a subscriber: Lydia_Pintscher.

How bad is this?

Two of the linked changes are rollbacks... aren't rollbacks excluded from AbuseFilter at all? This could also indicate a bug.

hoo added a comment.EditedFeb 18 2016, 5:09 PM

I could reproduce this by undoing a content object using the api with action=edit&undo=…&undoafter=…. Will look into this further.

These weren't actual rollbacks, but emulated ones by @Ricordisamoa's https://www.wikidata.org/wiki/User:Ricordisamoa/Rollback.js.

hoo claimed this task.EditedFeb 18 2016, 5:28 PM
hoo added projects: AbuseFilter, MediaWiki-API.
hoo added subscribers: aude, daniel, Anomie.

The problem here got visible with @aude's a225cbdca89563e878c19580fb04c9e1d805d6c7 which made it possible to use ApiEditPage for undoing edits on entity content.

ApiEditPage always runs APIEditBeforeSave, even when undoing an edit. That is inconsistent with for example EditFilterMergedContent.

I see two ways to fix this:

  • Don't run the hook on undo (this is a breaking change, but would make the behavior consistent with EditFilterMergedContent)
  • Make AbuseFilter ignore undos coming in via that hook

Whatever we choose to do, we should document the (new) behavior of the hook properly (right now its not obvious whether it's supposed to run on undo or not).

Ricordisamoa added a comment.EditedFeb 18 2016, 5:30 PM

These weren't actual rollbacks, but emulated ones by @Ricordisamoa's https://www.wikidata.org/wiki/User:Ricordisamoa/Rollback.js.

Ha! I knew it was going to mess up one day... (only using undo since Jan 23)

hoo added a comment.Feb 18 2016, 5:40 PM

@Anomie Given you moved this to not a core issue, I suspect you say that the core behavior shouldn't be altered?

Yeah, it makes more sense to me that the "ApiEditBeforeSave" hook would be called for all API action=edit calls, including undo.

Whatever conversion is applied to new_wikitext and added_lines, can't it be applied regardless of the action?

Change 278071 had a related patch set uploaded (by Hoo man):
Don't filter undos coming in via the "APIEditBeforeSave" hook

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

Change 278071 merged by jenkins-bot:
Don't filter undos coming in via the "APIEditBeforeSave" hook

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

hoo closed this task as Resolved.Mar 18 2016, 4:54 PM
hoo removed a project: Patch-For-Review.