Skip to content

Commit

Permalink
Remove dependency on ruby-filemagic in white/blacklist content types
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mehdi Lahmam committed Jan 2, 2016
1 parent 2e894cd commit 319b02d
Show file tree
Hide file tree
Showing 16 changed files with 147 additions and 275 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,3 @@ notifications:

addons:
postgresql: "9.3"
apt_packages:
- libmagic-dev
74 changes: 23 additions & 51 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion carrierwave.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/locale/el.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/locale/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/locale/id.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
6 changes: 4 additions & 2 deletions lib/carrierwave/uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,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"
Expand Down Expand Up @@ -56,6 +56,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
Expand Down
44 changes: 44 additions & 0 deletions lib/carrierwave/uploader/content_type_blacklist.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# encoding: utf-8

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
44 changes: 44 additions & 0 deletions lib/carrierwave/uploader/content_type_whitelist.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# encoding: utf-8

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
96 changes: 0 additions & 96 deletions lib/carrierwave/uploader/magic_mime_blacklist.rb

This file was deleted.

Loading

0 comments on commit 319b02d

Please sign in to comment.