-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MediaHelper: proper check if file is an image (#42105) #43345
Conversation
unable to test as I cannot replicate the reported error |
After some digging it appears that this error occurs if the A simple test script to get the MIME type from a blank PDF file yielded this result:
Meaning this problem will not occur if the If |
i can confirm that this issue occurs when exif is not available. |
@brianteeman Yes, but that code has been modified meanwhile. In PR #16091 it was checked if the file name extension is in the allowed list for images, But later the code was changed so it uses the list of allowed file name extensions for uploads, which can also be e.g. a video or a PDF file: The joomla-cms/libraries/src/Helper/MediaHelper.php Lines 92 to 96 in 4e36f77
So to me the change made by this PR here seems to be right. It replaces the hard-coded |
I have tested this item ✅ successfully on fde2fe8 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43345. |
The fix for unhelpful message is there #38536 |
Hm, the getMimeType() should not care about As for now the PR is okay. |
I have tested this item ✅ successfully on fde2fe8 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43345. |
r2c This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43345. |
Thank you! |
Pull Request for Issue #42105.
Summary of Changes
MediaHelper.php
to properly check if the uploaded file is actually an image, instead of assumingtrue
.Testing Instructions
Upload a PDF file to the media library, this should work properly. Uploading images will also still work fine in accordance with previous behavior.
Actual result BEFORE applying this Pull Request
Uploading a PDF fails, providing an unhelpful error to the user. When checking the request in within the browser the message is along the lines of "This is not a valid image".
Expected result AFTER applying this Pull Request
Uploading a PDF works fine, uploading images still follows the previous behavior.
Link to documentations
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed