#58 Applied Rob Anderton's patch, fixing many windows-related issues.

lib/paperclip/attachment.rb

@@ -62,6 +62,7 @@ def assign uploaded_file
 
       if uploaded_file.is_a?(Paperclip::Attachment)
         uploaded_file = uploaded_file.to_file(:original)
+        close_uploaded_file = true if uploaded_file.respond_to?(:close)
       end
 
       return nil unless valid_assignment?(uploaded_file)
@@ -88,6 +89,7 @@ def assign uploaded_file
       # Reset the file size if the original file was reprocessed.
       instance_write(:file_size, uploaded_file.size.to_i)
     ensure
+      uploaded_file.close if close_uploaded_file
       validate
     end
 
@@ -191,6 +193,7 @@ def self.interpolations
     # again.
     def reprocess!
       new_original = Tempfile.new("paperclip-reprocess")
+      new_original.binmode
       if old_original = to_file(:original)
         new_original.write( old_original.read )
         new_original.rewind

lib/paperclip/storage.rb

@@ -31,7 +31,7 @@ def exists?(style = default_style)
       # Returns representation of the data of the file assigned to the given
       # style, in the format most representative of the current storage.
       def to_file style = default_style
-        @queued_for_write[style] || (File.new(path(style)) if exists?(style))
+        @queued_for_write[style] || (File.new(path(style), 'rb') if exists?(style))
       end
       alias_method :to_io, :to_file
 

test/attachment_test.rb

@@ -77,7 +77,7 @@ class AttachmentTest < Test::Unit::TestCase
       @dummy.stubs(:id).returns(1024)
       @file = File.new(File.join(File.dirname(__FILE__),
                                  "fixtures",
-                                 "5k.png"))
+                                 "5k.png"), 'rb')
       @dummy.avatar = @file
     end
 
@@ -95,7 +95,7 @@ class AttachmentTest < Test::Unit::TestCase
       @dummy.stubs(:id).returns(@id)
       @file = File.new(File.join(File.dirname(__FILE__),
                                  "fixtures",
-                                 "5k.png"))
+                                 "5k.png"), 'rb')
       @dummy.avatar = @file
     end
 
@@ -131,7 +131,7 @@ class AttachmentTest < Test::Unit::TestCase
       setup do
         @file = File.new(File.join(File.dirname(__FILE__),
                                    "fixtures",
-                                   "5k.png"))
+                                   "5k.png"), 'rb')
         Paperclip::Thumbnail.stubs(:make)
         [:thumb, :large].each do |style|
           @dummy.avatar.stubs(:extra_options_for).with(style)
@@ -213,7 +213,7 @@ class AttachmentTest < Test::Unit::TestCase
       @attachment = Paperclip::Attachment.new(:avatar, @instance)
       @file = File.new(File.join(File.dirname(__FILE__),
                                  "fixtures",
-                                 "5k.png"))
+                                 "5k.png"), 'rb')
     end
 
     should "raise if there are not the correct columns when you try to assign" do
@@ -300,25 +300,27 @@ class AttachmentTest < Test::Unit::TestCase
             end
 
             should "return the real url" do
