Directory

⚓ T378781 Error when moving a StructuredDiscussion page
Page MenuHomePhabricator

Error when moving a StructuredDiscussion page
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue:

What happens?:
*Error message:

"Structured Discussions board" content is not allowed on page User talk:Quiddity (WMF)/Flow in slot "main"

flow-page-move-error.png (1×1 px, 140 KB)

What should have happened instead?:

  • A page-move

Notes:

Event Timeline

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

I took a look at what's happening here. It turns out to be a pretty interesting bug!

The error message shows because BoardContentHandler::canBeUsedOn( Title::newFromText( 'User talk:Quiddity (WMF)/Flow' ) ) returns false. That method boils down to checking either one of the following conditions is true:

  1. The namespace has Flow enabled on all pages by default
  2. $user has the flow-create-board right, where $user is determined by Flow logic (more below).

Condition 1 is false in this case, as NS_USER_TALK does not use Flow by default at MediaWiki.org.

Condition 2 is the problematic part. $user is constructed by instantiating the FlowUser service, which returns anonymous users if RUN_MAINTENANCE_IF_MAIN is defined and the currently logged in user (RequestContext::getMain()->getUser()) otherwise. The check for the RUN_MAINTENANCE_IF_MAIN constant is supposed to determine whether we're in a maintenance script context. Despite Special:Movepage is definitely not a maintenance script, it turns out that in production, the RUN_MAINTENANCE_IF_MAIN constant is defined even in web requests (GETs and POSTs).

The only place that defines RUN_MAINTENANCE_IF_MAIN in all of MediaWiki is the maintenance/Maintenance.php file from MediaWiki Core (which is not supposed to be included during web requests). To figure out what is requiring the maintenance/Maintenance.php file, I added a throw Exception command to that file at mwdebug1002 to get a trace, and here is what I got:

2024-11-01 00:38:13.074934 [116321e8-c8ac-9376-8646-acd23dae9d4c] mwdebug1002 mediawikiwiki 1.44.0-wmf.1 exception ERROR: [116321e8-c8ac-9376-8646-acd23dae9d4c] /wiki/Special:BlankPage   Exception:  {"exception_url":"/wiki/Special:BlankPage","reqId":"116321e8-c8ac-9376-8646-acd23dae9d4c","caught_by":"mwe_handler"} 
[Exception Exception] (/srv/mediawiki/php-1.44.0-wmf.1/maintenance/Maintenance.php:37) 
  #0 /srv/mediawiki/php-1.44.0-wmf.1/extensions/CentralAuth/maintenance/backfillLocalAccounts.php(30): require_once()
  #1 /srv/mediawiki/php-1.44.0-wmf.1/includes/AutoLoader.php(226): require_once(string)
  #2 [internal function]: AutoLoader::autoload(string)
  #3 /srv/mediawiki/php-1.44.0-wmf.1/extensions/CentralAuth/includes/CentralAuthHooks.php(627): spl_autoload_call(string)
  #4 /srv/mediawiki/php-1.44.0-wmf.1/includes/HookContainer/HookContainer.php(159): MediaWiki\Extension\CentralAuth\CentralAuthHooks->onUserGetReservedNames(array)
  #5 /srv/mediawiki/php-1.44.0-wmf.1/includes/HookContainer/HookRunner.php(4341): MediaWiki\HookContainer\HookContainer->run(string, array)
  #6 /srv/mediawiki/php-1.44.0-wmf.1/includes/user/UserNameUtils.php(179): MediaWiki\HookContainer\HookRunner->onUserGetReservedNames(array)
  #7 /srv/mediawiki/php-1.44.0-wmf.1/extensions/CentralAuth/includes/session/CentralAuthSessionProvider.php(235): MediaWiki\User\UserNameUtils->isUsable(string)
  #8 /srv/mediawiki/php-1.44.0-wmf.1/includes/session/SessionManager.php(542): CentralAuthSessionProvider->provideSessionInfo(MediaWiki\Request\WebRequest)
  #9 /srv/mediawiki/php-1.44.0-wmf.1/includes/session/SessionManager.php(248): MediaWiki\Session\SessionManager->getSessionInfoForRequest(MediaWiki\Request\WebRequest)
  #10 /srv/mediawiki/php-1.44.0-wmf.1/includes/Request/WebRequest.php(867): MediaWiki\Session\SessionManager->getSessionForRequest(MediaWiki\Request\WebRequest)
  #11 /srv/mediawiki/php-1.44.0-wmf.1/includes/session/SessionManager.php(167): MediaWiki\Request\WebRequest->getSession()
  #12 /srv/mediawiki/php-1.44.0-wmf.1/includes/Setup.php(492): MediaWiki\Session\SessionManager::getGlobalSession()
  #13 /srv/mediawiki/php-1.44.0-wmf.1/includes/WebStart.php(85): require_once(string)
  #14 /srv/mediawiki/php-1.44.0-wmf.1/index.php(50): require(string)
  #15 /srv/mediawiki/w/index.php(3): require(string)
  #16 {main}

