Page MenuHomePhabricator

UserGroupMembership::insert() needs a positive user ID exception breaking AbuseFilter block function
Closed, ResolvedPublic

Description

2017-04-14 21:52:59 [WPFEuwpAAD8AAJfhhygAAABC] mw1268 trwiki 1.29.0-wmf.20 exception ERROR: [WPFEuwpAAD8AAJfhhygAAABC] /w/index.php?title=Vikipedi:Deneme_tahtas%C4%B1&action=submit   UnexpectedValueException from line 200 of /srv/mediawiki/php-1.29.0-wmf.20/includes/user/UserGroupMembership.php: UserGroupMembership::insert() needs a positive user ID. Did you forget to add your User object to the database before calling addGroup()? {"exception_id":"WPFEuwpAAD8AAJfhhygAAABC","caught_by":"mwe_handler"} 
[Exception UnexpectedValueException] (/srv/mediawiki/php-1.29.0-wmf.20/includes/user/UserGroupMembership.php:200) UserGroupMembership::insert() needs a positive user ID. Did you forget to add your User object to the database before calling addGroup()?
  #0 /srv/mediawiki/php-1.29.0-wmf.20/includes/user/User.php(3413): UserGroupMembership->insert(boolean)
  #1 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/includes/AbuseFilter.class.php(1879): User->addGroup(string)
  #2 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/includes/AbuseFilter.class.php(1537): AbuseFilter::getFilterUser()
  #3 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/includes/AbuseFilter.class.php(1395): AbuseFilter::doAbuseFilterBlock(array, string, string, boolean)
  #4 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/includes/AbuseFilter.class.php(837): AbuseFilter::takeConsequenceAction(string, array, Title, AbuseFilterVariableHolder, string, integer)
  #5 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/includes/AbuseFilter.class.php(948): AbuseFilter::executeFilterActions(array, Title, AbuseFilterVariableHolder)
  #6 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/AbuseFilter.hooks.php(121): AbuseFilter::filterAction(AbuseFilterVariableHolder, Title)
  #7 /srv/mediawiki/php-1.29.0-wmf.20/extensions/AbuseFilter/AbuseFilter.hooks.php(54): AbuseFilterHooks::filterEdit(RequestContext, WikitextContent, string, Status, string, boolean)
  #8 /srv/mediawiki/php-1.29.0-wmf.20/includes/Hooks.php(186): AbuseFilterHooks::onEditFilterMergedContent(RequestContext, WikitextContent, Status, string, User, boolean)
  #9 /srv/mediawiki/php-1.29.0-wmf.20/includes/EditPage.php(1632): Hooks::run(string, array)
  #10 /srv/mediawiki/php-1.29.0-wmf.20/includes/EditPage.php(2046): EditPage->runPostMergeFilters(WikitextContent, Status, User)
  #11 /srv/mediawiki/php-1.29.0-wmf.20/includes/EditPage.php(1473): EditPage->internalAttemptSave(NULL, boolean)
  #12 /srv/mediawiki/php-1.29.0-wmf.20/includes/EditPage.php(620): EditPage->attemptSave(NULL)
  #13 /srv/mediawiki/php-1.29.0-wmf.20/includes/actions/EditAction.php(59): EditPage->edit()
  #14 /srv/mediawiki/php-1.29.0-wmf.20/includes/actions/SubmitAction.php(38): EditAction->show()
  #15 /srv/mediawiki/php-1.29.0-wmf.20/includes/MediaWiki.php(498): SubmitAction->show()
  #16 /srv/mediawiki/php-1.29.0-wmf.20/includes/MediaWiki.php(292): MediaWiki->performAction(Article, Title)
  #17 /srv/mediawiki/php-1.29.0-wmf.20/includes/MediaWiki.php(861): MediaWiki->performRequest()
  #18 /srv/mediawiki/php-1.29.0-wmf.20/includes/MediaWiki.php(522): MediaWiki->main()
  #19 /srv/mediawiki/php-1.29.0-wmf.20/index.php(43): MediaWiki->run()
  #20 /srv/mediawiki/w/index.php(3): include(string)
  #21 {main}

