Page MenuHomePhabricator

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

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

Krinkle created this task.Oct 16 2018, 9:22 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 16 2018, 9:22 PM
Krinkle updated the task description. (Show Details)Oct 16 2018, 9:28 PM
Krinkle moved this task from Untriaged to Found longer ago on the Wikimedia-production-error board.

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

D3r1ck01 moved this task from Backlog to Doing [WIP] on the User-D3r1ck01 board.
D3r1ck01 moved this task from Doing [WIP] to Under Review on the User-D3r1ck01 board.

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).Wed, Dec 4, 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 claimed this task.Wed, Dec 4, 2:54 PM
Anomie added a subscriber: D3r1ck01.

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

Anomie closed this task as Resolved.Wed, Dec 4, 9:41 PM

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