Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Converted the file copy on save into a file move

  • Loading branch information...
commit 5eff51c5207dd8eadeec36c3370388bf0e682da4 1 parent 06ee1b3
@jyurek jyurek authored
Showing with 1 addition and 2 deletions.
  1. +1 −2  lib/paperclip/storage.rb
View
3  lib/paperclip/storage.rb
@@ -40,9 +40,8 @@ def flush_writes #:nodoc:
@queued_for_write.each do |style, file|
FileUtils.mkdir_p(File.dirname(path(style)))
logger.info("[paperclip] -> #{path(style)}")
- result = file.stream_to(path(style))
+ FileUtils.mv(file.path, path(style))
file.close
- result.close
end
@queued_for_write = {}
end

6 comments on commit 5eff51c

@nagybence

Paperclip does not set chmod to 644 on uploaded files, which causes unauthorized access running with passenger.

@asheidan

That is probably because a temporary file is created in /tmp with permission 600. When this file is moved (commit 5eff51c…) it keeps that permission. This doesn’t cause a problem if the owner of the files is the same as the user running apache. If this differs the created file is owned by the same user as the owner of the rails-app and thus not readable by apache.

@dsmalko

also please check iostream.rb:12

def to_tempfile tempfile = Tempfile.new(“stream”) tempfile.binmode self.stream_to(tempfile) end

very expensive, why not just move it?

@cristibalan

Regarding the iostream note. I also had a problem recently where I was getting a too many files open error after creating about 60 records while doing an import of data.

Looking at the remaning open files with lsof, I could see they all had the “stream” prefix, which only appears here. Wasn’t able to track it down but, it appears that if the save fails, the files have a higher chance of getting “stuck”.

I’ll try and come up with a reproductible failure soon and post on lighthouse.

@thoughtbot

@dsmalko: Strictly speaking, it’s because to_tempfile is also placed on StringIO, which doesn’t exist on disk. Also, it wasn’t my intention to remove the original file, which moving would do.

@evilchelu: I very much would like to see that. The open file bugs have been very annoying for me. Thanks.

@thoughtbot

Also, thanks @nagybence for alerting me to this.

Please sign in to comment.
Something went wrong with that request. Please try again.