Page MenuHomePhabricator
Create Task
Maniphest T254078

STL upload file causes "Extension is null." via UploadStash
Closed, ResolvedPublic
Actions

Description

  • Steps to Reproduce
    1. Go to the https://commons.wikimedia.org/wiki/Main_Page with Google Chrome version 83.0.4103.61;
    2. Make sure you are logged in;
    3. Select "Upload file" in "Participate" menu;
    4. Click "Next";
    5. Click "Select media files to share";
    6. Select the STL file which I want to share.
  • Actual Results
Could not store upload in the stash (UploadStashFileException): "Extension is null.".

石造物3Dアーカイブ_Wikimedia Commonsへのアップロード方法_Actual Results.jpg (918×1 px, 638 KB)

  • Expected Results
Uploaded

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Stop throwing an exception in UploadStash::getExtensionForPathmediawiki/coremaster+1 -39
Revert "upload: Fix incorrect handling of missing file extension in UploadStash"mediawiki/coremaster+17 -3
Customize query in gerrit

Related Objects

Event Timeline

T009u640q841 created this task.May 30 2020, 7:25 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 30 2020, 7:25 PM
Reedy added a project: Wikimedia-production-error.May 30 2020, 7:35 PM
Reedy subscribed.

When did this happen? I can't find any logs for it

Reedy added a comment.May 30 2020, 7:41 PM

How big is the file? Can you upload it here?

T009u640q841 added a comment.May 30 2020, 7:46 PM

Thank you. For two days.

T009u640q841 added a comment.May 30 2020, 7:49 PM

The file size who I want to share is 73243KB.

Reedy added a comment.May 30 2020, 8:01 PM

I note you have uploaded them successfully before. I also note the filename isn't very descriptive ;)

There hasn't been any STL uploads since the 19th - https://commons.wikimedia.org/wiki/Special:NewFiles?user=&mediatype%5B%5D=3D&start=&end=&wpFormIdentifier=specialnewimages&limit=50&offset=

Reedy added a comment.May 30 2020, 8:07 PM

Ok, I can replicate it locally with https://commons.wikimedia.org/wiki/File:Holterh%C3%B6fchen.stl on UploadWizard. Special:Upload is fine

Reedy added a comment.May 30 2020, 8:08 PM

Suspecting T252228: Make MimeAnalyser fast might be related

T009u640q841 added a comment.May 30 2020, 8:10 PM

Thank you!

Reedy mentioned this in T252228: Make MimeAnalyser fast.May 30 2020, 8:10 PM
Reedy renamed this task from The STL file can’t be shared in "Upload file". to STL upload file causes "Extension is null." via UploadWizard.May 30 2020, 8:57 PM
Reedy added a subscriber: ori.May 30 2020, 9:59 PM

Ok, seems to have been caused by 0a82600a27892c0029871a16dd01831fdd71b6a9 by @ori

Reedy added a comment.May 30 2020, 10:00 PM

Reverting that patch gives an expected error of a duplicate file

Screenshot 2020-05-30 at 22.59.56.png (396×1 px, 50 KB)

gerritbot added a comment.May 30 2020, 10:01 PM

Change 600420 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Revert "upload: Fix incorrect handling of missing file extension in UploadStash"

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

gerritbot added a project: Patch-For-Review.May 30 2020, 10:01 PM
Reedy added a comment.May 30 2020, 10:15 PM

It's not replicable with the old or new code via eval.php and a file with or without an extension..

$ php maintenance/eval.php 
> $path = '/tmp/Foobar';

> $magic = MediaWiki\MediaWikiServices::getInstance()->getMimeAnalyzer();

> $mimeType = $magic->guessMimeType( $path );

> $extensions = explode( ' ', $magic->getExtensionsForType( $mimeType ) );

> if ( count( $extensions ) ) { $extension = $extensions[0]; }

> var_dump( $extensions, $extension );
/var/www/wiki/mediawiki/core/maintenance/eval.php(78) : eval()'d code:1:
array(1) {
  [0] =>
  string(3) "stl"
}
/var/www/wiki/mediawiki/core/maintenance/eval.php(78) : eval()'d code:1:
string(3) "stl"

