Page MenuHomePhabricator

Uploading fails with "internal error: key 'ljg40qoexthbj3vu8f9875e5uspj37n.' is not in a proper format"
Closed, ResolvedPublic

Description

Whenever I try to upload a file to my wiki (http://spiele.j-crew.de/wiki/Spezial:Hochladen), the upload fails with the following backtrace:

key 'ljg40qoexthbj3vu8f9875e5uspj37n.' is not in a proper format
Backtrace:
#0 /srv/www/mediawiki/code/includes/upload/UploadBase.php(642): UploadStash->stashFile('/tmp/phpoM0taX', Array, NULL)
#1 /srv/www/mediawiki/code/includes/upload/UploadBase.php(654): UploadBase->stashSessionFile(NULL)
#2 /srv/www/mediawiki/code/includes/specials/SpecialUpload.php(367): UploadBase->stashSession()
#3 /srv/www/mediawiki/code/includes/specials/SpecialUpload.php(461): SpecialUpload->showUploadWarning(Array)
#4 /srv/www/mediawiki/code/includes/specials/SpecialUpload.php(192): SpecialUpload->processUpload()
#5 /srv/www/mediawiki/code/includes/SpecialPage.php(580): SpecialUpload->execute(NULL)
#6 /srv/www/mediawiki/code/includes/Wiki.php(245): SpecialPage::executePath(Object(Title))
#7 /srv/www/mediawiki/code/includes/Wiki.php(63): MediaWiki->handleSpecialCases(Object(Title), Object(OutputPage), Object(WebRequest))
#8 /srv/www/mediawiki/code/w/index.php(103): MediaWiki->performRequestForTitle(Object(Title), NULL, Object(OutputPage), Object(User), Object(WebRequest))
#9 {main}

I tried it with the svg file from http://commons.wikimedia.org/wiki/File:Gtk-dialog-info.svg, feel free to make your own tests on the wiki, if it helps determining the cause.


Version: 1.18.x
Severity: major
URL: http://spiele.j-crew.de/wiki/Spezial:Hochladen

Details

Reference
bz26367

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:24 PM
bzimport set Reference to bz26367.

I tracked the problem down a bit, and it seems to be a problem in includes/MimeMagic.php.

MimeMagic has a list of predefined mime types, with assorted file extensions.
The code that parses the types treats everything up to the first space as the mime type, everything behind that as a file extension. As you can see from the diff below, the svg line does not follow this convention, leading to an extension of "image/svg".
This blows up in UploadStash.

With the change below, I can upload svg files again. But I'm not sure what the correct fix is here, as this particular line is in the code since r21411, from April 2007.

  • a/includes/MimeMagic.php

+++ b/includes/MimeMagic.php
@@ -41,7 +41,7 @@ image/x-bmp bmp
image/gif gif
image/jpeg jpeg jpg jpe
image/png png
-image/svg+xml image/svg svg
+image/svg+xml svg
image/tiff tiff tif
image/vnd.djvu image/x.djvu image/x-djvu djvu
image/x-portable-pixmap ppm

  • Bug 26368 has been marked as a duplicate of this bug. ***

neilk wrote:

Great work Thomas -- however it seems that other lines in the file follow the same convention. Perhaps the intention is to create equivalency between a list of mimetypes and a list of extensions, and one is supposed to just intuit that a mimetype has a slash and an extension doesn't.

This might be a bug in MimeMagic -- if you want to look into it further, that would be awesome, but I'm going to look into it anyway if you don't have the time.

(In reply to comment #1)

I tracked the problem down a bit, and it seems to be a problem in
includes/MimeMagic.php.

MimeMagic has a list of predefined mime types, with assorted file extensions.
The code that parses the types treats everything up to the first space as the
mime type, everything behind that as a file extension. As you can see from the
diff below, the svg line does not follow this convention, leading to an
extension of "image/svg".
This blows up in UploadStash.

With the change below, I can upload svg files again. But I'm not sure what the
correct fix is here, as this particular line is in the code since r21411, from
April 2007.

  • a/includes/MimeMagic.php

+++ b/includes/MimeMagic.php
@@ -41,7 +41,7 @@ image/x-bmp bmp
image/gif gif
image/jpeg jpeg jpg jpe
image/png png
-image/svg+xml image/svg svg
+image/svg+xml svg
image/tiff tiff tif
image/vnd.djvu image/x.djvu image/x-djvu djvu
image/x-portable-pixmap ppm

Bryan.TongMinh wrote:

For some reason your MediaWiki is not able to find mime.types, which in itself is odd, but the fallback is broken. I'm looking into the proper format for it.

Bryan.TongMinh wrote:

This should be fixed by r79835. However I think the real reason is UploadStash requiring the stash key to have an extension. Is there a specific reason for this? It seems to just work even if the extension does not match the mime type at all.

neilk wrote:

There is no reason for the stash key to have an extension, other than paranoia that some browsers might get confused when displaying previews or the full thing. In fact, it used to not have an extension (or rather, it claimed to have one and then discarded it). For simplicity I just made it have an extension.

Bryan.TongMinh wrote:

I guess we should make the extension optional, as it also breaks uploading files without extension.

neilk wrote:

Well, to my knowledge, it was *always* an error to upload files without an extension, at least for Wikimedia wikis -- an error is shown client-side. That's why I felt confident making that a requirement. I was trying to change as few things as possible.

I agree that it is a bit silly, since we are doing a mime type check anyway. If I were designing the system from scratch, I would analyze the file server side and canonicalize to one extension (rather than have, for instance, jpeg and jpg co-existing). But the rule was just -- if there's no jpg, png, gift, etc, we don't even let you begin to upload this.

(In reply to comment #8)

Well, to my knowledge, it was *always* an error to upload files without an
extension, at least for Wikimedia wikis -- an error is shown client-side.
That's why I felt confident making that a requirement. I was trying to change
as few things as possible.

If the *target* (on-wiki) name is extensionless, maybe. But with stashed uploading, the user hasn't even entered the target name yet, so I'm guessing this is based on the local filename instead, which is evil: we should make *zero* assumptions from the local filename, other than maybe prefilling the target name.

neilk wrote:

(In reply to comment #9)

We should make
*zero* assumptions from the local filename, other than maybe prefilling the
target name.

Agreed. On the other hand, if I spent time correcting everything about MediaWiki that didn't make sense to me I'd never get anything done. ;) Still, maybe we can fix THIS, since it seems to be actually harmful.

I'm sorry I haven't gotten around to addressing this yet.

(In reply to comment #5)

This should be fixed by r79835.

On my wiki, r79835 indeed fixes the issue I saw, and I can upload SVG files again.
I also looked at the code, and think that your description is correct.
Thanks for your work (the same goes to NealK)!

(In reply to comment #11)

(In reply to comment #5)

This should be fixed by r79835.

On my wiki, r79835 indeed fixes the issue I saw

Marking as FIXED then.

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