Page MenuHomePhabricator

User->doLogout() fatal error in 1.27 with AuthDrupal extension
Open, Needs TriagePublic

Description

Catchable fatal error: Method User::__toString() must return a string value in /.../wiki/includes/user/User.php on line 3857
1	0.0001	234152	{main}( )	../index.php:0
2	0.0887	4773560	MediaWiki->run( )	../index.php:43
3	0.0887	4774472	MediaWiki->main( )	../MediaWiki.php:519
4	0.1273	5237016	User->isLoggedIn( )	../MediaWiki.php:690
5	0.1273	5237016	User->getId( )	../User.php:3473
6	0.1274	5237016	User->load( )	../User.php:2108
7	0.1274	5237320	User->loadFromSession( )	../User.php:408
8	0.1274	5237952	Hooks::run( )	../User.php:1235
9	0.1278	5240560	call_user_func_array:{/.../wiki/includes/Hooks.php:195} ( )	../Hooks.php:195
10	0.1278	5241120	AuthDrupal::_cleanup( )	../Hooks.php:195
11	0.1279	5241488	User->doLogout( )	../AuthDrupal.php:715

This (old) AuthDrupal auth-plugin call User->doLogout() (according to the state of a cookie).
It seems that under some circumstances LoggerFactory::getInstance( 'session' )->warning() fails because it use objects as string.
In my case, $session->getUser() is OK (my username string), but $this->__toString() returns FALSE.

Using REL1_27 @ b9246dcb37483e121eb91502e462f3c72cfb0ab7

Event Timeline

Hi @Drzraf, thanks for taking the time to report this!

This (old) AuthDrupal auth-plugin

That means https://www.mediawiki.org/wiki/Extension:AuthDrupal ?

Wondering if this is another case of "Update AuthDrupal to use AuthManager" and should block T110291 ?

Aklapper renamed this task from User->doLogout() fatal error to User->doLogout() fatal error in 1.27 with AuthDrupal extension.May 31 2016, 10:00 AM

I didn't know about "other cases".
I do use my personal fork of this module
(https://gitlab.com/drzraf/drupal-mediawiki/tree/custom) that I intend updating later this year when AuthManager is stable and that I can grab some time to understand it.
(auth-plugin is an old story (http://osdir.com/ml/general/2012-10/msg17813.html))

For now I'd say there is no reason the extension wouldn't work with $wgDisableAuthManager = true; , and I don't see a reason for doLogout() to error-out as it does currently.
(I guess that it's about internal API being more robust to attempts to logout a user twice)

Any up on this?
Maybe more precisely:
Do internal API author assumed it should always be safe to call doLogout()?
Or, on the contrary, did he purposefully omitted error handling, thus forcing API consumers (plugins, ....) to makes necessary checks themselves?