Remove dependency on ruby-filemagic in white/blacklist content types

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.

.travis.yml

@@ -45,5 +45,3 @@ notifications:
 
 addons:
   postgresql: "9.3"
-  apt_packages:
-    - libmagic-dev

README.md

@@ -210,7 +210,7 @@ end
 
 Certain files might be dangerous if uploaded to the wrong location, such as php
 files or other script files. CarrierWave allows you to specify a white-list of
-allowed extensions.
+allowed extensions or content types.
 
 If you're mounting the uploader, uploading a file with the wrong extension will
 make the record invalid instead. Otherwise, an error is raised.
@@ -223,6 +223,28 @@ class MyUploader < CarrierWave::Uploader::Base
 end
 ```
 
+The same thing could be done using content types.
+Let's say we need an uploader that accepts only images. This can be done like this
+
+```ruby
+class MyUploader < CarrierWave::Uploader::Base
+  def content_type_whitelist_pattern
+    /image\//
+  end
+end
+```
+
+You can use a blacklist to reject content types.
+Let's say we need an uploader that reject JSON files. This can be done like this
+
+```ruby
+class NoJsonUploader < CarrierWave::Uploader::Base
+  def content_type_blacklist_pattern
+    /(application|text)/json/
+  end
+end
+```
+
 ### Filenames and unicode chars
 
 Another security issue you should care for is the file names (see
@@ -851,56 +873,6 @@ class AvatarUploader < CarrierWave::Uploader::Base
 end
 ```
 