-              assert @attachment.to_file
+              file = @attachment.to_file
+              assert file
               assert_match %r{^/avatars/#{@instance.id}/original/5k\.png}, @attachment.url
               assert_match %r{^/avatars/#{@instance.id}/small/5k\.jpg}, @attachment.url(:small)
+              file.close
             end
 
             should "commit the files to disk" do
               [:large, :medium, :small].each do |style|
                 io = @attachment.to_io(style)
                 assert File.exists?(io)
                 assert ! io.is_a?(::Tempfile)
+                io.close
               end
             end
 
             should "save the files as the right formats and sizes" do
               [[:large, 400, 61, "PNG"],
                [:medium, 100, 15, "GIF"],
                [:small, 32, 32, "JPEG"]].each do |style|
-                cmd = "identify -format '%w %h %b %m' " + 
-                      "#{@attachment.to_io(style.first).path}"
+                cmd = %Q[identify -format "%w %h %b %m" "#{@attachment.path(style.first)}"]
                 out = `#{cmd}`
                 width, height, size, format = out.split(" ")
                 assert_equal style[1].to_s, width.to_s 
@@ -328,7 +330,8 @@ class AttachmentTest < Test::Unit::TestCase
             end
 
             should "still have its #file attribute not be nil" do
-              assert ! @attachment.to_file.nil?
+              assert ! (file = @attachment.to_file).nil?
+              file.close
             end
 
             context "and deleted" do

test/geometry_test.rb

@@ -93,7 +93,7 @@ class GeometryTest < Test::Unit::TestCase
 
     should "be generated from a file" do
       file = File.join(File.dirname(__FILE__), "fixtures", "5k.png")
-      file = File.new(file)
+      file = File.new(file, 'rb')
       assert_nothing_raised{ @geo = Paperclip::Geometry.from_file(file) }
       assert @geo.height > 0
       assert @geo.width > 0

test/integration_test.rb

@@ -4,7 +4,7 @@ class IntegrationTest < Test::Unit::TestCase
   context "Many models at once" do
     setup do
       rebuild_model
-      @file      = File.new(File.join(FIXTURES_DIR, "5k.png"))
+      @file      = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb')
       300.times do |i|
         Dummy.create! :avatar => @file
       end
@@ -24,13 +24,13 @@ class IntegrationTest < Test::Unit::TestCase
       @dummy = Dummy.new
       @file = File.new(File.join(File.dirname(__FILE__),
                                  "fixtures",
-                                 "5k.png"))
+                                 "5k.png"), 'rb')
       @dummy.avatar = @file
       assert @dummy.save
     end
 
     should "create its thumbnails properly" do
-      assert_match /\b50x50\b/, `identify '#{@dummy.avatar.path(:thumb)}'`
+      assert_match /\b50x50\b/, `identify "#{@dummy.avatar.path(:thumb)}"`
     end
 
     context "redefining its attachment styles" do
@@ -44,7 +44,7 @@ class IntegrationTest < Test::Unit::TestCase
       end
 
       should "create its thumbnails properly" do
-        assert_match /\b150x25\b/, `identify '#{@dummy.avatar.path(:thumb)}'`
+        assert_match /\b150x25\b/, `identify "#{@dummy.avatar.path(:thumb)}"`
       end
     end
   end
@@ -133,8 +133,8 @@ class IntegrationTest < Test::Unit::TestCase
                     :url => "/:attachment/:class/:style/:id/:basename.:extension",
                     :path => ":rails_root/tmp/:attachment/:class/:style/:id/:basename.:extension"
       @dummy     = Dummy.new
-      @file      = File.new(File.join(FIXTURES_DIR, "5k.png"))
-      @bad_file  = File.new(File.join(FIXTURES_DIR, "bad.png"))
+      @file      = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb')
+      @bad_file  = File.new(File.join(FIXTURES_DIR, "bad.png"), 'rb')
 
       assert @dummy.avatar = @file
       assert @dummy.valid?
@@ -146,18 +146,18 @@ class IntegrationTest < Test::Unit::TestCase
        ["300x46", :large],
        ["100x15", :medium],
        ["32x32", :thumb]].each do |geo, style|
-        cmd = %Q[identify -format "%wx%h" #{@dummy.avatar.to_file(style).path}]
+        cmd = %Q[identify -format "%wx%h" "#{@dummy.avatar.path(style)}"]
         assert_equal geo, `#{cmd}`.chomp, cmd
       end
 
-      saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.to_file(s).path }
+      saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.path(s) }
 
       @d2 = Dummy.find(@dummy.id)
-      assert_equal "100x15", `identify -format "%wx%h" #{@d2.avatar.to_file.path}`.chomp
-      assert_equal "434x66", `identify -format "%wx%h" #{@d2.avatar.to_file(:original).path}`.chomp
-      assert_equal "300x46", `identify -format "%wx%h" #{@d2.avatar.to_file(:large).path}`.chomp
-      assert_equal "100x15", `identify -format "%wx%h" #{@d2.avatar.to_file(:medium).path}`.chomp
-      assert_equal "32x32",  `identify -format "%wx%h" #{@d2.avatar.to_file(:thumb).path}`.chomp
+      assert_equal "100x15", `identify -format "%wx%h" "#{@d2.avatar.path}"`.chomp
+      assert_equal "434x66", `identify -format "%wx%h" "#{@d2.avatar.path(:original)}"`.chomp
+      assert_equal "300x46", `identify -format "%wx%h" "#{@d2.avatar.path(:large)}"`.chomp
+      assert_equal "100x15", `identify -format "%wx%h" "#{@d2.avatar.path(:medium)}"`.chomp
+      assert_equal "32x32",  `identify -format "%wx%h" "#{@d2.avatar.path(:thumb)}"`.chomp
 
       @dummy.avatar = "not a valid file but not nil"
       assert_equal File.basename(@file.path), @dummy.avatar_file_name
