Page MenuHomePhabricator

ActorMigration.php: PHP Warning: Invalid argument supplied for foreach (via SpecialNewFiles)
Closed, ResolvedPublicPRODUCTION ERROR

Description

Error

Request ID: W8ZToApAIC8AADew76wAAAAB

message
PHP Warning: Invalid argument supplied for foreach()
trace
#1 /srv/mediawiki/php-1.32.0-wmf.24/includes/specials/pagers/NewFilesPager.php(72): ActorMigration->getWhere(Wikimedia\Rdbms\DatabaseMysqli, string, boolean)
#2 /srv/mediawiki/php-1.32.0-wmf.24/includes/pager/IndexPager.php(382): NewFilesPager->getQueryInfo()
#3 /srv/mediawiki/php-1.32.0-wmf.24/includes/pager/RangeChronologicalPager.php(104): IndexPager->buildQueryInfo(string, integer, boolean)
#4 /srv/mediawiki/php-1.32.0-wmf.24/includes/pager/IndexPager.php(367): RangeChronologicalPager->buildQueryInfo(string, integer, boolean)
#5 /srv/mediawiki/php-1.32.0-wmf.24/includes/pager/IndexPager.php(226): IndexPager->reallyDoQuery(string, integer, boolean)
#6 /srv/mediawiki/php-1.32.0-wmf.24/includes/pager/IndexPager.php(423): IndexPager->doQuery()
#7 /srv/mediawiki/php-1.32.0-wmf.24/includes/specials/SpecialNewimages.php(108): IndexPager->getBody()
#8 /srv/mediawiki/php-1.32.0-wmf.24/includes/specialpage/SpecialPage.php(569): SpecialNewFiles->execute(NULL)
source
## class SpecalNewFiles { ..
		$user = $opts->getValue( 'user' );
		if ( $user !== '' ) {
			$conds[] = ActorMigration::newMigration()
				->getWhere( wfGetDB( DB_REPLICA ), 'img_user', User::newFromName( $user, false ) )['conds'];
		}

## class ActorMigration { ..
	public function getWhere( IDatabase $db, $key, $users, $useId = true ) {
		// ..
		if ( $users instanceof UserIdentity ) {
			$users = [ $users ];
		}
		foreach ( $users as $user ) {

Impact

Uncertain.

It looks like in this case the preventing it from accessing the correct information, but the error ends up being called out later, given the code in question is only going wrong when the user doesn't exist, ergo it presumably couldn't have valid results either way.

Notes

This can be reproduced on Special:NewFiles on any wiki by entering any username that isn't canonical or valid. E.g. "In.valid!" or "Foo#".

These queries do come in from time to time, and currently produce an error in the logs indicating that the program isn't running the way is was expected to run while written.

Other code paths typically handle this sort of error by rejecting queries for invalid usernames and showing the user an explicit error to that end, or by accepting it silently and producing no results. In any case, it should get there without producing an application error (as opposed to a user error).

Stats:

  • 3,500 hits in past 30 days.
  • Mostly on two specific dates.
  • ~1,200 hits on 2018-09-26, mostly mediawiki.org.
  • ~2,000 hits on 2018-10-16 (today, and the reason for this task), mostly on nov.wikipedia.org and bs.wikipedia.org.

Event Timeline

Change 505430 had a related patch set uploaded (by D3r1ck01; owner: Derick Alangi):
[mediawiki/core@master] ActorMigration: Handle cases for when invalid usernames are supplied

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

A patch is underway. Tagging CPT because the issue seems in your scope, and to track the remaining work (just code review afaik).

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:08 PM
Krinkle renamed this task from ActorMigration.php: PHP Warning: Invalid argument supplied for foreach to ActorMigration.php: PHP Warning: Invalid argument supplied for foreach (via SpecialNewFiles).Dec 4 2019, 1:36 AM

Change 554522 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] ActorMigration: Improve getWhere() handling of $users

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

Anomie added a subscriber: xSavitar.

I'm going to reassign this task to myself and submit a less ambitious fix, since the existing patch seems stalled. The existing patch would still probably be an improvement.

Change 554522 merged by jenkins-bot:
[mediawiki/core@master] ActorMigration: Improve getWhere() handling of $users

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

Again, the other patch should still proceed on its own merits, even though the log message this task is about has been resolved.

Change 505430 abandoned by D3r1ck01:

[mediawiki/core@master] SpecialNewimages: Handle invalid usernames and return an error in these cases

Reason:

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