Page MenuHomePhabricator

Return meaningful error message, if file extension and MIME type are not matching.
Closed, ResolvedPublic5 Story Points

Description

Bug description
Clicking "Import" on Commons when trying to import the following file gives "Fatal exception of type RuntimeException". File extension in page title is png and non-matching mime type is gif.

I was able to import the following file after changing the file extension in page title beforehand.

Acceptance Criteria

  •  If a file's extension and the actual MIME type are not matching, redirect the moving person straight to the blocking error page
  • The error displayed is mediawiki core's filetype-mime-mismatch

Notes
Other ideas to solve the issues:

  • file extension could be changed automatically
  • FileImporter could allow changing it manually during the import, in case it isn't correct and the actual file type is permitted.

There is a somewhat related task (T185735) on error handling for file extensions that are correct, but not permitted for uploads.


Previous specialized wording of the error message:

The extension of the file you are importing (.png) does not match its MIME type (gif). To fix this, rename the original file so that it ends in .gif. This is usually done by admins. After that, you can try another import."

The error message contains the appropriate extension and MIME type explanations (in the example given above ".png" and "gif")

Event Timeline

Pikne created this task.Mar 12 2019, 10:13 AM
Restricted Application added a project: TCB-Team. · View Herald TranscriptMar 12 2019, 10:13 AM
Pikne renamed this task from File extension in page title not matching the actual file type results in "Fatal exception of type RuntimeException" to File extension in page title not matching the actual permitted file type results in an error.Mar 22 2019, 8:02 AM
Pikne updated the task description. (Show Details)
thiemowmde triaged this task as Normal priority.May 14 2019, 1:51 PM
thiemowmde added a subscriber: thiemowmde.

To me this sounds like some code in MediaWiki core does produce an error in a situation where FileImporter does not expect one, and crashes like that. The minimal fix we should do is to find out when this happens, and report the error as a useful error message to the user.

@Pikne @thiemowmde I'd like to start working on a more meaningful error message. To be sure I'm not misunderstanding:
The file extension of this file https://et.wikipedia.org/wiki/Fail:Kilingi_Nomme_suurvapp.png is png, but the detected mime type is gif. Is it the gif that's wrong or the png? If it's the gif, how could a user fix this?

Thanks for clarifying. Johanna

Pikne added a comment.May 28 2019, 7:07 AM

To be sure I'm not misunderstanding: The file extension of this file https://et.wikipedia.org/wiki/Fail:Kilingi_Nomme_suurvapp.png is png, but the detected mime type is gif. Is it the gif that's wrong or the png? If it's the gif, how could a user fix this?

Yes, file type "gif" is correct, file extension "png" is not. It would be nice if extension could be corrected during import, either manually or automatically.

If changing it during import would be overly complicated, then the error message could inform user that file type and extension do not match, and that one could retry after correcting the extension on source wiki. The latter means that file needs to be moved under new title that has correct extension (on most Wikimedia wikis you need to be an administrator to do that).

If file type and extension do not match, but the file type is not permitted anyway, then it should display a different error message (T222783).

In case FileImporter can't solve these issues automatically, I suggest this error message:

"The extension of the file you are importing (.png) does not match its MIME type (gif). To fix this, rename the original file so that it ends in .gif. This is usually done by admins. After that, you can try another import."

@Pikne What do you think?

Pikne added a comment.May 28 2019, 8:36 AM

Yes, this error message would probably do in case the extension cannot be corrected during import.

JStrodt_WMDE updated the task description. (Show Details)May 28 2019, 8:38 AM
JStrodt_WMDE awarded a token.
Lea_WMDE renamed this task from File extension in page title not matching the actual permitted file type results in an error to Return meaningful error message, if file extension and MIME type are not matching. .May 29 2019, 6:35 AM
Lea_WMDE updated the task description. (Show Details)
Lea_WMDE updated the task description. (Show Details)May 29 2019, 8:44 AM
Lea_WMDE set the point value for this task to 5.
awight claimed this task.May 31 2019, 10:23 AM
awight moved this task from Sprint Backlog to Doing on the WMDE-QWERTY-Sprint-2019-05-29 board.

Here are the internal errors, they look promising:

UploadBase::verifyExtension: mime type image/gif mismatches file extension png, rejecting file
[FileImporter] FileImporter\Services\UploadBase\ValidatingUploadBase::validateFile checks failed

Side note: FileRevisionFromRemoteUrl doesn't short-circuit after title or file type validation, I'm wondering why. Maybe to render a more verbose error message?

I'd like to change it to short-circuit...

@JStrodt_WMDE I've made some changes to the code which allow us to bubble up errors from MediaWiki core, which currently uses the filetype-mime-mismatch message:

I suppose there are three options to align us with the acceptance criteria:

  • Update the MediaWiki core error message to use our more elaborate text. This would have to be slightly reworded to not reference "import", since it might also be displayed in UploadWizard.
  • Special-case our code to catch the core error message, and wrap with a custom message bearing our new text.
  • Display the core error message as it is currently worded.

Change 513586 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/FileImporter@master] Return some localizable error messages from upload validation

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

@awight I would be ok with using the core message for now, since we are showing this from our blocking error page anyways.
Since in the screenshot you posted the message is shown o top of the file import preview page, I'm pasting the blocking error page mock in here again to be sure we have the same thing in mind:

Since in the screenshot you posted the message is shown o top of the file import preview page, I'm pasting the blocking error page mock in here again to be sure we have the same thing in mind:

Thanks for pointing this out! Do you mind if we split the error page into a new task, since it affects all import errors and isn't specific to this one?

Lea_WMDE updated the task description. (Show Details)May 31 2019, 1:07 PM

This error page exists already (I think it was done back then: T189439: Error situations (wrong file format, abuse filter failure...) lead to an error page )

Cool, I see now. However, reading the code I also see that our error page is only displayed for unrecoverable errors that happen during the import "planning" phase, but not for any of the errors that occur during the "actually import" phase.

Change 513586 merged by jenkins-bot:
[mediawiki/extensions/FileImporter@master] Return some localizable error messages from upload validation

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

Lea_WMDE closed this task as Resolved.Jun 12 2019, 1:40 PM
Lea_WMDE moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2019-05-29 board.