[#25351] - Better check mime type of uploaded file
- Closed
- 31 Oct 2020
- Medium
- Build: staging
- # 25351
- Diff
- Quy:patch-1
- Success Hound No violations found. Woof! Details
- Success continuous-integration/drone/pr Build is passing Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #16086
Credits go to @csthomas for PR #20968
Summary of Changes
A better way to check the MIME file type if the operating system is incorrectly configured.
If you still have a problem and your server is not running on Windows, test the following patch:
diff --git a/libraries/src/Helper/MediaHelper.php b/libraries/src/Helper/MediaHelper.php
index 60f7fab83d..99cb1d80e1 100644
--- a/libraries/src/Helper/MediaHelper.php
+++ b/libraries/src/Helper/MediaHelper.php
@@ -102,6 +102,12 @@ class MediaHelper
$mime = isset($imagesize['mime']) ? $imagesize['mime'] : false;
}
}
+
+ // Last chance to get to know the type of mime
+ if (!$mime && IS_UNIX && function_exists('exec') && function_exists('escapeshellarg'))
+ {
+ $mime = strtok(exec('file -bi ' . escapeshellarg($file)), ';');
+ }
}
catch (\Exception $e)
{
Testing Instructions
Try to upload pdf or image file.
Take a look at #16086
Please test if you still interested @goforitweb, @uglyeoin, @edthenet, @OrignlCin
Expected result
PDF file is recognized.
Actual result
Documentation Changes Required
No
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
?
|
||
there were several issues for this Pull Request, so i hope @goforitweb, @uglyeoin, @edthenet, @OrignlCin will test it.
@franz-wohlkoenig Thanks for tagging me. I'm happy to test.
I have not applied the patch.
I went to the media manager.
I uploaded a PDF.
It worked.
I went to an article, I clicked to upload an image, I uploaded a PDF. It worked. I couldn't see it until I went to the media manager but it worked.
Do I need different testing instructions? It all seems to work without the patch.
A better way to check the MIME file type if the operating system is incorrectly configured.
If you did not have a problem uploading then other than confirming that you still dont have a problem uploading with this PR there is nothing else you can do.
What this PR really needs is tests from people who do have a problem uploading to see if this resolves that
@brianteeman I did previously have a problem. I wish I could remember which PDF in case it was specifically tied to one.
Well in theory it should not be bound to an specific file but an specific server setup. But I personally would suggest to not merge this change without understanding what the actual issues is we are fixing here / how to actually reproduce the problem.
It is possible that either I or my host changed something. I don't know what if I did anything. If it reoccurs I will come back.
I tried to upload STEP files (inside a ZIP) and this patch did not work unfortunately, I added stp,step,STP,STEP to the extension list and text/plain to the mime types. unfortunately I can't remove text/html from disallowes extenstions to test if thats the issue.
If they are inside a zip then the problem is not the same as this one. Allowed extensions and mime types would apply to the zip and not to the contents of the zip
@brianteeman thank you, I opened issue #26408 for it.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2020-10-31 16:46:09 |
| Closed_By | ⇒ | Quy |
I'm not that deep into the issue we fixed back than in 3.7.1 but I would like to call @SniperSister @joomla/security to take a look here too. And I would like to understand why it worked and now it does not anymore? As the issue mention here should have been fixed by: #16091 lets be carefull here please. IIRC there was a reason we first checked with the image detectors and than with the the more generic ones.