Remove ruby-filemagic dependency and refactor/redesign content type white/blacklisting feature #1816
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mehlah
changed the title
mehlah
changed the title
mehlah
force-pushed
the
remove-magicmime
branch
2 times, most recently
from
58fb7a5
to
784b485
Compare
| @@ -3,19 +3,27 @@ | |||
| describe CarrierWave::Uploader do | |||
|
|
|||
| it "should keep callbacks on different classes isolated" do | |||
| default_before_callbacks = [ | |||
| :check_whitelist!, | |||
There was a problem hiding this comment.
Might be worth changing check_whitelist! to check_extension_whitelist! and similar. Happy to do it as a separate PR though.
There was a problem hiding this comment.
I actually did it already 
mehlah/carrierwave@remove-magicmime...mehlah:refactor-extensions-white/blacklist#diff-2bca1d8b1d06f3d29a6553468c7b940fR7
I'll PR the refactor for better consistency as soon as this is accepted.
added 4 commits
January 2, 2016 18:56
See carrierwaveuploader#1813 for background.
Before this, CarrierWave was relying on mime-types or ruby-filemagic gems to get/set the content type of a file in an inconsistent way. After making mime-types a runtime dependency for CW, `CarrierWave::MimeTypes` has been dropped (carrierwaveuploader#1813 for details), and `CarrierWave::MagicMimeTypes` as well. White/blacklisting content types feature (introduced by carrierwaveuploader#1596) was relying only on ruby-filemagic to read the content type of a file. It was optional and required mixing a module to the uploader. This commit removes this dependency by using the already available content type (thanks to `Mime::Types`). This let us drop any dependency on ruby-filemagic in CarrierWave. It makes JRuby support better as ruby-filemagic is unsupported on JRuby due to the C extension.
This is more consistent with the current way of white/blacklisting extensions.
mehlah
force-pushed
the
remove-magicmime
branch
from
784b485
to
fe98ee3
Compare
|
I screwed up 6df8c83 after a rebase (made the build red https://travis-ci.org/carrierwaveuploader/carrierwave/jobs/99844432). Now fixed ;) |
thomasfedb
added a commit
that referenced
this pull request
Jan 2, 2016
Refactor content type filtering and remove ruby-filemagic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes
CarrierWave::MagicMimeTypesprocessor module at d6fa112 (see #1813 for background).As discussed with @thomasfedb (#1813 (comment)), it also remove the dependency on
ruby-filemagicin white/blacklist content types (b5bec3d).Background: Before this, CarrierWave was relying on mime-types or ruby-filemagic
gems to get/set the content type of a file in an inconsistent way.
After making mime-types a runtime dependency for CW,
CarrierWave::MimeTypeshas been dropped, andCarrierWave::MagicMimeTypesas well with this.White/blacklisting content types feature (introduced by #1596) was relying only on ruby-filemagic to read the content type of a file.
It was optional and required mixing a module to the uploader.
This PR removes this dependency by using the already available content type (thanks to
Mime::Types).This let us drop any dependency on ruby-filemagic in CarrierWave.
It makes JRuby support better as ruby-filemagic is unsupported on JRuby due to the C extension.
I also made some refactors, renamed methods like
whitelist_mime_type_patterntocontent_type_whitelist, cleaned up specs and docs.The module doesn't need to be optional now, so it's included by default (like
ExtensionWhitelistandExtensionBlacklist)To keep it consistent with the current way of white/blacklisting extensions,
content_type_whitelistnow accepts an array of strings or regexps to white/blacklist content types. I can move 07c80cb to a separate PR if you prefer.