Skip to content

Commit

Permalink
Merge pull request #1825 from mehlah/refine-white/blacklists-options
Browse files Browse the repository at this point in the history
Allow whitelists and blacklists to accept single values
  • Loading branch information
thomasfedb committed Jan 10, 2016
2 parents a3c2ab9 + 6bbaa6c commit 424f028
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 144 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,8 @@ end

## Securing uploads

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
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 whitelist of
allowed extensions or content types.

If you're mounting the uploader, uploading a file with the wrong extension will
Expand All @@ -229,7 +229,7 @@ Let's say we need an uploader that accepts only images. This can be done like th
```ruby
class MyUploader < CarrierWave::Uploader::Base
def content_type_whitelist
[/image\//]
/image\//
end
end
```
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/uploader/content_type_blacklist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module ContentTypeBlacklist
#
# === Returns
#
# [NilClass, Array[String,Regexp]] a blacklist of content types which are not allowed to be uploaded
# [NilClass, String, Regexp, Array[String, Regexp]] a blacklist of content types which are not allowed to be uploaded
#
# === Examples
#
Expand All @@ -40,7 +40,7 @@ def check_content_type_blacklist!(new_file)
end

def blacklisted_content_type?(content_type)
content_type_blacklist.any? { |item| content_type =~ /#{item}/ }
Array(content_type_blacklist).any? { |item| content_type =~ /#{item}/ }
end

end # ContentTypeBlacklist
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/uploader/content_type_whitelist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module ContentTypeWhitelist
#
# === Returns
#
# [NilClass, Array[String,Regexp]] a whitelist of content types which are allowed to be uploaded
# [NilClass, String, Regexp, Array[String, Regexp]] a whitelist of content types which are allowed to be uploaded
#
# === Examples
#
Expand All @@ -40,7 +40,7 @@ def check_content_type_whitelist!(new_file)
end

def whitelisted_content_type?(content_type)
content_type_whitelist.any? { |item| content_type =~ /#{item}/ }
Array(content_type_whitelist).any? { |item| content_type =~ /#{item}/ }
end

end # ContentTypeWhitelist
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/uploader/extension_blacklist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module ExtensionBlacklist
#
# === Returns

# [NilClass, Array[String,Regexp]] a black list of extensions which are prohibited to be uploaded
# [NilClass, String, Regexp, Array[String, Regexp]] a black list of extensions which are prohibited to be uploaded
#
# === Examples
#
Expand All @@ -44,7 +44,7 @@ def check_extension_blacklist!(new_file)
end

def blacklisted_extension?(extension)
extension_blacklist.any? { |item| extension =~ /\A#{item}\z/i }
Array(extension_blacklist).any? { |item| extension =~ /\A#{item}\z/i }
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/carrierwave/uploader/extension_whitelist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module ExtensionWhitelist
#
# === Returns
#
# [NilClass, Array[String,Regexp]] a white list of extensions which are allowed to be uploaded
# [NilClass, String, Regexp, Array[String, Regexp]] a white list of extensions which are allowed to be uploaded
#
# === Examples
#
Expand All @@ -43,7 +43,7 @@ def check_extension_whitelist!(new_file)
end

def whitelisted_extension?(extension)
extension_whitelist.any? { |item| extension =~ /\A#{item}\z/i }
Array(extension_whitelist).any? { |item| extension =~ /\A#{item}\z/i }
end

end # ExtensionWhitelist
Expand Down
54 changes: 36 additions & 18 deletions spec/uploader/content_type_blacklist_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
require 'spec_helper'

describe CarrierWave::Uploader do

before do
@uploader_class = Class.new(CarrierWave::Uploader::Base)
@uploader = @uploader_class.new
Expand All @@ -12,7 +11,6 @@
end

describe '#cache!' do

before do
allow(CarrierWave).to receive(:generate_cache_id).and_return('1369894322-345-1234-2255')
end
Expand All @@ -28,28 +26,48 @@
end

context "when there is a blacklist" do
it "does not raise an integrity error when the file has not a blacklisted content type" do
allow(@uploader).to receive(:content_type_blacklist).and_return(['image/gif'])
context "when the blacklist is an array of values" do
it "does not raise an integrity error when the file has not a blacklisted content type" do
allow(@uploader).to receive(:content_type_blacklist).and_return(['image/gif'])

expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.not_to raise_error
end
expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.not_to raise_error
end

it "raises an integrity error the file has a blacklisted content type" do
allow(@uploader).to receive(:content_type_blacklist).and_return(['image/gif'])
it "raises an integrity error if the file has a blacklisted content type" do
allow(@uploader).to receive(:content_type_blacklist).and_return(['image/gif'])

expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.to raise_error(CarrierWave::IntegrityError)
expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.to raise_error(CarrierWave::IntegrityError)
end

it "accepts content types as regular expressions" do
allow(@uploader).to receive(:content_type_blacklist).and_return([/image\//])

expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.to raise_error(CarrierWave::IntegrityError)
end
end

it "accepts content types as regular expressions" do
allow(@uploader).to receive(:content_type_blacklist).and_return([/image\//])
context "when the blacklist is a single value" do
it "accepts a single extension string value" do
allow(@uploader).to receive(:extension_whitelist).and_return('jpeg')

expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.to raise_error(CarrierWave::IntegrityError)
expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end

it "accepts a single extension regular expression value" do
allow(@uploader).to receive(:extension_whitelist).and_return(/jpe?g/)

expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end
end
end
end
Expand Down
54 changes: 36 additions & 18 deletions spec/uploader/content_type_whitelist_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
require 'spec_helper'

describe CarrierWave::Uploader do

before do
@uploader_class = Class.new(CarrierWave::Uploader::Base)
@uploader = @uploader_class.new
Expand All @@ -12,7 +11,6 @@
end

describe '#cache!' do

before do
allow(CarrierWave).to receive(:generate_cache_id).and_return('1369894322-345-1234-2255')
end
Expand All @@ -28,28 +26,48 @@
end

context "when there is a whitelist" do
it "does not raise an integrity error when the file has a whitelisted content type" do
allow(@uploader).to receive(:content_type_whitelist).and_return(['image/gif'])
context "when the whitelist is an array of values" do
it "does not raise an integrity error when the file has a whitelisted content type" do
allow(@uploader).to receive(:content_type_whitelist).and_return(['image/gif'])

expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.not_to raise_error
end
expect {
@uploader.cache!(File.open(file_path('ruby.gif')))
}.not_to raise_error
end

it "raises an integrity error the file has not a whitelisted content type" do
allow(@uploader).to receive(:content_type_whitelist).and_return(['image/gif'])
it "raises an integrity error the file has not a whitelisted content type" do
allow(@uploader).to receive(:content_type_whitelist).and_return(['image/gif'])

expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.to raise_error(CarrierWave::IntegrityError)
expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.to raise_error(CarrierWave::IntegrityError)
end

it "accepts content types as regular expressions" do
allow(@uploader).to receive(:content_type_whitelist).and_return([/image\//])

expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.to raise_error(CarrierWave::IntegrityError)
end
end

it "accepts content types as regular expressions" do
allow(@uploader).to receive(:content_type_whitelist).and_return([/image\//])
context "when the whitelist is a single value" do
it "accepts a single extension string value" do
allow(@uploader).to receive(:extension_whitelist).and_return('jpeg')

expect {
@uploader.cache!(File.open(file_path('bork.txt')))
}.to raise_error(CarrierWave::IntegrityError)
expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end

it "accepts a single extension regular expression value" do
allow(@uploader).to receive(:extension_whitelist).and_return(/jpe?g/)

expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end
end
end
end
Expand Down
126 changes: 78 additions & 48 deletions spec/uploader/extension_blacklist_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,66 +11,96 @@
end

describe '#cache!' do

before do
allow(CarrierWave).to receive(:generate_cache_id).and_return('1369894322-345-1234-2255')
end

it "should not raise an integrity error if there is no blacklist" do
allow(@uploader).to receive(:extension_blacklist).and_return(nil)
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).not_to raise_error
end
context "when there is no blacklist" do
it "should not raise an integrity error" do
allow(@uploader).to receive(:extension_blacklist).and_return(nil)

it "should raise an integrity error if there is a blacklist and the file is on it" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(jpg gif png))
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).to raise_error(CarrierWave::IntegrityError)
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).not_to raise_error
end
end

it "should not raise an integrity error if there is a blacklist and the file is not on it" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt doc xls))
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).not_to raise_error
end
context "when there is a blacklist" do
context "when the blacklist is an array of values" do
it "raises an integrity error if the file has a blacklisted extension" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(jpg gif png))

it "should not raise an integrity error if there is a blacklist and the file is not on it, using start of string matcher" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt))
expect(running {
@uploader.cache!(File.open(file_path('bork.ttxt')))
}).not_to raise_error
end
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).to raise_error(CarrierWave::IntegrityError)
end

it "should not raise an integrity error if there is a blacklist and the file is not on it, using end of string matcher" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt))
expect(running {
@uploader.cache!(File.open(file_path('bork.txtt')))
}).not_to raise_error
end
it "does not raise an integrity error if the file has a blacklisted extension" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt doc xls))

it "should compare blacklist in a case insensitive manner when capitalized extension provided" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(jpg gif png))
expect(running {
@uploader.cache!(File.open(file_path('case.JPG')))
}).to raise_error(CarrierWave::IntegrityError)
end
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).not_to raise_error
end

