Page MenuHomePhabricator

getOAuthAccessToken() should check title before invoking isSpecial()
Open, LowPublic

Description

The fatal stack trace in Bug 58705 suggests that calling $wgLang->getDir() to determine RTL direction can trigger establishing a user, which can run the UserLoadFromSession hook, in response OAuth's getOAuthAccessToken() calls $title->isSpecial()... which crashes with "Fatal error: Call to a member function isSpecial() on a non-object" because if you do this early enough, RequestContext::getMain() ->getTitle() doesn't return a title object.

Bug 58380 ended up at the same fatal It seems there enough ways this could fail that the getOAuthAccessToken() function should guard against it.


Version: master
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=41201

Details

Reference
bz58731

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:39 AM
bzimport set Reference to bz58731.
bzimport added a subscriber: Unknown Object (MLST).

Note that "check the title" means either "throw an MWException" or "figure out a way to do this same check that doesn't involve RequestContext::getMain()->getTitle() at all". Suggestions welcome.

Both of the fatals so far were caused by other extensions trying to access the User object from $wgExtensionHooks, which now has documentation saying not to do that.

Thanks for opening the bug S. I've been trying to think of a better way to do this for a while.

Brad, should onUserLoadFromSession check if this is an api call, and only fill the user object if so? Then we can skip the check if it's a special page.

It *could*, I suppose. I think it's nice that OAuth can work for the whole site and not just the API though.

Fixing 41201 would be even better.

Aklapper triaged this task as Low priority.Mar 17 2015, 10:55 AM
Aklapper added a subscriber: Aklapper.