-## Using Filemagic
-
-[Filemagic](https://github.com/blackwinter/ruby-filemagic) is a gem that
-provides Ruby bindings to the magic library.
-
-Since magic is writtern in C, modules using Filemagic are optional and
-don't work with JRuby.
-
-### Validate with the actual content-type
-
-You can use the `MagicMimeWhitelist` mixin to validate uploaded files
-given a regexp to match the allowed content types.
-
-Let's say we need an uploader that accepts only images.
-
-This can be done like this
-
-```ruby
-class AvatarUploader < CarrierWave::Uploader::Base
-  include CarrierWave::Uploader::MagicMimeWhitelist
-
-  # Override it to your needs.
-  # By default it returns nil and the validator allows every
-  # content-type.
-  def whitelist_mime_type_pattern
-    /image\//
-  end
-end
-```
-
-There is also a `MagicMimeBlacklist` mixin to validate uploaded files
-given a rexp to match prohibited content types.
-
-Let's say we need an uploader that reject json files.
-
-This can be done like this
-
-```ruby
-class NoJsonUploader < CarrierWave::Uploader::Base
-  include CarrierWave::Uploader::MagicMimeBlacklist
-
-  # Override it to your needs.
-  # By default it returns nil and the validator allows every
-  # content-type.
-  def blacklist_mime_type_pattern
-    /(application|text)/json/
-  end
-end
-```
-
 ## Migrating from Paperclip
 
 If you are using Paperclip, you can use the provided compatibility module:

carrierwave.gemspec

@@ -43,7 +43,6 @@ Gem::Specification.new do |s|
   s.add_development_dependency "mini_magick", ">= 3.6.0"
   if RUBY_ENGINE != 'jruby'
     s.add_development_dependency "rmagick"
-    s.add_development_dependency "ruby-filemagic", ">= 0.6.3"
   end
   s.add_development_dependency "nokogiri", "~> 1.6.3"
   s.add_development_dependency "timecop", "0.7.1"

lib/carrierwave/locale/el.yml

@@ -6,8 +6,8 @@ el:
       carrierwave_download_error: "δεν ήταν δυνατό να μεταφορτωθεί"
       extension_white_list_error: "Δεν επιτρέπεται το ανέβασμα αρχείων %{extension}, επιτρεπτοί τύποι: %{allowed_types}"
       extension_black_list_error: "Δεν επιτρέπεται το ανέβασμα αρχείων %{extension}, μη επιτρεπτοί τύποι: %{prohibited_types}"
-      mime_type_pattern_white_list_error: "Δεν επιτρέπεται το ανέβασμα αρχείων τύπου %{content_type}"
-      mime_type_pattern_black_list_error: "Δεν επιτρέπεται το ανέβασμα αρχείων τύπου %{content_type}"
+      content_type_whitelist_error: "Δεν επιτρέπεται το ανέβασμα αρχείων τύπου %{content_type}"
+      content_type_blacklist_error: "Δεν επιτρέπεται το ανέβασμα αρχείων τύπου %{content_type}"
       rmagick_processing_error: "Απέτυχε ο χειρισμός με rmagick, ίσως δεν είναι εικόνα; Αρχικό Σφάλμα: %{e}"
       mini_magick_processing_error: "Απέτυχε ο χειρισμός με MiniMagick, ίσως δεν είναι εικόνα; Αρχικό Σφάλμα: %{e}"
       min_size_error: "το μέγεθος του αρχείου θα πρέπει να είναι μεγαλύτερο από %{min_size}"

lib/carrierwave/locale/en.yml

@@ -6,8 +6,8 @@ en:
       carrierwave_download_error: could not be downloaded
       extension_white_list_error: "You are not allowed to upload %{extension} files, allowed types: %{allowed_types}"
       extension_black_list_error: "You are not allowed to upload %{extension} files, prohibited types: %{prohibited_types}"
-      mime_type_pattern_white_list_error: "You are not allowed to upload %{content_type} files"
-      mime_type_pattern_black_list_error: "You are not allowed to upload %{content_type} files"
+      content_type_whitelist_error: "You are not allowed to upload %{content_type} files"
+      content_type_blacklist_error: "You are not allowed to upload %{content_type} files"
       rmagick_processing_error: "Failed to manipulate with rmagick, maybe it is not an image?"
       mini_magick_processing_error: "Failed to manipulate with MiniMagick, maybe it is not an image? Original Error: %{e}"
       min_size_error: "File size should be greater than %{min_size}"

lib/carrierwave/locale/fr.yml

@@ -6,8 +6,8 @@ fr:
       carrierwave_download_error: "Impossible de télécharger l'image."
       extension_white_list_error: "Vous n'êtes pas autorisé à uploader des fichiers %{extension}, types autorisés: %{allowed_types}"
       extension_black_list_error: "Vous n'êtes pas autorisé à uploader des fichiers %{extension}, types interdits: %{prohibited_types}"
-      mime_type_pattern_white_list_error: "Vous n'êtes pas autorisé à uploader des fichiers de type %{content_type}"
-      mime_type_pattern_black_list_error: "Vous n'êtes pas autorisé à uploader des fichiers de type %{content_type}"
+      content_type_whitelist_error: "Vous n'êtes pas autorisé à uploader des fichiers de type %{content_type}"
+      content_type_blacklist_error: "Vous n'êtes pas autorisé à uploader des fichiers de type %{content_type}"
       rmagick_processing_error: "La manipulation d'image avec rmagick a échoué. Peut-être que ce n'est pas une image ? Erreur originale: %{e}"
       mini_magick_processing_error: "La manipulation d'image avec MiniMagick a échoué. Peut-être que ce n'est pas une image ? Erreur originale: %{e}"
       min_size_error: "La taille du fichier doit être supérieure à %{min_size}"

lib/carrierwave/locale/id.yml

@@ -6,8 +6,8 @@ id:
       carrierwave_download_error: gagal download
       extension_white_list_error: "Dilarang mengupload tipe file %{extension}, tipe file yang dibolehkan: %{allowed_types}"
       extension_black_list_error: "Dilarang mengupload tipe file %{extension}, tipe file yang dilarang: %{prohibited_types}"
-      mime_type_pattern_white_list_error: "Anda dilarang mengupload file %{content_type}"
-      mime_type_pattern_black_list_error: "Anda dilarang mengupload file %{content_type}"
+      content_type_whitelist_error: "Anda dilarang mengupload file %{content_type}"
+      content_type_blacklist_error: "Anda dilarang mengupload file %{content_type}"
       rmagick_processing_error: "Gagal memanipulasi dengan rmagick, mungkin file ini bukan gambar?"
       mini_magick_processing_error: "Gagal memanipulasi citra menggunakan MiniMagick, mungkin bukan file gambar? Error semula: %{e}"
       min_size_error: "Ukuran file harus lebih besar daripada %{min_size}"

lib/carrierwave/uploader.rb

@@ -9,8 +9,8 @@
 require "carrierwave/uploader/remove"
 require "carrierwave/uploader/extension_whitelist"
 require "carrierwave/uploader/extension_blacklist"
-require "carrierwave/uploader/magic_mime_whitelist"
-require "carrierwave/uploader/magic_mime_blacklist"
+require "carrierwave/uploader/content_type_whitelist"
+require "carrierwave/uploader/content_type_blacklist"
 require "carrierwave/uploader/file_size"
 require "carrierwave/uploader/processing"
 require "carrierwave/uploader/versions"
@@ -54,6 +54,8 @@ class Base
       include CarrierWave::Uploader::Remove
       include CarrierWave::Uploader::ExtensionWhitelist
       include CarrierWave::Uploader::ExtensionBlacklist
+      include CarrierWave::Uploader::ContentTypeWhitelist
+      include CarrierWave::Uploader::ContentTypeBlacklist
       include CarrierWave::Uploader::FileSize
       include CarrierWave::Uploader::Processing
       include CarrierWave::Uploader::Versions

lib/carrierwave/uploader/content_type_blacklist.rb

@@ -0,0 +1,42 @@
+module CarrierWave
+  module Uploader
+    module ContentTypeBlacklist
+      extend ActiveSupport::Concern
+
+      included do
+        before :cache, :check_content_type_blacklist_pattern!
+      end
+
+      ##
+      # Override this method in your uploader to provide a black list pattern (regexp)
+      # of content-types which are prohibited to be uploaded.
+      # Compares the file's content-type.
+      #
+      # === Returns
+      #
+      # [Regexp] a black list regexp to match the content_type
+      #
+      # === Examples
+      #
+      #     def content_type_blacklist_pattern
+      #       /(text|application)\/json/
+      #     end
+      #
+      def content_type_blacklist_pattern; end
+
+    private
+
+      def check_content_type_blacklist_pattern!(new_file)
+        content_type = new_file.content_type
+        if content_type_blacklist_pattern && content_type.match(content_type_blacklist_pattern)
+          raise CarrierWave::IntegrityError, I18n.translate(:"errors.messages.content_type_blacklist_error", content_type: content_type)
+        end
+      end
+
+      def blacklisted_content_type?(content_type)
+        content_type.match(content_type_blacklist_pattern)
+      end
+
+    end # ContentTypeBlacklist
+  end # Uploader
+end # CarrierWave

lib/carrierwave/uploader/content_type_whitelist.rb

@@ -0,0 +1,42 @@
+module CarrierWave
+  module Uploader
+    module ContentTypeWhitelist
+      extend ActiveSupport::Concern
+
+      included do
+        before :cache, :check_content_type_whitelist_pattern!
+      end
+
+      ##
+      # Override this method in your uploader to provide a white list pattern (regexp)
+      # of content-types which are allowed to be uploaded.
+      # Compares the file's content-type.
+      #
+      # === Returns
+      #
+      # [Regexp] a white list regexp to match the content_type
+      #
+      # === Examples
+      #
+      #     def content_type_whitelist_pattern
+      #       /(text|application)\/json/
+      #     end
+      #
+      def content_type_whitelist_pattern; end
+
+    private
+
+      def check_content_type_whitelist_pattern!(new_file)
+        content_type = new_file.content_type
+        if content_type_whitelist_pattern && !whitelisted_content_type?(content_type)
+          raise CarrierWave::IntegrityError, I18n.translate(:"errors.messages.content_type_whitelist_error", content_type: content_type)
+        end
+      end
+
+      def whitelisted_content_type?(content_type)
+        content_type.match(content_type_whitelist_pattern)
+      end
+
+    end # ContentTypeWhitelist
+  end # Uploader
+end # CarrierWave

spec/spec_helper.rb

@@ -101,6 +101,5 @@ def color_of_pixel(path, x, y)
   config.include CarrierWave::Test::ManipulationHelpers
   if RUBY_ENGINE == 'jruby'
     config.filter_run_excluding :rmagick => true
-    config.filter_run_excluding :filemagic => true
   end
 end

spec/uploader/callback_spec.rb

@@ -3,19 +3,27 @@
 describe CarrierWave::Uploader do
 
   it "should keep callbacks on different classes isolated" do
+    default_before_callbacks = [
+      :check_whitelist!,
+      :check_blacklist!,
+      :check_content_type_whitelist_pattern!,
+      :check_content_type_blacklist_pattern!,
+      :check_size!,
+      :process!
+    ]
     @uploader_class_1 = Class.new(CarrierWave::Uploader::Base)
 
-    # First Uploader only has default before-callback
-    expect(@uploader_class_1._before_callbacks[:cache]).to eq([:check_whitelist!, :check_blacklist!, :check_size!, :process!])
+    # First Uploader only has default before-callbacks
+    expect(@uploader_class_1._before_callbacks[:cache]).to eq(default_before_callbacks)
 
     @uploader_class_2 = Class.new(CarrierWave::Uploader::Base)
     @uploader_class_2.before :cache, :before_cache_callback
 
     # Second Uploader defined with another callback
-    expect(@uploader_class_2._before_callbacks[:cache]).to eq([:check_whitelist!, :check_blacklist!, :check_size!, :process!, :before_cache_callback])
+    expect(@uploader_class_2._before_callbacks[:cache]).to eq(default_before_callbacks + [:before_cache_callback])
 
     # Make sure the first Uploader doesn't inherit the same callback
-    expect(@uploader_class_1._before_callbacks[:cache]).to eq([:check_whitelist!, :check_blacklist!, :check_size!, :process!])
+    expect(@uploader_class_1._before_callbacks[:cache]).to eq(default_before_callbacks)
   end
 
 

spec/uploader/magic_mime_blacklist_spec.rb → spec/uploader/content_type_blacklist_spec.rb

@@ -1,13 +1,12 @@
 require 'spec_helper'
 
-describe CarrierWave::Uploader, filemagic: true do
+describe CarrierWave::Uploader do
 
   before do
     @uploader_class = Class.new(CarrierWave::Uploader::Base) do
-      include CarrierWave::Uploader::MagicMimeBlacklist
 
       # Accepts only images
-      def blacklist_mime_type_pattern
+      def content_type_blacklist_pattern
         /image\//
       end
     end
@@ -25,9 +24,9 @@ def blacklist_mime_type_pattern
       allow(CarrierWave).to receive(:generate_cache_id).and_return('1369894322-345-1234-2255')
     end
 
-    context "when the is no blacklist pattern" do
+    context "when there is no blacklist pattern" do
       before do
-        allow(@uploader).to receive(:blacklist_mime_type_pattern).and_return(nil)
+        allow(@uploader).to receive(:content_type_blacklist_pattern).and_return(nil)
       end
 
       it "does not raise an integrity error" do
@@ -38,15 +37,15 @@ def blacklist_mime_type_pattern
     end
 
     context "when there is a blacklist pattern" do
-      context "and the file has compliant content-type" do
+      context "and the file has a compliant content type" do
         it "does not raise an integrity error" do
           expect {
             @uploader.cache!(File.open(file_path('bork.txt')))
           }.not_to raise_error
         end
       end
 
-      context "and the file has not compliant content-type" do
+      context "and the file has not a compliant content type" do
         it "raises an integrity error" do
           expect {
             @uploader.cache!(File.open(file_path('ruby.gif')))

spec/uploader/magic_mime_whitelist_spec.rb → spec/uploader/content_type_whitelist_spec.rb

@@ -1,13 +1,12 @@
 require 'spec_helper'
 
-describe CarrierWave::Uploader, filemagic: true do
+describe CarrierWave::Uploader do
 
   before do
     @uploader_class = Class.new(CarrierWave::Uploader::Base) do
-      include CarrierWave::Uploader::MagicMimeWhitelist
 
       # Accepts only images
-      def whitelist_mime_type_pattern
+      def content_type_whitelist_pattern
         /image\//
       end
     end
@@ -25,9 +24,9 @@ def whitelist_mime_type_pattern
       allow(CarrierWave).to receive(:generate_cache_id).and_return('1369894322-345-1234-2255')
     end
 
-    context "when the is no whitelist pattern" do
+    context "when there is no whitelist pattern" do
       before do
-        allow(@uploader).to receive(:whitelist_mime_type_pattern).and_return(nil)
+        allow(@uploader).to receive(:content_type_whitelist_pattern).and_return(nil)
       end
 
       it "does not raise an integrity error" do
@@ -38,15 +37,15 @@ def whitelist_mime_type_pattern
     end
 
     context "when there is a whitelist pattern" do
-      context "and the file has compliant content-type" do
+      context "and the file has a compliant content type" do
         it "does not raise an integrity error" do
           expect {
             @uploader.cache!(File.open(file_path('ruby.gif')))
           }.not_to raise_error
         end
       end
 
-      context "and the file has not compliant content-type" do
+      context "and the file has not a compliant content type" do
         it "raises an integrity error" do
           expect {
             @uploader.cache!(File.open(file_path('bork.txt')))