@@ -186,10 +186,10 @@ class IntegrationTest < Test::Unit::TestCase
 
       assert_equal @dummy.avatar_file_name, @d2.avatar_file_name
       [:thumb, :medium, :large, :original].each do |style|
-        assert_equal @dummy.avatar.to_file(style).path, @d2.avatar.to_file(style).path
+        assert_equal @dummy.avatar.path(style), @d2.avatar.path(style)
       end
 
-      saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.to_file(s).path }
+      saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.path(s) }
 
       @d2.avatar = nil
       assert @d2.save
@@ -203,7 +203,8 @@ class IntegrationTest < Test::Unit::TestCase
       expected = @dummy.avatar.to_file
       @dummy.avatar = "not a file"
       assert @dummy.valid?
-      assert_equal expected.path, @dummy.avatar.to_file.path
+      assert_equal expected.path, @dummy.avatar.path
+      expected.close
 
       @dummy.avatar = @bad_file
       assert ! @dummy.valid?
@@ -234,19 +235,19 @@ class IntegrationTest < Test::Unit::TestCase
     context "that is assigned its file from another Paperclip attachment" do
       setup do
         @dummy2 = Dummy.new
-        @file2  = File.new(File.join(FIXTURES_DIR, "12k.png"))
+        @file2  = File.new(File.join(FIXTURES_DIR, "12k.png"), 'rb')
         assert  @dummy2.avatar = @file2
         @dummy2.save
       end
 
       should "work when assigned a file" do
-        assert_not_equal `identify -format "%wx%h" #{@dummy.avatar.to_file(:original).path}`,
-                         `identify -format "%wx%h" #{@dummy2.avatar.to_file(:original).path}`
+        assert_not_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`,
+                         `identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"`
 
         assert @dummy.avatar = @dummy2.avatar
         @dummy.save
-        assert_equal `identify -format "%wx%h" #{@dummy.avatar.to_file(:original).path}`,
-                     `identify -format "%wx%h" #{@dummy2.avatar.to_file(:original).path}`
+        assert_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`,
+                     `identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"`
       end
 
       should "work when assigned a nil file" do
@@ -265,8 +266,9 @@ class IntegrationTest < Test::Unit::TestCase
   if ENV['S3_TEST_BUCKET']
     def s3_files_for attachment
       [:thumb, :medium, :large, :original].inject({}) do |files, style|
-        data = `curl '#{attachment.url(style)}' 2>/dev/null`.chomp
+        data = `curl "#{attachment.url(style)}" 2>/dev/null`.chomp
         t = Tempfile.new("paperclip-test")
+        t.binmode
         t.write(data)
         t.rewind
         files[style] = t
@@ -275,7 +277,7 @@ def s3_files_for attachment
     end
 
     def s3_headers_for attachment, style
-      `curl --head '#{attachment.url(style)}' 2>/dev/null`.split("\n").inject({}) do |h,head|
+      `curl --head "#{attachment.url(style)}" 2>/dev/null`.split("\n").inject({}) do |h,head|
         split_head = head.chomp.split(/\s*:\s*/, 2)
         h[split_head.first.downcase] = split_head.last unless split_head.empty?
         h
@@ -295,8 +297,8 @@ def s3_headers_for attachment, style
                       :bucket => ENV['S3_TEST_BUCKET'],
                       :path => ":class/:attachment/:id/:style/:basename.:extension"
         @dummy     = Dummy.new
-        @file      = File.new(File.join(FIXTURES_DIR, "5k.png"))
-        @bad_file  = File.new(File.join(FIXTURES_DIR, "bad.png"))
+        @file      = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb')
+        @bad_file  = File.new(File.join(FIXTURES_DIR, "bad.png"), 'rb')
 
         assert @dummy.avatar = @file
         assert @dummy.valid?
