From 0ddc62b0d88b97202222579e0be56c5c2aa33046 Mon Sep 17 00:00:00 2001 From: "karel.picman@lbcfree.net" Date: Tue, 8 Sep 2020 15:50:01 +0200 Subject: [PATCH] max size of upload-able file #1170 --- app/helpers/dmsf_upload_helper.rb | 19 +++++---- app/models/dmsf_file_revision.rb | 3 +- .../dmsf_max_file_size_validator.rb | 36 ++++++++++++++++ lib/redmine_dmsf.rb | 1 + lib/redmine_dmsf/webdav/dmsf_resource.rb | 11 +++-- .../rest_api/dmsf_file_api_test.rb | 41 +++++++++++++++++++ .../webdav/dmsf_webdav_put_test.rb | 10 ++++- test/unit/dmsf_file_revision_test.rb | 6 +++ 8 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 app/validators/dmsf_max_file_size_validator.rb diff --git a/app/helpers/dmsf_upload_helper.rb b/app/helpers/dmsf_upload_helper.rb index aaf48f35..7c16ef78 100644 --- a/app/helpers/dmsf_upload_helper.rb +++ b/app/helpers/dmsf_upload_helper.rb @@ -57,7 +57,7 @@ module DmsfUploadHelper end if file.locked_for_user? - failed_uploads.push(commited_file) + failed_uploads.push file next end @@ -92,13 +92,18 @@ module DmsfUploadHelper # Need to save file first to generate id for it in case of creation. # File id is needed to properly generate revision disk filename - if new_revision.valid? && file.save - new_revision.disk_filename = new_revision.new_storage_filename - else - Rails.logger.error (new_revision.errors.full_messages + file.errors.full_messages).to_sentence - failed_uploads.push commited_file + unless new_revision.valid? + Rails.logger.error new_revision.errors.full_messages.to_sentence + failed_uploads.push new_revision next end + unless file.save + Rails.logger.error file.errors.full_messages.to_sentence + failed_uploads.push file + next + end + + new_revision.disk_filename = new_revision.new_storage_filename if new_revision.save new_revision.assign_workflow commited_file[:dmsf_workflow_id] @@ -117,7 +122,7 @@ module DmsfUploadHelper failed_uploads.push file end else - failed_uploads.push commited_file + failed_uploads.push new_revision end # Approval workflow if commited_file[:workflow_id].present? diff --git a/app/models/dmsf_file_revision.rb b/app/models/dmsf_file_revision.rb index 038a7e89..c650f5cb 100644 --- a/app/models/dmsf_file_revision.rb +++ b/app/models/dmsf_file_revision.rb @@ -75,6 +75,7 @@ class DmsfFileRevision < ActiveRecord::Base validates :dmsf_file, presence: true validates :name, dmsf_file_name: true validates :description, length: { maximum: 1.kilobyte } + validates :size, dmsf_max_file_size: true def project dmsf_file.project if dmsf_file @@ -176,7 +177,7 @@ class DmsfFileRevision < ActiveRecord::Base end def new_storage_filename - raise DmsfAccessError, 'File id is not set' unless dmsf_file.id + raise DmsfAccessError, 'File id is not set' unless dmsf_file&.id filename = DmsfHelper.sanitize_filename(name) timestamp = DateTime.current.strftime('%y%m%d%H%M%S') while File.exist?(storage_base_path.join("#{timestamp}_#{dmsf_file.id}_#{filename}")) diff --git a/app/validators/dmsf_max_file_size_validator.rb b/app/validators/dmsf_max_file_size_validator.rb new file mode 100644 index 00000000..44cf505f --- /dev/null +++ b/app/validators/dmsf_max_file_size_validator.rb @@ -0,0 +1,36 @@ +# encoding: utf-8 +# frozen_string_literal: true +# +# Redmine plugin for Document Management System "Features" +# +# Copyright © 2011 Vít Jonáš +# Copyright © 2011-20 Karel Pičman +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +class DmsfMaxFileSizeValidator < ActiveModel::EachValidator + + include Redmine::I18n + + def validate_each(record, attribute, value) + if value && (value > Setting.attachment_max_size.to_i.kilobytes) + record.errors.add attribute, l(:error_attachment_too_big, + max_size: ActiveSupport::NumberHelper.number_to_human_size(Setting.attachment_max_size.to_i.kilobytes)) + return false + end + true + end + +end \ No newline at end of file diff --git a/lib/redmine_dmsf.rb b/lib/redmine_dmsf.rb index 4c05e632..0a002505 100644 --- a/lib/redmine_dmsf.rb +++ b/lib/redmine_dmsf.rb @@ -27,6 +27,7 @@ DMSF_MAX_NOTIFICATION_RECEIVERS_INFO = 10 # Validators require_dependency File.dirname(__FILE__) + '/../app/validators/dmsf_file_name_validator' +require_dependency File.dirname(__FILE__) + '/../app/validators/dmsf_max_file_size_validator' require_dependency File.dirname(__FILE__) + '/../app/validators/dmsf_workflow_name_validator' require_dependency File.dirname(__FILE__) + '/../app/validators/dmsf_url_validator' require_dependency File.dirname(__FILE__) + '/../app/validators/dmsf_folder_parent_validator' diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index a5d50afd..32c5df56 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -633,13 +633,18 @@ module RedmineDmsf # Ignore 1b files sent for authentication if Setting.plugin_redmine_dmsf['dmsf_webdav_ignore_1b_file_for_authentication'].present? && (new_revision.size == 1) - Rails.logger.info "1b file '#{basename}' sent for authentication ignored" + Rails.logger.warn "1b file '#{basename}' sent for authentication ignored" return NoContent end - if new_revision.valid? && (!f.save) + unless new_revision.valid? + Rails.logger.error new_revision.errors.full_messages.to_sentence + raise UnprocessableEntity + end + + unless f.save Rails.logger.error f.errors.full_messages.to_sentence - raise InternalServerError + raise UnprocessableEntity end new_revision.disk_filename = new_revision.new_storage_filename unless reuse_revision diff --git a/test/integration/rest_api/dmsf_file_api_test.rb b/test/integration/rest_api/dmsf_file_api_test.rb index 943c4791..09e01e2c 100644 --- a/test/integration/rest_api/dmsf_file_api_test.rb +++ b/test/integration/rest_api/dmsf_file_api_test.rb @@ -145,6 +145,47 @@ class DmsfFileApiTest < RedmineDmsf::Test::IntegrationTest assert revision && revision.size > 0 end + def test_upload_document_exceeded_attachment_max_size + Setting.attachment_max_size = '1' + #curl --data-binary "@cat.gif" -H "Content-Type: application/octet-stream" -X POST -u ${1}:${2} http://localhost:3000/projects/12/dmsf/upload.xml?filename=cat.gif + file_content = 'x' * 2.kilobytes + post "/projects/#{@project1.id}/dmsf/upload.xml?filename=test.txt&key=#{@token.value}", params: file_content, + headers: { "CONTENT_TYPE" => 'application/octet-stream' } + assert_response :created + assert_equal 'application/xml', response.content_type + # + # + # 2.8bb2564936980e92ceec8a5759ec34a8 + # + xml = Hash.from_xml(response.body) + assert_kind_of Hash, xml['upload'] + ftoken = xml['upload']['token'] + assert_not_nil ftoken + #curl -v -H "Content-Type: application/xml" -X POST --data "@file.xml" -u ${1}:${2} http://localhost:3000/projects/12/dmsf/commit.xml + payload = %{ + + + + test.txt + test.txt + REST API + From API + + #{ftoken} + + } + assert_difference 'DmsfFileRevision.count', +0 do + post "/projects/#{@project1.id}/dmsf/commit.xml?key=#{@token.value}", params: payload, headers: { 'CONTENT_TYPE' => 'application/xml' } + end + # + # + # Size This file cannot be uploaded because it exceeds the maximum allowed file size (1 KB) + # + assert_select 'error', text: 'Size ' + l(:error_attachment_too_big, + max_size: ActiveSupport::NumberHelper.number_to_human_size(Setting.attachment_max_size.to_i.kilobytes)) + assert_response :unprocessable_entity + end + def test_delete_file # curl -v -H "Content-Type: application/xml" -X DELETE -u ${1}:${2} http://localhost:3000/dmsf/files/196118.xml delete "/dmsf/files/#{@file1.id}.xml?key=#{@token.value}", headers: { 'CONTENT_TYPE' => 'application/xml' } diff --git a/test/integration/webdav/dmsf_webdav_put_test.rb b/test/integration/webdav/dmsf_webdav_put_test.rb index dba30ffb..3a7e977e 100644 --- a/test/integration/webdav/dmsf_webdav_put_test.rb +++ b/test/integration/webdav/dmsf_webdav_put_test.rb @@ -103,7 +103,7 @@ class DmsfWebdavPutTest < RedmineDmsf::Test::IntegrationTest def test_put_succeeds_for_non_admin_with_correct_permissions put "/dmsf/webdav/#{@project1.identifier}/test-1234.txt", params: '1234', headers: @jsmith.merge!({ content_type: :text }) - assert_response :created # Now we have permissions + assert_response :created # Lets check for our file file = DmsfFile.find_file_by_name @project1, nil, 'test-1234.txt' assert file, 'File test-1234 was not found in projects dmsf folder.' @@ -311,5 +311,13 @@ class DmsfWebdavPutTest < RedmineDmsf::Test::IntegrationTest headers: @jsmith.merge!({ content_type: :text }) assert_response :created end + + def test_files_exceeded_max_attachment_size + Setting.attachment_max_size = '1' + file_content = 'x' * 2.kilobytes + put "/dmsf/webdav/#{@project1.identifier}/2kbfile.txt", params: file_content, + headers: @jsmith.merge!({ content_type: :text }) + assert_response :unprocessable_entity + end end \ No newline at end of file diff --git a/test/unit/dmsf_file_revision_test.rb b/test/unit/dmsf_file_revision_test.rb index 8f4b27be..ecd5b194 100644 --- a/test/unit/dmsf_file_revision_test.rb +++ b/test/unit/dmsf_file_revision_test.rb @@ -234,4 +234,10 @@ class DmsfFileRevisionTest < RedmineDmsf::Test::UnitTest assert @revision1.errors.full_messages.to_sentence.include?('Major version cannot be blank') end + def test_size_validation + Setting.attachment_max_size = '1' + @revision1.size = 2.kilobytes + assert !@revision1.valid? + end + end \ No newline at end of file