max size of upload-able file #1170

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
+        unless new_revision.valid?
-          new_revision.disk_filename = new_revision.new_storage_filename
+          Rails.logger.error new_revision.errors.full_messages.to_sentence
-        else
+          failed_uploads.push new_revision
-          Rails.logger.error (new_revision.errors.full_messages + file.errors.full_messages).to_sentence
-          failed_uploads.push commited_file
           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?

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}"))

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áš <vit.jonas@gmail.com>
+# Copyright © 2011-20 Karel Pičman <karel.picman@kontron.com>
+#
+# 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

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'

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

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
+    #<?xml version="1.0" encoding="UTF-8"?>
+    # <upload>
+    #   <token>2.8bb2564936980e92ceec8a5759ec34a8</token>
+    # </upload>
+    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 = %{<?xml version="1.0" encoding="utf-8" ?>
+                <attachments>
+                 <folder_id/>
+                 <uploaded_file>
+                   <name>test.txt</name>
+                   <title>test.txt</title>
+                   <description>REST API</description>
+                   <comment>From API</comment>
+                   <version/>
+                   <token>#{ftoken}</token>
+                 </uploaded_file>
+                </attachments>}
+    assert_difference 'DmsfFileRevision.count', +0 do
+      post "/projects/#{@project1.id}/dmsf/commit.xml?key=#{@token.value}", params: payload, headers: { 'CONTENT_TYPE' => 'application/xml' }
+    end
+    #<?xml version=\"1.0\" encoding=\"UTF-8\"?>
+    # <errors type=\"array\">
+    #   <error>Size This file cannot be uploaded because it exceeds the maximum allowed file size (1 KB)</error>
+    # </errors>
+    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' }

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

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