Page MenuHomePhabricator

Argument 1 passed to MediaWiki\User\UserNameUtils::getCanonical() must be of the type string, null given, called in /srv/mediawiki/php-1.36.0-wmf.33/extensions/CentralAuth/includes/CentralAuthGroupMembershipProxy.php on line 48
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

MediaWiki version: 1.36.0-wmf.33

message
Argument 1 passed to MediaWiki\User\UserNameUtils::getCanonical() must be of the type string, null given, called in /srv/mediawiki/php-1.36.0-wmf.33/extensions/CentralAuth/includes/CentralAuthGroupMembershipProxy.php on line 48

Impact

Minor, for now.

Notes

Looks like a programming error to me. I admit any errors from authentication code makes me extra nervous.

Details

Request ID
YD9swO-QsBNzLVht7tYW2gAAAAE
Request URL
https://gn.wikibooks.org/wiki/Mba%27ech%C4%A9ch%C4%A9:Permisos_de_usuario_global
Stack Trace
exception.trace
from /srv/mediawiki/php-1.36.0-wmf.33/includes/user/UserNameUtils.php(258)
#0 /srv/mediawiki/php-1.36.0-wmf.33/extensions/CentralAuth/includes/CentralAuthGroupMembershipProxy.php(48): MediaWiki\User\UserNameUtils->getCanonical(NULL)
#1 /srv/mediawiki/php-1.36.0-wmf.33/extensions/CentralAuth/includes/specials/SpecialGlobalGroupMembership.php(106): CentralAuthGroupMembershipProxy::newFromName(NULL)
#2 /srv/mediawiki/php-1.36.0-wmf.33/includes/specials/SpecialUserrights.php(147): SpecialGlobalGroupMembership->fetchUser(NULL, boolean)
#3 /srv/mediawiki/php-1.36.0-wmf.33/includes/specialpage/SpecialPage.php(645): UserrightsPage->execute(NULL)
#4 /srv/mediawiki/php-1.36.0-wmf.33/includes/specialpage/SpecialPageFactory.php(1383): SpecialPage->run(NULL)
#5 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(309): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#6 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(925): MediaWiki->performRequest()
#7 /srv/mediawiki/php-1.36.0-wmf.33/includes/MediaWiki.php(547): MediaWiki->main()
#8 /srv/mediawiki/php-1.36.0-wmf.33/index.php(53): MediaWiki->run()
#9 /srv/mediawiki/php-1.36.0-wmf.33/index.php(46): wfIndexMain()
#10 /srv/mediawiki/w/index.php(3): require(string)
#11 {main}

Event Timeline

This seems to be happening when you try to view Special:GlobalUserRights without specifying the username. That special page is normally only used on meta, which is probably the reason why it hasn't happened more than a few times now.

LarsWirzenius triaged this task as Unbreak Now! priority.Mar 3 2021, 12:10 PM
LarsWirzenius added a project: Security.

Upon consideration, anything auth related is security related so raising priority and adding teams that can hopefully help.

In T276316#6878194, @Majavah wrote:

Looks like something related to 73ade52/T275030.

I'm investigating this hypothesis but at first glance, it's not related. According to the stack trace, the issue appeared much earlier:

exception.trace
#4 /srv/mediawiki/php-1.36.0-wmf.33/includes/specialpage/SpecialPageFactory.php(1383): SpecialPage->run(NULL)

And what I'm checking is that both UserNameUtils::getCanonical and User::getCanonical fail with the same error.

In T276316#6878194, @Majavah wrote:

Looks like something related to 73ade52/T275030.

I'm investigating this hypothesis but at first glance, it's not related. According to the stack trace, the issue appeared much earlier:

exception.trace
#4 /srv/mediawiki/php-1.36.0-wmf.33/includes/specialpage/SpecialPageFactory.php(1383): SpecialPage->run(NULL)

And what I'm checking is that both UserNameUtils::getCanonical and User::getCanonical fail with the same error.

git bisect suggests that the mentioned commit is related. With a2c867c6563b my local installation works just fine but with 73ade5297d0e I can reproduce this error. Reverting the changes in CentralAuthGroupMembershipProxy does fix the issue.

The main problem was that in User:getCanonicalName method we always passed string to UserNameUtils::getCanonical with using (string) before parameter. Please, wait 15 minutes I will create the patch which will fix it.

@Vlad.shapik how is it going?

5 minutes

I know CentralAuth is a pain to set up locally (well...anywhere). If you need any advice, I'm happy to help - just ping me on IRC (I'm Urbanecm there).

Change 668098 had a related patch set uploaded (by Vlad.shapik; owner: Vlad.shapik):
[mediawiki/extensions/CentralAuth@master] Transform the first parameter to string

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

The problem was fixed in this patch. Have a look at my changes, please.

Thanks, people on IRC are discussing it now.

Change 668048 had a related patch set uploaded (by Urbanecm; owner: Vlad.shapik):
[mediawiki/extensions/CentralAuth@wmf/1.36.0-wmf.33] Transform the first parameter to string

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

@Urbanecm is backporting the change to train. I'll leave this task open and marked as a train blocker so that we don't forget to finish the review that all necessary call sites have been fixed.

Change 668048 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@wmf/1.36.0-wmf.33] Transform the first parameter to string

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

Mentioned in SAL (#wikimedia-operations) [2021-03-03T15:13:14Z] <urbanecm@deploy1002> Synchronized php-1.36.0-wmf.33/extensions/CentralAuth/: rECAUaf899b681822: Transform the first parameter to string (T276316) (duration: 01m 11s)

I followed all possible code paths and the only bogus one I found is in core: https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/specials/SpecialUserrights.php$147. This calls SpecialGlobalGroupMembership::fetchUser() even if $this->mTarget is null. The (probably) relevant callstack looks something like this:

  • SpecialGlobalGroupMembership::fetchUser( null )
  • CentralAuthGroupMembershipProxy::newFromName( null )
  • UserNameUtils::getCanonical( null )

Since other people are already working on this, I will not make my own patch, but leave what I found here in this comment.

Thank you @thiemowmde for your comment.

@Pchelolo @Vlad.shapik For now, I only backported the patch to wmf.33, to let train roll forward, but I didn't merge the master patch, as I don't think adding the casts is a good final solution. I'd really appreciate if a final solution can be found before next train will start roll forward, as that would automatically bring this issue back. For that reason, I'm classifying this as a wmf.34 train blocker.

Thank you @Urbanecm. We will get a better fix in before 1.34.

Change 668205 had a related patch set uploaded (by Vlad.shapik; owner: Vlad.shapik):
[mediawiki/core@master] Prevent call of fetchUser method with null

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

Change 668205 merged by jenkins-bot:
[mediawiki/core@master] Prevent call of fetchUser method with null

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

Change 668098 abandoned by Ppchelko:
[mediawiki/extensions/CentralAuth@master] Transform the first parameter to string

Reason:
no longer required.

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

Pchelolo lowered the priority of this task from Unbreak Now! to High.Mar 5 2021, 2:48 AM

This is no longer a UBN since both a quick fix for wmf.33 and a proper fix for master were merged. Let's see if it reappears during the next week train and close.

I tested how new changes in the core influence resolving this production error. Now we don't have any exception traces. This error was localized.

brennen added a subscriber: brennen.

Removing as a train blocker since this appears to have been resolved before the wmf.34 branch cut.

Could we mark this task as resolved?