Page MenuHomePhabricator

ErrorException SpecialUserlogin.php: Invalid operand type was used
Closed, ResolvedPublic

Description

2015-08-15 19:18:15 translatewiki.net translatewiki_net-bw_: [c1d4f49b]
                    /w/i.php?title=Special:UserLogin&action=submitlogin&type=login&returnto=Special:UserLogin&returntoquery=type%3Dsignup
                    ErrorException from line 1070 of /srv/mediawiki/tags/2015-08-15_15:50:02/includes/specials/SpecialUserlogin.php: PHP
                    Warning: Invalid operand type was used: Invalid type used as key
2015-08-15 19:18:15 translatewiki.net translatewiki_net-bw_: [2cefad9b]
                    /w/i.php?title=Special:UserLogin&action=submitlogin&type=login&returnto=Special:UserLogin&returntoquery=type%3Dsignup
                    ErrorException from line 1070 of /srv/mediawiki/tags/2015-08-15_15:50:02/includes/specials/SpecialUserlogin.php: PHP
                    Notice: Undefined index:

Details

Related Gerrit Patches:

Event Timeline

Nemo_bis created this task.Aug 15 2015, 7:29 PM
Nemo_bis raised the priority of this task from to Medium.
Nemo_bis updated the task description. (Show Details)
Nemo_bis added a subscriber: Nemo_bis.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 15 2015, 7:29 PM
bd808 added a comment.Aug 16 2015, 8:34 PM

Notice: Undefined index: looks like $status is null or an empty string. The Invalid operand type was used: Invalid type used as key error points to it being an array or object based on an HHVM source search.

I'm not sure however how any of these states would make it to line 1070 as the switch ( $status ) { ... } ends with default: throw new MWException( 'Unhandled case value' ); for any value that is not already listed in LoginForm::$statusCodes.

@Nemo_bis is this easily repeatable? If so could you live hack to try and get a var_dump of $status?

Tgr added a comment.Aug 16 2015, 9:28 PM

Type juggling, probably.

$v = null;
switch( $v ) {
    case 0:
        echo '0';
        break;
    default:
        echo 'default';
        break;
}
// '0'

Change 231974 had a related patch set uploaded (by Gergő Tisza):
Fix array access error in LoginForm

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

Tgr added a comment.Aug 16 2015, 9:45 PM

Duct tape fix; would be nice to figure out what returns an invalid return status. LoginForm::authenticateUserData() itself seems sound, so it's probably something using the AbortLogin hook.

Anomie added a subscriber: Anomie.Aug 17 2015, 2:36 PM

so it's probably something using the AbortLogin hook.

A better fix then would be to check the value output by that hook and throw an exception if it's a wrong type, instead of just blindly casting it to an integer.

Although checking extensions in Gerrit, I don't see any that are likely to be misbehaving. More likely is SpecialUserlogin.php line 980 overwriting the $status variable.

Change 232046 had a related patch set uploaded (by Anomie):
Rename variable to avoid collisions

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

Change 232046 merged by jenkins-bot:
Rename variable to avoid collisions

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

The above log entry was truncated, here is the full one:

[Sat Aug 15 21:18:15 2015] [hphp] [28681:7f5ea07ff700:3402:000002] [] \nNotice: Undefined index: <OK, collected 1 error(s) on the way, no value set>\n+------+---------------------------+-------------
-----------------------------+\n|    1 | passwordtooshort          | 8                                        |\n+------+---------------------------+------------------------------------------+\n in /
srv/mediawiki/tags/2015-08-15_15:50:02/includes/specials/SpecialUserlogin.php on line 1070
bd808 added a comment.Aug 17 2015, 8:02 PM

The above log entry was truncated, here is the full one:

[Sat Aug 15 21:18:15 2015] [hphp] [28681:7f5ea07ff700:3402:000002] [] \nNotice: Undefined index: <OK, collected 1 error(s) on the way, no value set>\n+------+---------------------------+-------------
-----------------------------+\n|    1 | passwordtooshort          | 8                                        |\n+------+---------------------------+------------------------------------------+\n in /
srv/mediawiki/tags/2015-08-15_15:50:02/includes/specials/SpecialUserlogin.php on line 1070

That looks like the problem that @Anomie fixed in https://gerrit.wikimedia.org/r/232046 where $status was being reused inside the self:SUCCESS case of LoginForm::processLogin(). That would also explain HHVM seeing the key as an object.

Change 231974 abandoned by Gergő Tisza:
Fix array access error in LoginForm

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

Change 232220 had a related patch set uploaded (by Gergő Tisza):
Validate status codes returned from the AbortLogin hook

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

Umherirrender closed this task as Resolved.Sep 29 2015, 3:08 PM
Umherirrender set Security to None.

Change 232220 merged by jenkins-bot:
Validate status codes returned from the AbortLogin hook

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