Permalink
Browse files

Merge pull request #707 from fbuenemann/feature/directory_permissions

Support setting permissions of created directories
  • Loading branch information...
2 parents b038398 + 26a4d34 commit ebebfdff288fff622550aea7500f56d62eb3199d @trevorturk trevorturk committed Apr 26, 2012
View
1 README.md
@@ -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
```
View
16 lib/carrierwave/sanitized_file.rb
@@ -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)
View
4 lib/carrierwave/storage/file.rb
@@ -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
View
28 lib/carrierwave/test/matchers.rb
@@ -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
View
4 lib/carrierwave/uploader/cache.rb
@@ -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
View
2 lib/carrierwave/uploader/configuration.rb
@@ -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"
View
16 spec/sanitized_file_spec.rb
@@ -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
View
14 spec/uploader/cache_spec.rb
@@ -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
View
7 spec/uploader/download_spec.rb
@@ -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')
View
7 spec/uploader/store_spec.rb
@@ -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 ebebfdf

Please sign in to comment.