it "should compare blacklist in a case insensitive manner when lowercase extension provided" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(JPG GIF PNG))
expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).to raise_error(CarrierWave::IntegrityError)
end
it "does not raise an integrity error if the file has a blacklisted extension, using start of string matcher" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt))

expect(running {
@uploader.cache!(File.open(file_path('bork.ttxt')))
}).not_to raise_error
end

it "does not raise an integrity error if the file has a blacklisted extension, using end of string matcher" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(txt))

expect(running {
@uploader.cache!(File.open(file_path('bork.txtt')))
}).not_to raise_error
end

it "should accept and check regular expressions" do
allow(@uploader).to receive(:extension_blacklist).and_return([/jpe?g/, 'gif', 'png'])
expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).to raise_error(CarrierWave::IntegrityError)
it "compares extensions in a case insensitive manner when capitalized extension provided" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(jpg gif png))

expect(running {
@uploader.cache!(File.open(file_path('case.JPG')))
}).to raise_error(CarrierWave::IntegrityError)
end

it "compares extensions in a case insensitive manner when lowercase extension provided" do
allow(@uploader).to receive(:extension_blacklist).and_return(%w(JPG GIF PNG))

expect(running {
@uploader.cache!(File.open(file_path('test.jpg')))
}).to raise_error(CarrierWave::IntegrityError)
end

it "accepts extensions as regular expressions" do
allow(@uploader).to receive(:extension_blacklist).and_return([/jpe?g/, 'gif', 'png'])

expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).to raise_error(CarrierWave::IntegrityError)
end
end

context "when the blacklist is a single value" do
it "accepts a single extension string value" do
allow(@uploader).to receive(:extension_whitelist).and_return('jpeg')

expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end

it "accepts a single extension regular expression value" do
allow(@uploader).to receive(:extension_whitelist).and_return(/jpe?g/)

expect(running {
@uploader.cache!(File.open(file_path('test.jpeg')))
}).not_to raise_error
end
end
end
end

end
Loading

0 comments on commit 424f028

Please sign in to comment.