diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index b69dcf983..0c76490b2 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -137,16 +137,16 @@ class AttachmentsController < ApplicationController 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 + zip_data = Attachment.archive_attachments(@attachments) + if zip_data + file_name = "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip" + send_data( + zip_data, + :type => Redmine::MimeType.of(file_name), + :filename => file_name + ) + else + render_404 end end diff --git a/app/models/attachment.rb b/app/models/attachment.rb index b5a3332ac..734b8f7fa 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -346,28 +346,28 @@ class Attachment < ActiveRecord::Base Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all end - def self.archive_attachments(out_file, attachments) + def self.archive_attachments(attachments) attachments = attachments.select(&:readable?) return nil if attachments.blank? Zip.unicode_names = true archived_file_names = [] - Zip::File.open(out_file.path, Zip::File::CREATE) do |zip| + Zip::OutputStream.write_buffer do |zos| attachments.each do |attachment| filename = attachment.filename # rename the file if a file with the same name already exists dup_count = 0 while archived_file_names.include?(filename) dup_count += 1 - basename = File.basename(attachment.filename, '.*') extname = File.extname(attachment.filename) + basename = File.basename(attachment.filename, extname) filename = "#{basename}(#{dup_count})#{extname}" end - zip.add(filename, attachment.diskfile) + zos.put_next_entry(filename) + zos << IO.binread(attachment.diskfile) archived_file_names << filename end - end - out_file + end.string end # Moves an existing attachment to its target directory diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb index 209775514..5adfc5116 100644 --- a/test/unit/attachment_test.rb +++ b/test/unit/attachment_test.rb @@ -280,28 +280,32 @@ class AttachmentTest < ActiveSupport::TestCase 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 + zip_data = Attachment.archive_attachments([attachment]) + file_names = [] + Zip::InputStream.open(StringIO.new(zip_data)) do |io| + while (entry = io.get_next_entry) + file_names << entry.name + end end + assert_equal ['testfile.txt'], file_names end def test_archive_attachments_without_attachments - Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| - zip_file = Attachment.archive_attachments(tempfile, []) - assert_nil zip_file - end + zip_data = Attachment.archive_attachments([]) + assert_nil zip_data end 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| - assert_equal ['testfile.txt', 'testfile(1).txt'], z.map(&:name) + zip_data = Attachment.archive_attachments([attachment1, attachment2]) + file_names = [] + Zip::InputStream.open(StringIO.new(zip_data)) do |io| + while (entry = io.get_next_entry) + file_names << entry.name end end + assert_equal ['testfile.txt', 'testfile(1).txt'], file_names end def test_move_from_root_to_target_directory_should_move_root_files