Page MenuHomePhabricator

PHP Fatal error: Call to a member function parseParamString() on a non-object in SpecialUploadStash.php on line 137
Closed, DeclinedPublic

Description

[05-Sep-2013 13:55:08] Fatal error: Call to a member function parseParamString() on a non-object at /usr/local/apache/common-local/php-1.22wmf15/includes/specials/SpecialUploadStash.php on line 137
Server: mw1178
Method: GET
URL: http://commons.wikimedia.org/wiki/Special:UploadStash/thumb/11p7ilv5g0zk.psleb4.1113395.jpg/500px-11p7ilv5g0zk.psleb4.1113395.jpg
Backtrace:
#0 /usr/local/apache/common-local/php-1.22wmf15/includes/specials/SpecialUploadStash.php(137): SpecialUploadStash::parseKey()
#1 /usr/local/apache/common-local/php-1.22wmf15/includes/specials/SpecialUploadStash.php(85): SpecialUploadStash->parseKey('thumb/11p7ilv5g...')
#2 /usr/local/apache/common-local/php-1.22wmf15/includes/specials/SpecialUploadStash.php(69): SpecialUploadStash->showUpload('thumb/11p7ilv5g...')
#3 /usr/local/apache/common-local/php-1.22wmf15/includes/SpecialPage.php(631): SpecialUploadStash->execute('thumb/11p7ilv5g...')
#4 /usr/local/apache/common-local/php-1.22wmf15/includes/SpecialPageFactory.php(490): SpecialPage->run('thumb/11p7ilv5g...')
#5 /usr/local/apache/common-local/php-1.22wmf15/includes/Wiki.php(291): SpecialPageFactory::executePath(Object(Title), Object(RequestContext))
#6 /usr/local/apache/common-local/php-1.22wmf15/includes/Wiki.php(588): MediaWiki->performRequest()
#7 /usr/local/apache/common-local/php-1.22wmf15/includes/Wiki.php(459): MediaWiki->main()
#8 /usr/local/apache/common-local/php-1.22wmf15/index.php(55): MediaWiki->run()
#9 /usr/local/apache/common-local/w/index.php(3): require('/usr/local/apac...')


Version: 1.22.0
Severity: normal

Details

Reference
bz53820

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:54 AM
bzimport set Reference to bz53820.
Reedy created this task.Sep 5 2013, 6:45 PM
bd808 added a comment.Sep 5 2013, 7:17 PM

It looks like $handler could be boolean false via MediaHandler::getHandler() if $wgMediaHandlers[$type] is not found.

$handler is derived via a long chain of object lookups/calls:

RepoGroup::singleton()->getLocalRepo()->getUploadStash()->getFile()->getHandler()

I would expect to see a wfDebug() of "MediaHandler::getHandler: no handler found for X" just before this crash. "X" would be the mime type that MimeMagic::singleton()->guessMimeType() is returning for the local file.

bd808 added a comment.Sep 5 2013, 7:28 PM

File::getHandler() should probably check the return value of MediaHandler::getHandler() and raise an MWException when it === false.

Interesting. The file has extension jpg. Stashed files should get their ext from detected mime type, which means mime type should def be image/jpeg, which is a built in handler and should always be found. (Otoh maybe my memory of how upload stash works is wrong. In any case code should never assume that all files have a handler)

Change 83010 had a related patch set uploaded by BryanDavis:
Assert that $this->handler is not false.

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

bd808 added a comment.Sep 5 2013, 9:02 PM

UploadStash::getFile() returns an UnregisteredLocalFile subclass which in turn derives mime type via MimeMagic::guessMimeType(). Looking there you can see that the first guess is made via magic header checks with a fallback to extension based guessing as Bawolff remembered.

There's a loophole in that fallback though that my money is on for this bug. MimeMagic::doGuessMimeType() will return 'unknown/unknown' if the file can't be opened for reading. Based on some of the other bugs I've been chasing this week, my guess is that the stashed file is actually missing from disk.

Change 83010 merged by jenkins-bot:
Guard against non-object returns from File::getHander()

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

Bryan's patch makes this easier to debug, but it's not fixed. Assigning.

bd808 added a comment.Sep 19 2013, 6:36 PM

The patch fixes the php fatal error, but not whatever underlying problem caused it. The patch will raise an exception with a hopefully informative message if this error path is encountered again.

Tgr added a comment.Jan 8 2014, 7:13 PM

Shouldn't you just not return any params if there is no handler? Not all file types have a handler, this should not be an error condition.

Gilles added a comment.Sep 8 2014, 4:43 PM

No trace of the exception in the last 30 days on logstash nor in the fluorine archives, which go as far back as mid-March.

Gilles raised the priority of this task from High to Unbreak Now!.Dec 4 2014, 10:11 AM
Gilles moved this task from Untriaged to Done on the Multimedia board.
Gilles lowered the priority of this task from Unbreak Now! to High.Dec 4 2014, 11:21 AM