> $extensions = $magic->getExtensionsForType( $mimeType );

> if ( $extensions !== null ) { $extension = strtok( $extensions, ' ' ); }

> var_dump( $extensions, $extension );
/var/www/wiki/mediawiki/core/maintenance/eval.php(78) : eval()'d code:1:
string(3) "stl"
/var/www/wiki/mediawiki/core/maintenance/eval.php(78) : eval()'d code:1:
string(3) "stl"

So it's something odd between the two versions when dealing with upload stash files

Reedy added a comment.May 30 2020, 11:33 PM
diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php
index 833b1ce1bd..2993311bf4 100644
--- a/includes/upload/UploadStash.php
+++ b/includes/upload/UploadStash.php
@@ -485,7 +485,17 @@ class UploadStash {
                        // If not, assume that it should be related to the MIME type of the original file.
                        $magic = MediaWiki\MediaWikiServices::getInstance()->getMimeAnalyzer();
                        $mimeType = $magic->guessMimeType( $path );
+
                        $extension = $magic->getExtensionFromMimeTypeOrNull( $mimeType );
+wfDebugLog( 'uploadstash', "mime: " . var_export( $mimeType, true ) );
+wfDebugLog( 'uploadstash', "getExtensionFromMimeTypeOrNull: " . var_export( $extension, true ) );
+
+                       $extensions2 = explode( ' ', $magic->getExtensionsForType( $mimeType ) );
+                       if ( count( $extensions2 ) ) {
+                               $extension2 = $extensions2[0];
+                       }
+wfDebugLog( 'uploadstash', "getExtensionsForType: " . var_export( $extension2, true ) );
+wfDebugLog( 'uploadstash', "getExtensionsForType[0]: " . var_export( $extensions2, true ) );
                }
 
                if ( $extension === null ) {

Output:

[uploadstash] mime: 'unknown/unknown'
[uploadstash] getExtensionFromMimeTypeOrNull: NULL
[uploadstash] getExtensionsForType: ''
[uploadstash] getExtensionsForType[0]: array (
  0 => '',
)

I agree formatting of the old methods output don't make much sense.

I guess the old version didn't get null, so it didn't throw an exception (but it does now)... And then it did some fallback somewhere else to get the type eventually, so MW is happy? So that's the problem caused by this patch, even if the old version didn't make much sense

But there's obviously something odd about the stashed version of the file vs a downloaded version on disk so it's not getting the type

> var_dump( UploadStash::getExtensionForPath( '/tmp/Foobar' ) );
/var/www/wiki/mediawiki/core/maintenance/eval.php(78) : eval()'d code:1:
string(3) "stl"
Reedy renamed this task from STL upload file causes "Extension is null." via UploadWizard to STL upload file causes "Extension is null." via UploadStash.May 30 2020, 11:38 PM
Reedy added a project: MediaWiki-Uploading.
Reedy added a comment.EditedMay 30 2020, 11:44 PM

Which seems to be the case based on the only caller of UploadStash::getExtensionForPath()...

		// we will be initializing from some tmpnam files that don't have extensions.
		// most of MediaWiki assumes all uploaded files have good extensions. So, we fix this.
		$extension = self::getExtensionForPath( $path );
		if ( !preg_match( "/\\.\\Q$extension\\E$/", $path ) ) {
			$pathWithGoodExtension = "$path.$extension";
		} else {
			$pathWithGoodExtension = $path;
		}

If we look at the code just before that...

		$mwProps = new MWFileProps( MediaWiki\MediaWikiServices::getInstance()->getMimeAnalyzer() );
		$fileProps = $mwProps->getPropsFromPath( $path, true );
		wfDebug( __METHOD__ . " stashing file at '$path'\n" );

Passing in $ext = true to getPropsFromPath then does some calculations to work out what's going on

			# MIME type according to file contents
			$info['file-mime'] = $this->magic->guessMimeType( $path, false );
			# Logical MIME type
			$ext = ( $ext === true ) ? FileBackend::extensionFromPath( $path ) : $ext;

			# XXX: MimeAnalyzer::improveTypeFromExtension() may return null (T253483).
			# Unclear if callers of this method expect that.
			$info['mime'] = $this->magic->improveTypeFromExtension( $info['file-mime'], $ext );

			list( $info['major_mime'], $info['minor_mime'] ) = File::splitMime( $info['mime'] );
			$info['media_type'] = $this->magic->getMediaType( $path, $info['mime'] );

So yeah... It feels like to me that causing that exception there is definitely the problem in getExtensionForPath. That is, unless we can fix whatever uploadstash is doing with the stl files to cause MW to be unable to get the extension from the temporary file at that point of the upload process

Reedy added a comment.May 31 2020, 12:12 AM

Aha, replicated by fudging another test - https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/600502/

01:11:13 There was 1 error:
01:11:13 
01:11:13 1) ApiUploadTest::testUploadChunks
01:11:13 ApiUsageException: Could not store upload in the stash (UploadStashFileException): "Extension is null.".
01:11:13 
01:11:13 /workspace/src/includes/api/ApiBase.php:1493
01:11:13 /workspace/src/includes/api/ApiUpload.php:375
01:11:13 /workspace/src/includes/api/ApiUpload.php:240
01:11:13 /workspace/src/includes/api/ApiUpload.php:133
01:11:13 /workspace/src/includes/api/ApiUpload.php:104
01:11:13 /workspace/src/includes/api/ApiMain.php:1584
01:11:13 /workspace/src/includes/api/ApiMain.php:493
01:11:13 /workspace/src/tests/phpunit/includes/api/ApiTestCase.php:109
01:11:13 /workspace/src/tests/phpunit/includes/api/ApiTestCase.php:138
01:11:13 /workspace/src/tests/phpunit/includes/api/ApiUploadTest.php:231
01:11:13 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:437
01:11:13 /workspace/src/maintenance/doMaintenance.php:105
01:11:13 === Logs generated by test case
01:11:13 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [MessageCache] [debug] MessageCache using store {class} {"class":"HashBagOStuff"}
01:11:13 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [localisation] [debug] LocalisationCache using store LCStoreNull []
01:11:13 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
01:11:13 [wfDebug] [debug] WebRequestUpload::getName: blob normalized to 'blob' {"private":false}
01:11:13 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
01:11:13 [wfDebug] [debug] WebRequestUpload::getName: blob normalized to 'blob' {"private":false}
01:11:13 [wfDebug] [debug] UploadFromChunks::__construct creating new UploadFromChunks instance for 2 {"private":false}
01:11:13 [wfDebug] [debug] User::getBlockedStatus: checking... {"private":false}
01:11:13 [wfDebug] [debug] IP: 127.0.0.1 {"private":false}
01:11:13 [localisation] [debug] LocalisationCache::isExpired(en): cache missing, need to make one []
01:11:13 [objectcache] [debug] fetchOrRegenerate(wikidb-unittest_:page:6:ba968d21c64d9ce0a96ea2c55c1da766d8a4ba5f): miss, new value computed []
01:11:13 [MessageCache] [debug] MessageCache using store {class} {"class":"HashBagOStuff"}
01:11:13 [Mime] [info] MimeAnalyzer::doGuessMimeType: analyzing head and tail of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u for magic numbers.
01:11:13  []
01:11:13 [wfDebug] [debug] DjVuImage::getInfo: not a DjVu file {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: internal type detection failed for /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u (.)...
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::detectMimeType: magic mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: application/octet-stream
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: guessed mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: unknown/unknown
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::improveTypeFromExtension: refusing to guess mime type for .stl file, we should have recognized it
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::improveTypeFromExtension: improved mime type for .stl: unknown/unknown
01:11:13  []
01:11:13 [wfDebug] [debug] MediaHandlerFactory::getHandler: no handler found for unknown/unknown. {"private":false}
01:11:13 [wfDebug] [debug] mime: <unknown/unknown> extension: <stl> {"private":false}
01:11:13 [wfDebug] [debug] UploadBase::detectScript: checking for embedded scripts and HTML stuff {"private":false}
01:11:13 [wfDebug] [debug] UploadBase::detectScript: no scripts found {"private":false}
01:11:13 [wfDebug] [debug] ZipDirectoryReader: Fatal error: zip file lacks EOCDR signature. It probably isn't a zip file. {"private":false}
01:11:13 [wfDebug] [debug] UploadBase::detectVirus: virus scanner disabled {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::doGuessMimeType: analyzing head and tail of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u for magic numbers.
01:11:13  []
01:11:13 [wfDebug] [debug] DjVuImage::getInfo: not a DjVu file {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: internal type detection failed for /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u (.)...
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::detectMimeType: magic mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: application/octet-stream
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: guessed mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: unknown/unknown
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::improveTypeFromExtension: improved mime type for .: 
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::doGuessMimeType: analyzing head and tail of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u for magic numbers.
01:11:13  []
01:11:13 [wfDebug] [debug] DjVuImage::getInfo: not a DjVu file {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: internal type detection failed for /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u (.)...
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::detectMimeType: magic mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: application/octet-stream
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: guessed mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: unknown/unknown
01:11:13  []
01:11:13 [wfDebug] [debug] MediaHandlerFactory::getHandler: no handler found for . {"private":false}
01:11:13 [wfDebug] [debug] UploadStash::stashFile stashing file at '/tmp/MW_PHPUnit_ApiUploadTest_5QZE2u' {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: WARNING: use of the $ext parameter is deprecated. Use improveTypeFromExtension($mime, $ext) instead.
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::doGuessMimeType: analyzing head and tail of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u for magic numbers.
01:11:13  []
01:11:13 [wfDebug] [debug] DjVuImage::getInfo: not a DjVu file {"private":false}
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: internal type detection failed for /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u (.1)...
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::detectMimeType: WARNING: use of the $ext parameter is deprecated. Use improveTypeFromExtension($mime, $ext) instead.
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::detectMimeType: magic mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: application/octet-stream
01:11:13  []
01:11:13 [Mime] [info] MimeAnalyzer::guessMimeType: guessed mime type of /tmp/MW_PHPUnit_ApiUploadTest_5QZE2u: unknown/unknown
01:11:13  []
01:11:13 [UserOptionsManager] [debug] Loading options from database {"user_id":2}
01:11:13 [wfDebug] [debug] ApiUpload::performStash Stashing temporary file failed: UploadStashFileException Extension is null. {"private":false}
01:11:13 [wfDebug] [debug] wfRecursiveRemoveDir( /tmp/MW_PHPUnit_ApiUploadTest_E6fMP9 ) {"private":false}
gerritbot added a comment.May 31 2020, 12:38 AM

Change 600420 abandoned by Reedy:
Revert "upload: Fix incorrect handling of missing file extension in UploadStash"

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

gerritbot added a comment.May 31 2020, 1:01 AM

Change 600533 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Stop throwing an exception in UploadStash::getExtensionForPath

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

Reedy mentioned this in T254083: UploadStash::getExtensionForPath() is unable to detect type of upload-stashed stl file.May 31 2020, 1:20 AM
gerritbot added a comment.May 31 2020, 2:58 AM

Change 600533 merged by jenkins-bot:
[mediawiki/core@master] Stop throwing an exception in UploadStash::getExtensionForPath

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.35; 2020-06-02).May 31 2020, 3:00 AM
Maintenance_bot removed a project: Patch-For-Review.May 31 2020, 3:10 AM
Jdforrester-WMF closed this task as Resolved.Jun 8 2020, 9:24 PM
Jdforrester-WMF assigned this task to Reedy.
Jdforrester-WMF subscribed.

https://commons.wikimedia.org/wiki/File:Steinbruch_Weuste_2020.stl was uploaded following the deployment of wmf.35 last week with the above patch.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits