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 created this task.Feb 23 2016, 7:01 PM
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptFeb 23 2016, 7:01 PM
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.

bd808 added a comment.Feb 23 2016, 7:41 PM

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 closed this task as Resolved.Feb 23 2016, 8:37 PM
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