Event Timeline

Reedy created this task.Apr 14 2017, 10:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 14 2017, 10:06 PM

The blocking feature of AbuseFilter does not work due to this problem, and we're under constant attack from our notorious vandal. I'd appreciate it if this could be treated a high-priority task.

Legoktm triaged this task as Unbreak Now! priority.Apr 16 2017, 7:10 AM
Restricted Application added subscribers: Jay8g, TerraCodes. · View Herald TranscriptApr 16 2017, 7:10 AM
Legoktm renamed this task from UserGroupMembership::insert() needs a positive user ID. Did you forget to add your User object to the database before calling addGroup() to UserGroupMembership::insert() needs a positive user ID exception breaking AbuseFilter block function.Apr 16 2017, 7:11 AM

+@Anomie and @TTO since this is related to the UserGroupMembership stuff. I think I have a band-aid patch that should mitigate most of this?

Change 348322 had a related patch set uploaded (by Legoktm):
[mediawiki/extensions/AbuseFilter@master] Only add 'sysop' group to filter user if not in it

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

I'm not seeing how that patch would fix the bug reported here (although the patch does seem like a good idea on its own). Nor am I seeing how the exception is occurring in the first place, since User::newSystemUser() should never be returning a user that isn't already in the database and therefore having a positive ID.

Further, I can't seem to reproduce the exception by calling AbuseFilter::getFilterUser() from mwrepl, which I'd think would work to reproduce it.

It happened when we enabled block for a vandalism filter, and @Mavrikant triggered it (while logged-out) with a test edit. If you need us to reproduce it for testing purposes, just say the word.

I'm not seeing how that patch would fix the bug reported here (although the patch does seem like a good idea on its own).

It would avoid calling the code that is causing the exception to be thrown in nearly all cases. On trwp it's already flagged as a sysop (https://tr.wikipedia.org/wiki/%C3%96zel:Kullan%C4%B1c%C4%B1Haklar%C4%B1/K%C3%B6t%C3%BCye_kullan%C4%B1m_s%C3%BCzgeci) so it should fix it for them.

But what was the state of the user before it was created on 17 April 2017 at 14:40? Is the error even still occurring now that the user has somehow been created successfully? I see no logs of the error on fluorine mwlog1001 since 2017-04-17 14:30:45.

I'm delighted to report that the patch has fixed the problem. I've successfully managed to have the account block me while not logged-in. Thank you, @Legoktm.

@Anomie: The user was not registered before the patch was deployed.

I'm delighted to report that the patch has fixed the problem. I've successfully managed to have the account block me while not logged-in. Thank you, @Legoktm.

The patch hasn't been deployed, so there's no way it could have fixed the problem...

Oh right, sorry. Which action did trigger the account creation then?

Possibly when I tried to reproduce it from mwrepl.

TTO lowered the priority of this task from Unbreak Now! to High.Apr 18 2017, 7:25 AM

Not UBN! anymore.

Change 348322 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Only add 'sysop' group to filter user if not in it

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

Restricted Application added a subscriber: Daimona. · View Herald TranscriptSep 2 2018, 10:42 AM
Daimona lowered the priority of this task from High to Low.EditedSep 2 2018, 10:53 AM

Definitely not high: no-one working on it, nor it seems to be happening anymore.

Krinkle closed this task as Resolved.Sep 6 2018, 11:00 PM
Krinkle assigned this task to Anomie.
Krinkle added a subscriber: Krinkle.

Only 4 instances of UnexpectedValueException in the last 30 days, and they're not related to this task.

mmodell changed the subtype of this task from "Task" to "Production Error".Wed, Aug 28, 11:10 PM