Page MenuHomePhabricator

ErrorException from line 1274 of /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php: PHP Warning: fread() expects parameter 1 to be resource, boolean given
Closed, ResolvedPublic

Description

Error

Request URL:
Request ID: XN1kUgpAAE4AAGAb8UAAAACF

message
PHP Warning: fread() expects parameter 1 to be resource, boolean given
trace
#0 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(1274): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(517): UploadBase::detectScript(boolean, NULL, string)
#2 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(441): UploadBase->verifyPartialFile()
#3 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(344): UploadBase->verifyFile()
#4 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiUpload.php(547): UploadBase->verifyUpload()
#5 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiUpload.php(78): ApiUpload->verifyUpload()
#6 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(1593): ApiUpload->execute()
#7 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(531): ApiMain->executeAction()
#8 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(502): ApiMain->executeActionWithErrorHandling()
#9 /srv/mediawiki/php-1.34.0-wmf.5/api.php(87): ApiMain->execute()
#10 /srv/mediawiki/w/api.php(3): include(string)
#11 {main}

And related:

PHP Warning: Filename cannot be empty
#0 /srv/mediawiki/php-1.34.0-wmf.5/includes/utils/ZipDirectoryReader.php(149): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.34.0-wmf.5/includes/utils/ZipDirectoryReader.php(91): ZipDirectoryReader->execute()
#2 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(533): ZipDirectoryReader::read(boolean, array)
#3 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(441): UploadBase->verifyPartialFile()
#4 /srv/mediawiki/php-1.34.0-wmf.5/includes/upload/UploadBase.php(344): UploadBase->verifyFile()
#5 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiUpload.php(547): UploadBase->verifyUpload()
#6 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiUpload.php(78): ApiUpload->verifyUpload()
#7 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(1593): ApiUpload->execute()
#8 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(531): ApiMain->executeAction()
#9 /srv/mediawiki/php-1.34.0-wmf.5/includes/api/ApiMain.php(502): ApiMain->executeActionWithErrorHandling()
#10 /srv/mediawiki/php-1.34.0-wmf.5/api.php(87): ApiMain->execute()
#11 /srv/mediawiki/w/api.php(3): include(string)
#12 {main}

Impact

Done via https://commons.wikimedia.org/wiki/Special:UploadWizard

Notes

Event Timeline

hashar created this task.May 16 2019, 1:26 PM
Restricted Application added a project: Multimedia. · View Herald TranscriptMay 16 2019, 1:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

And eventually that causes some other code path to emit PHP Warning: Filename cannot be empty

BBlack added a subscriber: BBlack.May 16 2019, 1:33 PM

Any chance this is interrelated with T222994 ?

Based on log coorelation, I think this,T222994 and T223446 are caused by the same regression.

hashar updated the task description. (Show Details)May 17 2019, 1:11 PM

Change 510893 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/core@master] Don't attempt to perform complete verification on async uploads

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

38ec6d8a344d4eda0307dd3a72653dd2171305d6 T208539 made MediaWiki core to always verify uploads send other the API. when previously it would apparently sometime do it asynchronously? That might have exacerbated a long standing bug of some sort.

Notably, a boolean is passed to the verification function, which might have its origin in:

includes/upload/UploadFromChunks.php
public function concatenateChunks() {
...
        $tmpFile = TempFSFile::factory( 'chunkedupload_', $ext, wfTempDir() );
        $tmpPath = false; // fail in concatenate()
        if ( $tmpFile ) {
            // keep alive with $this
            $tmpPath = $tmpFile->bind( $this )->getPath();
        }

        $this->setTempFile( $tmpPath );
}

And the temp file value is later reused for the verification?

matthiasmullie added a subscriber: matthiasmullie.EditedMay 17 2019, 1:27 PM

As I understand it, when doing async uploads:

Originally:

  • no verification (which includes checking if title is valid, and dieing with an error message)
  • invalid titles could lead to PHP errors, because they weren't verified, but later assumed to exist when checking if there are already duplicates (in UploadBase::checkWarnings)

38ec6d8a344d4eda0307dd3a72653dd2171305d6

  • fixed the latter, by ensuring files (and titles) are verified, even for async uploads, however
  • the files are assembled in a job, and some of the verification checks assume that the file exists (e.g. ZipDirectoryReader will attempt to fopen()), causing errors

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/510893 (proposed patch)

  • will not verify the file, only name (should fix current issue)
  • but will verify the name (should fix original issue)

Change 510923 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/core@master] Revert "Always validate uploads over api"

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

Change 510924 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/core@wmf/1.34.0-wmf.5] Revert "Always validate uploads over api"

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

Change 510923 merged by jenkins-bot:
[mediawiki/core@master] Revert "Always validate uploads over api"

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

Change 510924 merged by jenkins-bot:
[mediawiki/core@wmf/1.34.0-wmf.5] Revert "Always validate uploads over api"

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

Mentioned in SAL (#wikimedia-operations) [2019-05-17T15:18:26Z] <hashar> Deploying hotfix https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/510924/ . Should restore upload of large files on commons and other wikis #T223448 (poke T22994 T223446 )

Mentioned in SAL (#wikimedia-operations) [2019-05-17T15:20:50Z] <hashar@deploy1001> Synchronized php-1.34.0-wmf.5/includes/api/ApiUpload.php: Revert "Always validate uploads over api" - T223448 (T222994 T223446) (duration: 01m 00s)

hashar closed this task as Resolved.May 17 2019, 3:24 PM
hashar assigned this task to matthiasmullie.

We ended up reverting the faulty change: https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/498984/

We have tested on mwdebug1001 before deployment and that fixed the issue.

I'd like to tag on some comments about chunked uploads.

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/510893/ ought to fix the problems I am seeing I should think.

Currently verify is only called for a subset of upload operations, however it is also the only place that checks isIniSizeOverflow. The ini max size problem is always possible - regardless of the exact POST endpoint.

This was preventing image upload on my deployment of latest stable (1.32.2) where there's a completely unhelpful error because php goes crazy later on when it's trying to use the non-existent temp_file name in a single-chunk chunked upload of size larger than php.ini's

Of course I just raised the max size in php.ini, however it took an extremely long time to figure out what was going on.

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:07 PM