Index: app/controllers/attachments_controller.rb =================================================================== --- app/controllers/attachments_controller.rb (revision 19615) +++ app/controllers/attachments_controller.rb (working copy) @@ -137,17 +137,18 @@ end def download_all - Tempfile.create('attachments_zip-', Rails.root.join('tmp')) do |tempfile| - zip_file = Attachment.archive_attachments(tempfile, @attachments) - if zip_file - send_data( - File.read(zip_file.path), - :type => 'application/zip', - :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip") - else - render_404 - end + tempfile_path = Rails.root.join('tmp', "attachments_zip-#{SecureRandom.hex(8)}.zip") + zip_file = Attachment.archive_attachments(tempfile_path, @attachments) + if zip_file + send_data( + File.read(zip_file), + :type => 'application/zip', + :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip") + else + render_404 end + ensure + FileUtils.rm_rf(tempfile_path) end def update Index: app/models/attachment.rb =================================================================== --- app/models/attachment.rb (revision 19615) +++ app/models/attachment.rb (working copy) @@ -352,7 +352,7 @@ Zip.unicode_names = true archived_file_names = [] - Zip::File.open(out_file.path, Zip::File::CREATE) do |zip| + Zip::File.open(out_file, Zip::File::CREATE) do |zip| attachments.each do |attachment| filename = attachment.filename # rename the file if a file with the same name already exists Index: test/unit/attachment_test.rb =================================================================== --- test/unit/attachment_test.rb (revision 19615) +++ test/unit/attachment_test.rb (working copy) @@ -280,15 +280,15 @@ def test_archive_attachments attachment = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1) - Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| - zip_file = Attachment.archive_attachments(tempfile, [attachment]) - assert_instance_of File, zip_file + with_tempfile_path do |tempfile_path| + zip_file = Attachment.archive_attachments(tempfile_path, [attachment]) + assert_instance_of Pathname, zip_file end end def test_archive_attachments_without_attachments - Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| - zip_file = Attachment.archive_attachments(tempfile, []) + with_tempfile_path do |tempfile_path| + zip_file = Attachment.archive_attachments(tempfile_path, []) assert_nil zip_file end end @@ -296,9 +296,9 @@ def test_archive_attachments_should_rename_duplicate_file_names attachment1 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1) attachment2 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1) - Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| - zip_file = Attachment.archive_attachments(tempfile, [attachment1, attachment2]) - Zip::File.open(zip_file.path) do |z| + with_tempfile_path do |tempfile_path| + zip_file = Attachment.archive_attachments(tempfile_path, [attachment1, attachment2]) + Zip::File.open(zip_file) do |z| assert_equal ['testfile.txt', 'testfile(1).txt'], z.map(&:name) end end @@ -571,4 +571,13 @@ assert_equal expected, attachment.is_text?, attachment.inspect end end + + private + + def with_tempfile_path + tempfile_path = Rails.root.join('tmp', "attachments_zip-#{SecureRandom.hex(8)}.zip") + yield(tempfile_path) + ensure + FileUtils.rm_rf(tempfile_path) + end end