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

Assigned To

None

Authored By

	• Spage
	Dec 20 2013, 6:06 AM

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

Related Objects

Mentioned In: T43201: UserLoadFromSession considered evil

Event Timeline

• bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:39 AM

• bzimport added a project: MediaWiki-extensions-OAuth.

• bzimport set Reference to bz58731.

• bzimport added a subscriber: Unknown Object (MLST).

• Spage created this task.Dec 20 2013, 6:06 AM

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 subscribed.

Tgr mentioned this in T43201: UserLoadFromSession considered evil.Aug 27 2015, 6:27 AM

Aklapper removed subscribers: Anomie, • Spage, • wikibugs-l-list.Oct 16 2020, 5:03 PM

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

Description

Details

Related Objects

Event Timeline

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