Page MenuHomePhabricator

The `UserrightsPage` class is not safe to extend
Open, MediumPublic

Description

UserrightsPage (SpecialUserrights.php) is not safe to extend, but it is extended by:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

So, what do you recommend? Mark it as @stable to extend?

So, what do you recommend? Mark it as @stable to extend?

No, I recommend splitting out whatever functionality needs to be available to extensions to a separate class/trait/service

Change 642859 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Revert "Inject services into SpecialUserrights"

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

@daniel @Naike would you be willing to reassess your evaluation of this as a future initiative / something for the icebox. The above revert isn't the first time that a change to SpecialUserrights has broken things for CentralAuth

From the latest revert

the problem is that SpecialGlobalGroupMembership doesn't actually call parent::__construct() and so none of the fallbacks provided for in the constructor were used, resulting in

[X7xOV6wQBHcAAE5b7KoAAAAC] /wiki/Special:GlobalUserRights/DannyS712 Error from line 143 of /srv/mediawiki/php-master/includes/specials/SpecialUserrights.php: Call to a member function getCanonical() on null

Backtrace:

#0 /srv/mediawiki/php-master/includes/specialpage/SpecialPage.php(639): UserrightsPage->execute(string)
#1 /srv/mediawiki/php-master/includes/specialpage/SpecialPageFactory.php(1272): SpecialPage->run(string)
#2 /srv/mediawiki/php-master/includes/MediaWiki.php(310): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#3 /srv/mediawiki/php-master/includes/MediaWiki.php(945): MediaWiki->performRequest()
#4 /srv/mediawiki/php-master/includes/MediaWiki.php(548): MediaWiki->main()
#5 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#6 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#7 /srv/mediawiki/w/index.php(3): require(string)
#8 {main}

Change 642859 merged by jenkins-bot:
[mediawiki/core@master] Revert "Inject services into SpecialUserrights"

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

Change 643299 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/CentralAuth@master] Prepare special page constructor for service injection in core

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

Change 643302 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/GlobalUserrights@master] Prepare special page constructor for service injection in core

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

Change 643299 merged by jenkins-bot:
[mediawiki/extensions/CentralAuth@master] Prepare special page constructor for service injection in core

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

Change 643155 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] Reapply "Inject services into SpecialUserrights"

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

Change 643302 merged by jenkins-bot:
[mediawiki/extensions/GlobalUserrights@master] Prepare special page constructor for service injection in core

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

Change 643155 merged by jenkins-bot:
[mediawiki/core@master] Reapply "Inject services into SpecialUserrights"

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

Pchelolo added a subscriber: Pchelolo.

UserrightPage special in core uses cross-wiki User access via a UserrightsProxy, which is ill-documented and ill-supported concept. With introduction of ActorStore, we are going to refactor UserrightPage.

In order to even approach doing that we need to make make SpecialGlobalGroupMembership not extend the UserrightsPage. As a first step in this, we need to just make SpecialGlobalGroupMembership extend SpecialPage and copy all of the methods inherited from UserrightsPage directly into it. This will be an intermediate step in this refactoring, as we standardize on cross-wiki user access, we will refactor CentralAuth to use it as well.

daniel lowered the priority of this task from High to Medium.Feb 10 2021, 12:15 PM

Change 738574 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/extensions/CentralAuth@master] Stop extending the UserrightsPage class

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