Page MenuHomePhabricator

MobileFrontend using ResourceLoaderGetConfigVars hook with User dependent data
Closed, ResolvedPublic

Description

From local dev instance where I'm trying out disabling sessions for load.php requests:

2016-02-23 17:55:24 wiki exception ERROR: [084fb034] /w/load.php?debug=false&lang=en&modules=startup&only=scripts&skin=vector   BadMethodCallException from line 960 of /vagrant/mediawiki/includes/session/SessionManager.php: Sessions are disabled for this entry point {"exception_id":"084fb034"}
[Exception BadMethodCallException] (/vagrant/mediawiki/includes/session/SessionManager.php:960) Sessions are disabled for this entry point
  #0 /vagrant/mediawiki/includes/session/SessionManager.php(187): MediaWiki\Session\SessionManager->getSessionFromInfo(MediaWiki\Session\SessionInfo, WebRequest)
  #1 /vagrant/mediawiki/includes/WebRequest.php(664): MediaWiki\Session\SessionManager->getSessionForRequest(WebRequest)
  #2 /vagrant/mediawiki/includes/user/User.php(1175): WebRequest->getSession()
  #3 /vagrant/mediawiki/includes/user/User.php(384): User->loadFromSession()
  #4 /vagrant/mediawiki/includes/user/User.php(2020): User->load()
  #5 /vagrant/mediawiki/includes/user/User.php(3303): User->getId()
  #6 /vagrant/mediawiki/includes/user/User.php(3311): User->isLoggedIn()
  #7 /vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php(348): User->isAnon()
  #8 /vagrant/mediawiki/extensions/MobileFrontend/includes/MobileContext.php(400): MobileContext->getMobileMode()
  #9 /vagrant/mediawiki/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(411): MobileContext->isBetaGroupMember()
  #10 /vagrant/mediawiki/includes/Hooks.php(195): MobileFrontendHooks::onResourceLoaderGetConfigVars(array)
  #11 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderStartUpModule.php(109): Hooks::run(string, array)
  #12 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderStartUpModule.php(370): ResourceLoaderStartUpModule->getConfigSettings(DerivativeResourceLoaderContext)
  #13 /vagrant/mediawiki/includes/resourceloader/ResourceLoaderModule.php(707): ResourceLoaderStartUpModule->getDefinitionSummary(DerivativeResourceLoaderContext)
  #14 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(622): ResourceLoaderModule->getVersionHash(DerivativeResourceLoaderContext)
  #15 [internal function]: Closure$ResourceLoader::getCombinedVersion(string)
  #16 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(623): array_map(Closure$ResourceLoader::getCombinedVersion;1299315079, array)
  #17 /vagrant/mediawiki/includes/resourceloader/ResourceLoader.php(675): ResourceLoader->getCombinedVersion(ResourceLoaderContext, array)
  #18 /vagrant/mediawiki/load.php(43): ResourceLoader->respond(ResourceLoaderContext)
  #19 /var/www/w/load.php(5): include(string)
 #20 {main}

The docs for the ResourceLoaderGetConfigVars says:

Called at the end of ResourceLoaderStartUpModule::getConfigSettings(). Use this to export static configuration variables to JavaScript. Things that depend on the current page or request state must be added through MakeGlobalVariablesScript instead.

Event Timeline

bd808 renamed this task from MobileFrontend using ResourceLoaderGetConfigVars hook to MobileFrontend using ResourceLoaderGetConfigVars hook with User dependent data.Feb 23 2016, 7:01 PM

To clarify, we are not adding user dependent data.
MobileContext only reads User to determine if the current user is in beta.
A user can be in beta if they have a cookie and are anon or are logged in.

The code path is unstubbing $wgUser which is the problem here. Cookie + anon vs logged in an pref aside this is violating "things that depend on the current page or request state must be added through MakeGlobalVariablesScript instead". Both scenarios are dependent on request state.

Legoktm triaged this task as High priority.Feb 23 2016, 7:58 PM
Legoktm added a project: Technical-Debt.

Change 272810 had a related patch set uploaded (by Jdlrobson):
Don't call isBetaGroupMember in ResourceLoaderGetConfigVars hook

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

Florian assigned this task to Jdlrobson.
Florian removed a project: Patch-For-Review.

Change 272810 merged by jenkins-bot:
Don't call isBetaGroupMember in ResourceLoaderGetConfigVars hook

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