It turns out that CentralAuthHooks references the BackfillLocalAccounts::ACCOUNT_CREATOR constant, which is coming from the BackfillLocalAccounts maintenance script. That maintenance script manually requires maintenance/Maintenance.php, to ensure it can be executed as a standalone PHP script (via php /path/to/BackfillLocalAccounts.php). The problem is that Autoloader executes this require as well, this defining the RUN_MAINTENANCE_IF_MAIN constant, thus causing this error. This is apparently happening since rECAU38ff1448ff4b: Script to backfill local account creation for global users, which is a part of wmf.1.

Funnily, the maintenance/Maintenance.php file predicts this kind of issue by including the following guardrail:

if ( defined( 'MEDIAWIKI' ) ) {
	// This file is included by many autoloaded class files, and so may
	// potentially be invoked in the context of a web request or another CLI
	// script. It's not appropriate to run the following file-scope code in
	// such a case.
	return;
}

Unfortunately, this guardrail is only executed after defining the RUN_MAINTENANCE_IF_MAIN constant, so it doesn't prevent this issue from happening. I just hope that this (or some other) guardrail is actually preventing us from accidentally executing the script in production and on every pageview.


The good question is how to fix this. I see several options (all completely untested):

  1. In CentralAuth, Move the BackfillLocalAccounts::ACCOUNT_CREATOR constant out of a maintenance script: if the constant was elsewhere (referenced from the maintenance script rather than defined there), then the script wouldn't be loaded on each request, thus resolving this bug.
  2. In Core, make the guardrail in Maintenance.php to be the very first thing: this might throw an error when autoloader inevitably executes the require_once RUN_MAINTENANCE_IF_MAIN; line
  3. In Flow, whether we're in maintenance script context in a different way: maybe MW_ENTRY_POINT === 'cli' would be a more precise mechanism?
  4. In Flow, do not check whether we're running a maintenance script at all, and simply always use RequestContext::getMain()->getUser() (in maint scripts, that is going to be the anonymous user anyway, unless the script overrides the context, at which point it probably does that for a reason)
  5. Something else

I'm not sure any of those options is a clear winner. @ArielGlenn @Tgr, as the author/reviewer of rECAU38ff1448ff4b: Script to backfill local account creation for global users (which caused this), do you have any thoughts on what fix to take?

Tagging CentralAuth as well, given the error ultimately comes from there.

Thanks for investigating that. Yeah we should just move the constant.

Using RUN_MAINTENANCE_IF_MAIN is weird (Flow is not the only one doing it but it's still pretty rare) and probably broken (e.g. what if the script is runJobs?) and I think checking the entry point would be more normal, but not sure of the implictions of changing that while moving the constant is guaranteed to be a no-op.

Great spot, @Quiddity and wonderful work identifying the cause, @Urbanecm_WMF!

Resulting question: is this issue specific to mediawiki.org? Is it wiki-agnostic?

...I ask the above in an effort to understand whether this issue ought to be related to a particular migration phase (e.g. T378829, T378827, and/or T376749).

Change #1085921 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/CentralAuth@master] BackfillLocalAccounts: Move constant to CentralAuthHooks

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

Change #1085921 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] BackfillLocalAccounts: Move constant to CentralAuthHooks

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

Thanks! This seems resolved, albeit just from a single test. -- I successfully moved mw:User talk:Quiddity (WMF)/sandbox11 to mw:User talk:Quiddity (WMF)/sandbox11-1.

Etonkovidova claimed this task.
Etonkovidova subscribed.

Thanks! This seems resolved, albeit just from a single test. -- I successfully moved mw:User talk:Quiddity (WMF)/sandbox11 to mw:User talk:Quiddity (WMF)/sandbox11-1.

Thanks, @Quiddity for confirming that the fix is in place! I did some testing in beta and moving flow-board pages seems to be working as expected.

Resulting question: is this issue specific to mediawiki.org? Is it wiki-agnostic?

...I ask the above in an effort to understand whether this issue ought to be related to a particular migration phase (e.g. T378829, T378827, and/or T376749).

Belated answer: It was impacting all Flow wikis, except wikis when Flow is the default discussion mechanism (not sure if those still exists). WIth this task resolved, the bug shouldn't be present anywhere (there might be a different bug, hard to predict unfortunately :)). Hope this helps!