Permalink
Browse files

Support setting permissions of created directories

This adds a new uploader configuration option :directory_permissions
which allows to set the permissions of directories created by the
uploader. Specs have been added or fixed to reflect the changes.
This does NOT modify the permissions of existing directories because
that could be potentially harmful.
  • Loading branch information...
1 parent 6670975 commit 26a4d3431d50b6ee57b75f476f54598e3a83c7cd @felixbuenemann felixbuenemann committed Apr 17, 2012
View
@@ -406,6 +406,7 @@ both globally and on a per-uploader basis:
```ruby
CarrierWave.configure do |config|
config.permissions = 0666
+ config.directory_permissions = 0777
config.storage = :file
end
```
@@ -168,12 +168,13 @@ def read
#
# [new_path (String)] The path where the file should be moved.
# [permissions (Integer)] permissions to set on the file in its new location.
+ # [directory_permissions (Integer)] permissions to set on created directories.
#
- def move_to(new_path, permissions=nil)
+ def move_to(new_path, permissions=nil, directory_permissions=nil)
return if self.empty?
new_path = File.expand_path(new_path)
- mkdir!(new_path)
+ mkdir!(new_path, directory_permissions)
if exists?
FileUtils.mv(path, new_path) unless new_path == path
else
@@ -191,16 +192,17 @@ def move_to(new_path, permissions=nil)
#
# [new_path (String)] The path where the file should be copied to.
# [permissions (Integer)] permissions to set on the copy
+ # [directory_permissions (Integer)] permissions to set on created directories.
#
# === Returns
#
# @return [CarrierWave::SanitizedFile] the location where the file will be stored.
#
- def copy_to(new_path, permissions=nil)
+ def copy_to(new_path, permissions=nil, directory_permissions=nil)
return if self.empty?
new_path = File.expand_path(new_path)
- mkdir!(new_path)
+ mkdir!(new_path, directory_permissions)
if exists?
FileUtils.cp(path, new_path) unless new_path == path
else
@@ -278,8 +280,10 @@ def file=(file)
end
# create the directory if it doesn't exist
- def mkdir!(path)
- FileUtils.mkdir_p(File.dirname(path)) unless File.exists?(File.dirname(path))
+ def mkdir!(path, directory_permissions)
+ options = {}
+ options[:mode] = directory_permissions if directory_permissions
+ FileUtils.mkdir_p(File.dirname(path), options) unless File.exists?(File.dirname(path))
end
def chmod!(path, permissions)
@@ -29,9 +29,9 @@ class File < Abstract
def store!(file)
path = ::File.expand_path(uploader.store_path, uploader.root)
if uploader.move_to_store
- file.move_to(path, uploader.permissions)
+ file.move_to(path, uploader.permissions, uploader.directory_permissions)
else
- file.copy_to(path, uploader.permissions)
+ file.copy_to(path, uploader.permissions, uploader.directory_permissions)
end
end
@@ -64,6 +64,34 @@ def have_permissions(expected)
HavePermissions.new(expected)
end
+ class HaveDirectoryPermissions # :nodoc:
+ def initialize(expected)
+ @expected = expected
+ end
+
+ def matches?(actual)
+ @actual = actual
+ # Satisfy expectation here. Return false or raise an error if it's not met.
+ (File.stat(File.dirname @actual.path).mode & 0777) == @expected
+ end
+
+ def failure_message
+ "expected #{File.dirname @actual.current_path.inspect} to have permissions #{@expected.to_s(8)}, but they were #{(File.stat(@actual.path).mode & 0777).to_s(8)}"
+ end
+
+ def negative_failure_message
+ "expected #{File.dirname @actual.current_path.inspect} not to have permissions #{@expected.to_s(8)}, but it did"
+ end
+
+ def description
+ "have permissions #{@expected.to_s(8)}"
+ end
+ end
+
+ def have_directory_permissions(expected)
+ HaveDirectoryPermissions.new(expected)
+ end
+
class BeNoLargerThan # :nodoc:
def initialize(width, height)
@width, @height = width, height
@@ -116,9 +116,9 @@ def cache!(new_file)
self.original_filename = new_file.filename
if move_to_cache
- @file = new_file.move_to(cache_path, permissions)
+ @file = new_file.move_to(cache_path, permissions, directory_permissions)
else
- @file = new_file.copy_to(cache_path, permissions)
+ @file = new_file.copy_to(cache_path, permissions, directory_permissions)
end
end
end
@@ -10,6 +10,7 @@ module Configuration
add_config :root
add_config :base_path
add_config :permissions
+ add_config :directory_permissions
add_config :storage_engines
add_config :store_dir
add_config :cache_dir
@@ -103,6 +104,7 @@ def configure
def reset_config
configure do |config|
config.permissions = 0644
+ config.directory_permissions = 0755
config.storage_engines = {
:file => "CarrierWave::Storage::File",
:fog => "CarrierWave::Storage::Fog"
@@ -247,7 +247,7 @@
describe '#move_to' do
after do
- FileUtils.rm(file_path('gurr.png'))
+ FileUtils.rm_f(file_path('gurr.png'))
end
it "should be moved to the correct location" do
@@ -281,6 +281,12 @@
@sanitized_file.should have_permissions(0755)
end
+ it "should set the right directory permissions" do
+ @sanitized_file.move_to(file_path('new_dir','gurr.png'), nil, 0775)
+ @sanitized_file.should have_directory_permissions(0775)
+ FileUtils.rm_rf(file_path('new_dir'))
+ end
+
it "should return itself" do
@sanitized_file.move_to(file_path('gurr.png')).should == @sanitized_file
end
@@ -290,7 +296,7 @@
describe '#copy_to' do
after do
- FileUtils.rm(file_path('gurr.png'))
+ FileUtils.rm_f(file_path('gurr.png'))
end
it "should be copied to the correct location" do
@@ -339,6 +345,12 @@
new_file.should have_permissions(0755)
end
+ it "should set the right directory permissions" do
+ new_file = @sanitized_file.copy_to(file_path('new_dir', 'gurr.png'), nil, 0755)
+ new_file.should have_directory_permissions(0755)
+ FileUtils.rm_rf(file_path('new_dir'))
+ end
+
it "should preserve the file's content type" do
new_file = @sanitized_file.copy_to(file_path('gurr.png'))
new_file.content_type.should == @sanitized_file.content_type
@@ -112,6 +112,13 @@
@uploader.should have_permissions(0777)
end
+ it "should set directory permissions if options are given" do
+ @uploader_class.directory_permissions = 0777
+
+ @uploader.cache!(File.open(file_path('test.jpg')))
+ @uploader.should have_directory_permissions(0777)
+ end
+
describe "with ensuring multipart form deactivated" do
before do
@@ -148,7 +155,8 @@
CarrierWave.stub!(:generate_cache_id).and_return('20071201-1234-345-2255')
@cached_path = public_path('uploads/tmp/20071201-1234-345-2255/test_move.jpeg')
- @uploader_class.permissions = 777
+ @uploader_class.permissions = 0777
+ @uploader_class.directory_permissions = 0777
end
after do
@@ -169,7 +177,7 @@
end
it "should use move_to() during cache!()" do
- CarrierWave::SanitizedFile.any_instance.should_receive(:move_to).with(@cached_path, 777)
+ CarrierWave::SanitizedFile.any_instance.should_receive(:move_to).with(@cached_path, 0777, 0777)
CarrierWave::SanitizedFile.any_instance.should_not_receive(:copy_to)
@uploader.cache!(@tmpfile)
end
@@ -189,7 +197,7 @@
end
it "should use copy_to() during cache!()" do
- CarrierWave::SanitizedFile.any_instance.should_receive(:copy_to).with(@cached_path, 777)
+ CarrierWave::SanitizedFile.any_instance.should_receive(:copy_to).with(@cached_path, 0777, 0777)
CarrierWave::SanitizedFile.any_instance.should_not_receive(:move_to)
@uploader.cache!(@tmpfile)
end
@@ -75,6 +75,13 @@
@uploader.should have_permissions(0777)
end
+ it "should set directory permissions if options are given" do
+ @uploader_class.directory_permissions = 0777
+
+ @uploader.download!('http://www.example.com/test.jpg')
+ @uploader.should have_directory_permissions(0777)
+ end
+
it "should raise an error when trying to download a local file" do
running {
@uploader.download!('/etc/passwd')
@@ -331,7 +331,8 @@ def filename
before do
@file = File.open(file_path('test.jpg'))
- @uploader_class.permissions = 777
+ @uploader_class.permissions = 0777
+ @uploader_class.directory_permissions = 0777
CarrierWave.stub!(:generate_cache_id).and_return('20071201-1234-345-2255')
end
@@ -360,7 +361,7 @@ def filename
@uploader.cache!(@file)
@stored_path = ::File.expand_path(@uploader.store_path, @uploader.root)
- @uploader.file.should_receive(:move_to).with(@stored_path, 777)
+ @uploader.file.should_receive(:move_to).with(@stored_path, 0777, 0777)
@uploader.file.should_not_receive(:copy_to)
@uploader.store!
@@ -376,7 +377,7 @@ def filename
@uploader.cache!(@file)
@stored_path = ::File.expand_path(@uploader.store_path, @uploader.root)
- @uploader.file.should_receive(:copy_to).with(@stored_path, 777)
+ @uploader.file.should_receive(:copy_to).with(@stored_path, 0777, 0777)
@uploader.file.should_not_receive(:move_to)
@uploader.store!

0 comments on commit 26a4d34

Please sign in to comment.