diff --git a/Gemfile b/Gemfile index e9b751e79..9f45d1679 100644 --- a/Gemfile +++ b/Gemfile @@ -17,6 +17,7 @@ gem "nokogiri", "~> 1.10.0" gem 'i18n', '~> 1.8.2' gem "rbpdf", "~> 1.20.0" gem 'addressable' +gem 'rubyzip', '~> 2.2.0' # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :x64_mingw, :mswin] diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index 4a880793e..dbebd5a34 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -19,6 +19,8 @@ class AttachmentsController < ApplicationController before_action :find_attachment, :only => [:show, :download, :thumbnail, :update, :destroy] + before_action :find_container, :only => [:edit_all, :update_all, :download_all] + before_action :find_downloadable_attachments, :only => :download_all before_action :find_editable_attachments, :only => [:edit_all, :update_all] before_action :file_readable, :read_authorize, :only => [:show, :download, :thumbnail] before_action :update_authorize, :only => :update @@ -132,6 +134,18 @@ class AttachmentsController < ApplicationController render :action => 'edit_all' end + def download_all + Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| + zip_file = Attachment.attachments_to_zip(tempfile, @attachments) + if zip_file.nil? + render_404 + return + end + send_data(File.read(zip_file.path), :type => 'application/zip', + :filename => "#{@container.class.to_s.downcase}-#{@container.id}-attachments.zip") + end + end + def update @attachment.safe_attributes = params[:attachment] saved = @attachment.save @@ -195,6 +209,11 @@ class AttachmentsController < ApplicationController end def find_editable_attachments + @attachments = @container.attachments.select(&:editable?) + render_404 if @attachments.empty? + end + + def find_container klass = params[:object_type].to_s.singularize.classify.constantize rescue nil unless klass && klass.reflect_on_association(:attachments) render_404 @@ -206,15 +225,24 @@ class AttachmentsController < ApplicationController render_403 return end - @attachments = @container.attachments.select(&:editable?) if @container.respond_to?(:project) @project = @container.project end - render_404 if @attachments.empty? rescue ActiveRecord::RecordNotFound render_404 end + def find_downloadable_attachments + @attachments = @container.attachments.select{|a| File.readable?(a.diskfile) } + bulk_download_max_size = Redmine::Configuration['bulk_download_max_size'].to_i.kilobytes + if @attachments.sum(&:filesize) > bulk_download_max_size + flash[:error] = l(:error_bulk_download_size_too_big, + :max_size => bulk_download_max_size) + redirect_to back_url + return + end + end + # Checks that the file exists and is readable def file_readable if @attachment.readable? diff --git a/app/helpers/attachments_helper.rb b/app/helpers/attachments_helper.rb index d03997c4b..700ec7fae 100644 --- a/app/helpers/attachments_helper.rb +++ b/app/helpers/attachments_helper.rb @@ -27,6 +27,10 @@ module AttachmentsHelper object_attachments_path container.class.name.underscore.pluralize, container.id end + def container_attachments_download_path(container) + object_attachments_download_path container.class.name.underscore.pluralize, container.id + end + # Displays view/delete links to the attachments of the given object # Options: # :author -- author names are not displayed if set to false diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 627c1a181..b64bda9a5 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -19,6 +19,7 @@ require "digest" require "fileutils" +require "zip" class Attachment < ActiveRecord::Base include Redmine::SafeAttributes @@ -345,6 +346,25 @@ class Attachment < ActiveRecord::Base Attachment.where("created_on < ? AND (container_type IS NULL OR container_type = '')", Time.now - age).destroy_all end + def self.attachments_to_zip(tmpfile, attachments) + attachments = attachments.select{|attachment| File.readable?(attachment.diskfile) } + return nil if attachments.blank? + Zip.unicode_names = true + existing_file_names = [] + Zip::File.open(tmpfile.path, Zip::File::CREATE) do |zip| + attachments.each do |attachment| + filename = attachment.filename + existing_file_names << filename + # If a file with the same name already exists, change the file name. + if existing_file_names.count(filename) > 1 + filename = "#{File.basename(filename, ".*")}(#{existing_file_names.count(filename)})#{File.extname(filename)}" + end + zip.add(filename, attachment.diskfile) + end + end + tmpfile + end + # Moves an existing attachment to its target directory def move_to_target_directory! return unless !new_record? & readable? diff --git a/app/views/attachments/_links.html.erb b/app/views/attachments/_links.html.erb index 0a9f5e3eb..d4cad6d05 100644 --- a/app/views/attachments/_links.html.erb +++ b/app/views/attachments/_links.html.erb @@ -42,4 +42,13 @@ <% end %> <% end %> +<% if attachments.size > 1 %> +
+ <%= link_to(l(:label_download_all_attachments), + container_attachments_download_path(container), + :title => l(:label_download_all_attachments), + :class => 'icon icon-download' + ) %> +
+<% end %> diff --git a/config/configuration.yml.example b/config/configuration.yml.example index a8b6be83c..54c3ab490 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -219,6 +219,9 @@ default: #avatar_server_url: https://www.gravatar.com # default #avatar_server_url: https://seccdn.libravatar.org + # Upper limit of the total file size when bulk downloading attached files (KB) + # bulk_download_max_size: 512000 + # specific configuration options for production environment # that overrides the default ones production: diff --git a/config/locales/en.yml b/config/locales/en.yml index 3f0fd2d77..046d57238 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -211,6 +211,7 @@ en: error_unable_delete_issue_status: 'Unable to delete issue status (%{value})' error_unable_to_connect: "Unable to connect (%{value})" error_attachment_too_big: "This file cannot be uploaded because it exceeds the maximum allowed file size (%{max_size})" + error_bulk_download_size_too_big: "These attachments cannot be bulk downloaded because the total file size exceeds the maximum allowed size (%{max_size})" error_session_expired: "Your session has expired. Please login again." error_token_expired: "This password recovery link has expired, please try again." warning_attachments_not_saved: "%{count} file(s) could not be saved." @@ -1015,6 +1016,7 @@ en: label_users_visibility_all: All active users label_users_visibility_members_of_visible_projects: Members of visible projects label_edit_attachments: Edit attached files + label_download_all_attachments: Download all attached files label_link_copied_issue: Link copied issue label_ask: Ask label_search_attachments_yes: Search attachment filenames and descriptions diff --git a/config/locales/ja.yml b/config/locales/ja.yml index dcd4598c4..0d74e9ff7 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -1016,6 +1016,7 @@ ja: button_export: エクスポート label_export_options: "%{export_format} エクスポート設定" error_attachment_too_big: このファイルはアップロードできません。添付ファイルサイズの上限(%{max_size})を超えています。 + error_bulk_download_size_too_big: これらの添付ファイルをダウンロードできません。一括ダウンロードサイズの上限(%{max_size})を超えています。 notice_failed_to_save_time_entries: "全%{total}件中%{count}件の作業時間が保存できませんでした: %{ids}。" label_x_issues: zero: 0 チケット diff --git a/config/routes.rb b/config/routes.rb index d8e5fd710..1ca659da1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -289,6 +289,7 @@ Rails.application.routes.draw do resources :attachments, :only => [:show, :update, :destroy] get 'attachments/:object_type/:object_id/edit', :to => 'attachments#edit_all', :as => :object_attachments_edit patch 'attachments/:object_type/:object_id', :to => 'attachments#update_all', :as => :object_attachments + get 'attachments/:object_type/:object_id/download', :to => 'attachments#download_all', :as => :object_attachments_download resources :groups do resources :memberships, :controller => 'principal_memberships' diff --git a/lib/redmine/configuration.rb b/lib/redmine/configuration.rb index 1dfe8ddfc..ccbdd386a 100644 --- a/lib/redmine/configuration.rb +++ b/lib/redmine/configuration.rb @@ -24,7 +24,8 @@ module Redmine @defaults = { 'avatar_server_url' => 'https://www.gravatar.com', 'email_delivery' => nil, - 'max_concurrent_ajax_uploads' => 2 + 'max_concurrent_ajax_uploads' => 2, + 'bulk_download_max_size' => 512000 } @config = nil diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 815bd4e15..be03de53f 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -899,6 +899,8 @@ div.attachments p { margin:4px 0 2px 0; } div.attachments img { vertical-align: middle; } div.attachments span.author { font-size: 0.9em; color: #888; } +div.bulk-download { margin-top: 1em; margin-left: 0.3em; margin-bottom: 0.4em;} + div.thumbnails {margin:0.6em;} div.thumbnails div {background:#fff;border:2px solid #ddd;display:inline-block;margin-right:2px;} div.thumbnails img {margin: 3px; vertical-align: middle;} diff --git a/test/functional/attachments_controller_test.rb b/test/functional/attachments_controller_test.rb index bf889892d..235ec2c77 100644 --- a/test/functional/attachments_controller_test.rb +++ b/test/functional/attachments_controller_test.rb @@ -577,6 +577,50 @@ class AttachmentsControllerTest < Redmine::ControllerTest assert_equal 'This is a Ruby source file', attachment.description end + def test_download_all_with_valid_container + @request.session[:user_id] = 2 + get :download_all, :params => { + :object_type => 'issues', + :object_id => '2' + } + assert_response 200 + assert_equal response.headers['Content-Type'], 'application/zip' + assert_match (/issue-2-attachments.zip/), response.headers['Content-Disposition'] + assert_not_includes Dir.entries(Rails.root.join('tmp')), /attachments_zip/ + end + + def test_download_all_with_invalid_container + @request.session[:user_id] = 2 + get :download_all, :params => { + :object_type => 'issues', + :object_id => '999' + } + assert_response 404 + end + + def test_download_all_without_readable_attachments + @request.session[:user_id] = 2 + get :download_all, :params => { + :object_type => 'issues', + :object_id => '1' + } + assert_equal Issue.find(1).attachments, [] + assert_response 404 + end + + def test_download_all_with_maximum_bulk_download_size_larger_than_attachments + Redmine::Configuration.with 'bulk_download_max_size' => 0 do + @request.session[:user_id] = 2 + get :download_all, :params => { + :object_type => 'issues', + :object_id => '2', + :back_url => '/issues/2' + } + assert_redirected_to '/issues/2' + assert_equal flash[:error], 'These attachments cannot be bulk downloaded because the total file size exceeds the maximum allowed size (0)' + end + end + def test_destroy_issue_attachment set_tmp_attachments_directory issue = Issue.find(3) diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb index 831278634..932266a9d 100644 --- a/test/unit/attachment_test.rb +++ b/test/unit/attachment_test.rb @@ -278,6 +278,36 @@ class AttachmentTest < ActiveSupport::TestCase end end + def test_attachments_to_zip_with_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.attachments_to_zip(tempfile, [attachment]) + assert_instance_of File, zip_file + end + end + + def test_attachments_to_zip_without_attachments + Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| + zip_file = Attachment.attachments_to_zip(tempfile, []) + assert_nil zip_file + end + end + + def test_attachments_to_zip_should_not_duplicate_file_names + attachment_1 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1) + attachment_2 = Attachment.create!(:file => uploaded_test_file("testfile.txt", ""), :author_id => 1) + Tempfile.create('attachments_zip', Rails.root.join('tmp')) do |tempfile| + zip_file = Attachment.attachments_to_zip(tempfile, [attachment_1, attachment_2]) + zip_file_names = ['testfile.txt', 'testfile(2).txt'] + + Zip::File.open(zip_file.path) do |z| + z.each_with_index do |entry, i| + assert_includes zip_file_names[i], entry.name + end + end + end + end + def test_move_from_root_to_target_directory_should_move_root_files a = Attachment.find(20) assert a.disk_directory.blank?