Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Currently file_get_mimetype() incorrectly reports any file with an uppercase (or mixed case) extension as 'application/octet-stream', since the comparison is only done on all-lowercase extensions. This patch converts the file extension to lowercase before performing the check. Note that there was also a single extension in the mime-type list, "pcf.Z" that included an uppercase character. I converted it to lowercase so it won't be broken by this change.
The use of drupal_strtolower() is not necessary here, because none of the extensions that we support contain any UTF8 or non-latin characters.
Comment | File | Size | Author |
---|---|---|---|
#7 | file_mime_case_sensitive_520664_03.patch | 5.91 KB | rfay |
#5 | file_mime_case_sensitive_520664_02.patch | 5.91 KB | rfay |
drupal_file_mime_case_sensitive.patch | 1.23 KB | quicksketch | |
Comments
Comment #1
quicksketchThis problem was made clear by uploading .JPG files for me in the new File module, so I'm tagging with the image tag. It'll be a clear problem if pictures are all being marked as application/octet-stream.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedIndeed. Do we have tests that would be worth extending for that?
Comment #3
drewish CreditAttribution: drewish commentedWhile we're in here we need to fix up some documentation.
file_default_mimetype_mapping() doesn't document it's return value at all and could use a @see to file_get_mimetype(). Additionally it needs a comment where the extensions are set reminding future developers to ensure that they're all lowercase.
file_get_mimetype() also doesn't document that the extensions should be lowercase. I'd like it to mention that if no value is provided that the variable mime_extension_mapping is checked for a value and then file_default_mimetype_mapping() is called. And a @see to file_default_mimetype_mapping() would be good.
As dmaz notes, the tests need to be considered. We have tests in FileMimeTypeTest and testFileMimeTypeDetection() part actually passes in a custom mapping with 'pcf.Z' => 'application/octet-stream', in it but since it's using the default octet-stream it won't be catching our casting to lower case changes... though who ever committed this half baked MIME patch didn't require tests for setting a mapping through the mime_extension_mapping variable.
Comment #4
rfayPer drewish I will roll a new patch.
Comment #5
rfayOK, I think I covered the issues in #3.
I also added quite a lot of test coverage in the tests by adding sections that test with and without the variable set, and that test when you pass in a mapping.
Comment #6
quicksketchNice! The extra tests look handy and thorough. Just a few code style fixes:
ID tag (though I think either way works technically):
No extra newline between @return and @see.
Sentence-case comments.
Close parentheses at same level.
End comments in a period.
Other than that, this looks great!
Comment #7
rfayI think this patch adds on the style issues in #6.
Comment #8
quicksketchLooks fantastic!
Comment #9
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!