@@ -310,7 +312,7 @@ def s3_headers_for attachment, style
          ["300x46", :large],
          ["100x15", :medium],
          ["32x32", :thumb]].each do |geo, style|
-          cmd = %Q[identify -format "%wx%h" #{@files_on_s3[style].path}]
+          cmd = %Q[identify -format "%wx%h" "#{@files_on_s3[style].path}"]
           assert_equal geo, `#{cmd}`.chomp, cmd
         end
 
@@ -320,7 +322,7 @@ def s3_headers_for attachment, style
          ["300x46", :large],
          ["100x15", :medium],
          ["32x32", :thumb]].each do |geo, style|
-          cmd = %Q[identify -format "%wx%h" #{@d2_files[style].path}]
+          cmd = %Q[identify -format "%wx%h" "#{@d2_files[style].path}"]
           assert_equal geo, `#{cmd}`.chomp, cmd
         end
 

test/iostream_test.rb

@@ -11,29 +11,43 @@ class IOStreamTest < Test::Unit::TestCase
 
   context "A file" do
     setup do
-      @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"))
+      @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"), 'rb')
     end
 
     context "that is sent #stream_to" do
 
-      [["/tmp/iostream.string.test", File],
-       [Tempfile.new('iostream.test'), Tempfile]].each do |args|
+      context "and given a String" do
+        setup do
+          assert @result = @file.stream_to("/tmp/iostream.string.test")
+        end
 
-        context "and given a #{args[0].class.to_s}" do
-          setup do
-            assert @result = @file.stream_to(args[0])
-          end
+        should "return a File" do
+          assert @result.is_a?(File)
+        end
 
-          should "return a #{args[1].to_s}" do
-            assert @result.is_a?(args[1])
-          end
+        should "contain the same data as the original file" do
+          @file.rewind; @result.rewind
+          assert_equal @file.read, @result.read
+        end
+      end
 
-          should "contain the same data as the original file" do
-            @file.rewind; @result.rewind
-            assert_equal @file.read, @result.read
-          end
+      context "and given a Tempfile" do
+        setup do
+          tempfile = Tempfile.new('iostream.test')
+          tempfile.binmode
+          assert @result = @file.stream_to(tempfile)
+        end
+
+        should "return a Tempfile" do
+          assert @result.is_a?(Tempfile)
+        end
+
+        should "contain the same data as the original file" do
+          @file.rewind; @result.rewind
+          assert_equal @file.read, @result.read
         end
       end
+
     end
 
     context "that is sent #to_tempfile" do

test/paperclip_test.rb

@@ -35,7 +35,7 @@ class PaperclipTest < Test::Unit::TestCase
   context "An ActiveRecord model with an 'avatar' attachment" do
     setup do
       rebuild_model :path => "tmp/:class/omg/:style.:extension"
-      @file = File.new(File.join(FIXTURES_DIR, "5k.png"))
+      @file = File.new(File.join(FIXTURES_DIR, "5k.png"), 'rb')
     end
 
     should "not error when trying to also create a 'blah' attachment" do
@@ -176,8 +176,8 @@ def self.should_validate validation, options, valid_file, invalid_file
      [:content_type,  {:content_type => "text/plain"}, "text.txt", "5k.png"],
      [:content_type,  {:content_type => %r{image/.*}}, "5k.png",   "text.txt"]].each do |args|
       validation, options, valid_file, invalid_file = args
-      valid_file   &&= File.new(File.join(FIXTURES_DIR, valid_file))
-      invalid_file &&= File.new(File.join(FIXTURES_DIR, invalid_file))
+      valid_file   &&= File.open(File.join(FIXTURES_DIR, valid_file), "rb")
+      invalid_file &&= File.open(File.join(FIXTURES_DIR, invalid_file), "rb")
 
       should_validate validation, options, valid_file, invalid_file
     end

test/storage_test.rb

@@ -77,7 +77,7 @@ class StorageTest < Test::Unit::TestCase
 
     context "when assigned" do
       setup do
-        @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'))
+        @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb')
         @dummy = Dummy.new
         @dummy.avatar = @file
       end
@@ -144,7 +144,7 @@ class StorageTest < Test::Unit::TestCase
 
       context "when assigned" do
         setup do
-          @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'))
+          @file = File.new(File.join(File.dirname(__FILE__), 'fixtures', '5k.png'), 'rb')
           @dummy.avatar = @file
         end