diff --git a/app/controllers/dmsf_controller.rb b/app/controllers/dmsf_controller.rb index 73182275..c1a39b61 100644 --- a/app/controllers/dmsf_controller.rb +++ b/app/controllers/dmsf_controller.rb @@ -378,9 +378,10 @@ class DmsfController < ApplicationController zipped_content = DmsfHelper.temp_dir.join(DmsfHelper.temp_filename('dmsf_email_sent_documents.zip')) File.open(zipped_content, 'wb') do |f| - zip_file = File.open(zip.finish, 'rb') - while (buffer = zip_file.read(8192)) - f.write(buffer) + File.open(zip.finish, 'rb') do |zip_file| + while (buffer = zip_file.read(8192)) + f.write(buffer) + end end end @@ -402,8 +403,8 @@ class DmsfController < ApplicationController :folders => selected_folders, :files => selected_files, :subject => "#{@project.name} #{l(:label_dmsf_file_plural).downcase}", - :from => Setting.plugin_redmine_dmsf['dmsf_documents_email_from'].blank? ? - "#{User.current.name} <#{User.current.mail}>" : Setting.plugin_redmine_dmsf['dmsf_documents_email_from'], + :from => Setting.plugin_redmine_dmsf['dmsf_documents_email_from'].presence || + "#{User.current.name} <#{User.current.mail}>", :reply_to => Setting.plugin_redmine_dmsf['dmsf_documents_email_reply_to'] } render :action => 'email_entries' @@ -434,7 +435,7 @@ class DmsfController < ApplicationController end def zip_entries(zip, selected_folders, selected_files) - member = Member.where(user_id: User.current.id, project_id: @project.id).first + member = Member.find_by(user_id: User.current.id, project_id: @project.id) selected_folders.each do |selected_folder_id| folder = DmsfFolder.visible.find_by(id: selected_folder_id) if folder @@ -684,7 +685,7 @@ class DmsfController < ApplicationController @subfolders = DmsfHelper.visible_folders(@subfolders, @project) end - @ajax_upload_size = Setting.plugin_redmine_dmsf['dmsf_max_ajax_upload_filesize'].present? ? Setting.plugin_redmine_dmsf['dmsf_max_ajax_upload_filesize'] : 100 + @ajax_upload_size = Setting.plugin_redmine_dmsf['dmsf_max_ajax_upload_filesize'].presence || 100 # Trash @trash_visible = @folder_manipulation_allowed && @file_manipulation_allowed && diff --git a/app/controllers/dmsf_files_controller.rb b/app/controllers/dmsf_files_controller.rb index b4785365..689eb0ba 100644 --- a/app/controllers/dmsf_files_controller.rb +++ b/app/controllers/dmsf_files_controller.rb @@ -57,7 +57,7 @@ class DmsfFilesController < ApplicationController access.dmsf_file_revision = @revision access.action = DmsfFileRevisionAccess::DownloadAction access.save! - member = Member.where(user_id: User.current.id, project_id: @file.project.id).first + member = Member.find_by(user_id: User.current.id, project_id: @file.project.id) if member && !member.dmsf_title_format.nil? && !member.dmsf_title_format.empty? title_format = member.dmsf_title_format else diff --git a/app/controllers/dmsf_state_controller.rb b/app/controllers/dmsf_state_controller.rb index e3551e56..9af7e476 100644 --- a/app/controllers/dmsf_state_controller.rb +++ b/app/controllers/dmsf_state_controller.rb @@ -27,7 +27,7 @@ class DmsfStateController < ApplicationController before_action :authorize def user_pref_save - member = @project.members.where(user_id: User.current.id).first + member = @project.members.find_by(user_id: User.current.id) if member member.dmsf_mail_notification = params[:email_notify] member.dmsf_title_format = params[:title_format] @@ -49,7 +49,7 @@ class DmsfStateController < ApplicationController private def format_valid?(format) - format.blank? || ((format =~ /%(t|d|v|i|r)/) && format.length < 256) + format.blank? || ((/%(t|d|v|i|r)/.match?(format)) && format.length < 256) end end \ No newline at end of file diff --git a/app/controllers/dmsf_workflows_controller.rb b/app/controllers/dmsf_workflows_controller.rb index 57001ed3..ad37ab06 100644 --- a/app/controllers/dmsf_workflows_controller.rb +++ b/app/controllers/dmsf_workflows_controller.rb @@ -339,7 +339,7 @@ class DmsfWorkflowsController < ApplicationController end operator = (params[:commit] == l(:dmsf_and)) ? DmsfWorkflowStep::OPERATOR_AND : DmsfWorkflowStep::OPERATOR_OR user_ids = User.where(id: params[:user_ids]).ids - if user_ids.count > 0 + if user_ids.any? user_ids.each do |user_id| ws = DmsfWorkflowStep.new ws.dmsf_workflow_id = @dmsf_workflow.id diff --git a/app/helpers/dmsf_helper.rb b/app/helpers/dmsf_helper.rb index 0f7ec4e3..a3af0d0c 100644 --- a/app/helpers/dmsf_helper.rb +++ b/app/helpers/dmsf_helper.rb @@ -48,11 +48,13 @@ module DmsfHelper # get only the filename, not the whole path just_filename = File.basename(filename.gsub('\\\\', '/')) # replace all non alphanumeric, hyphens or periods with underscore - just_filename = just_filename.gsub(/[^\w\.\-]/,'_') - unless just_filename =~ %r{^[a-zA-Z0-9_\.\-]*$} + just_filename.gsub!(/[^\w\.\-]/, '_') + unless %r{^[a-zA-Z0-9_\.\-]*$}.match?(just_filename) # keep the extension if any - extension = $1 if just_filename =~ %r{(\.[a-zA-Z0-9]+)$} - just_filename = Digest::SHA256.hexdigest(just_filename) << extension + if just_filename =~ %r{(\.[a-zA-Z0-9]+)$} + extension = $1 + just_filename = Digest::SHA256.hexdigest(just_filename) << extension + end end just_filename end diff --git a/app/helpers/dmsf_workflows_helper.rb b/app/helpers/dmsf_workflows_helper.rb index 01509f6c..62ba5fbc 100644 --- a/app/helpers/dmsf_workflows_helper.rb +++ b/app/helpers/dmsf_workflows_helper.rb @@ -51,7 +51,7 @@ module DmsfWorkflowsHelper options = Array.new options << [l(:dmsf_new_step), 0] steps.each do |step| - options << [step.name.present? ? step.name : step.step.to_s, step.step] + options << [step.name.presence || step.step.to_s, step.step] end options_for_select(options, 0) end diff --git a/app/models/dmsf_file.rb b/app/models/dmsf_file.rb index e286bec1..eb0cbee0 100644 --- a/app/models/dmsf_file.rb +++ b/app/models/dmsf_file.rb @@ -391,7 +391,7 @@ class DmsfFile < ActiveRecord::Base next if dmsf_attrs.length == 0 || id_attribute == 0 next unless results.select{|f| f.id.to_s == id_attribute}.empty? - dmsf_file = DmsfFile.visible.where(limit_options).where(id: id_attribute).first + dmsf_file = DmsfFile.visible.where(limit_options).find_by(id: id_attribute) if dmsf_file && DmsfFolder.permissions?(dmsf_file.dmsf_folder) if user.allowed_to?(:view_dmsf_files, dmsf_file.project) && @@ -417,7 +417,7 @@ class DmsfFile < ActiveRecord::Base end def display_name - member = Member.where(user_id: User.current.id, project_id: project_id).first + member = Member.find_by(user_id: User.current.id, project_id: project_id) if member && !member.dmsf_title_format.nil? && !member.dmsf_title_format.empty? title_format = member.dmsf_title_format else diff --git a/app/models/dmsf_folder.rb b/app/models/dmsf_folder.rb index fe4d5a0c..8d217888 100644 --- a/app/models/dmsf_folder.rb +++ b/app/models/dmsf_folder.rb @@ -130,9 +130,9 @@ class DmsfFolder < ActiveRecord::Base def self.find_by_title(project, folder, title) if folder - visible.where(project_id: project.id, dmsf_folder_id: nil, title: title).first + visible.find_by(project_id: project.id, dmsf_folder_id: nil, title: title) else - visible.where(project_id: project.id, dmsf_folder_id: folder.id, title: title).first + visible.find_by(project_id: project.id, dmsf_folder_id: folder.id, title: title) end end diff --git a/app/models/dmsf_mailer.rb b/app/models/dmsf_mailer.rb index cfec0697..33adc322 100644 --- a/app/models/dmsf_mailer.rb +++ b/app/models/dmsf_mailer.rb @@ -52,7 +52,7 @@ class DmsfMailer < Mailer end def files_deleted(user, project, files) - if user && files.count > 0 + if user && files.any? redmine_headers 'Project' => project.identifier if project @files = files @project = project diff --git a/app/validators/dmsf_file_name_validator.rb b/app/validators/dmsf_file_name_validator.rb index 5996dbc7..7e9b0411 100644 --- a/app/validators/dmsf_file_name_validator.rb +++ b/app/validators/dmsf_file_name_validator.rb @@ -22,7 +22,7 @@ class DmsfFileNameValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - unless value =~ /\A[^#{DmsfFolder::INVALID_CHARACTERS}]*\z/ + unless /\A[^#{DmsfFolder::INVALID_CHARACTERS}]*\z/.match?(value) record.errors.add attribute, :error_contains_invalid_character end end diff --git a/app/views/dmsf/_dmsf_rows.erb b/app/views/dmsf/_dmsf_rows.erb index 45540ab1..bdfd2420 100644 --- a/app/views/dmsf/_dmsf_rows.erb +++ b/app/views/dmsf/_dmsf_rows.erb @@ -23,7 +23,7 @@ <% parent = @folder ? @folder : @project %> <% DmsfHelper.all_children_sorted(parent, @pos, @idnt).each do |obj, position| %> <% classes = "dmsf_tree idnt-#{@idnt} hascontextmenu" %> - <% if obj.is_a?(DmsfFolder) && ((obj.dmsf_folders.visible.count > 0) || (obj.dmsf_files.visible.count > 0) || (obj.dmsf_links.visible.count > 0)) %> + <% if obj.is_a?(DmsfFolder) && ((obj.dmsf_folders.visible.any?) || (obj.dmsf_files.visible.any?) || (obj.dmsf_links.visible.any?)) %> <% classes += ' idnt dmsf_collapsed dmsf-not-loaded' %> <% id = "id='#{obj.id}span'".html_safe %> <% onclick = "onclick=\"dmsfToggle('#{obj.id}','#{obj.id}span','#{escape_javascript(expand_folder_dmsf_path)}')\"" %> diff --git a/app/views/settings/_dmsf_settings.html.erb b/app/views/settings/_dmsf_settings.html.erb index 2e5aaa5c..501c8750 100644 --- a/app/views/settings/_dmsf_settings.html.erb +++ b/app/views/settings/_dmsf_settings.html.erb @@ -78,11 +78,11 @@ <% end %> <% testfilename = DmsfFile.storage_path.join('test.test') %> <% if File.exist?(storage_dir) %> - <% begin %> - <% File.open(testfilename, 'wb') %> + <% begin %> + <% File.open(testfilename, 'wb') {} %> <% rescue %>

<%= l(:error_file_can_not_be_created) %>

- <% ensure %> + <% ensure %> <% FileUtils.rm_f(testfilename) %> <% end %> <% end %> @@ -108,12 +108,12 @@ <% path = Pathname.new(tmpdir) %> <% testfilename = path.join('test.test') %> <% if File.exist?(tmpdir) %> - <% begin %> - <% File.open(testfilename, 'wb') %> + <% begin %> + <% File.open(testfilename, 'wb') {} %> <% rescue %>

<%= l(:error_tmpfile_can_not_be_created) %>

- <% ensure %> - <% File.delete(testfilename) if File.exist?(testfilename) %> + <% ensure %> + <% FileUtils.rm_f(testfilename) %> <% end %> <% end %> diff --git a/lib/dav4rack/controller.rb b/lib/dav4rack/controller.rb index f75086dd..da77a260 100644 --- a/lib/dav4rack/controller.rb +++ b/lib/dav4rack/controller.rb @@ -102,12 +102,14 @@ module DAV4Rack # Return response to HEAD def head if(resource.exist?) - response['Etag'] = resource.etag - response['Content-Type'] = resource.content_type - response['Content-Length'] = resource.content_length.to_s - response['Last-Modified'] = resource.last_modified.httpdate - resource.head(request, response) - OK + res = resource.head(request, response) + if(res == OK) + response['Etag'] ||= resource.etag + response['Content-Type'] ||= resource.content_type + response['Content-Length'] ||= resource.content_length.to_s + response['Last-Modified'] ||= resource.last_modified.httpdate + end + res else NotFound end @@ -197,7 +199,8 @@ module DAV4Rack return BadRequest unless request.depth == :infinity return BadRequest unless dest = request.destination - if status = dest.validate + if status = dest.validate(host: request.host, + resource_path: resource.path) return status end diff --git a/lib/dav4rack/destination_header.rb b/lib/dav4rack/destination_header.rb index fe43e7e1..bdaae6ec 100644 --- a/lib/dav4rack/destination_header.rb +++ b/lib/dav4rack/destination_header.rb @@ -5,24 +5,13 @@ module DAV4Rack attr_reader :host, :path, :path_info - def initialize(value, script_name: nil) - @script_name = script_name.to_s - @value = value.to_s.strip - parse - end - - def parse - uri = Addressable::URI.parse @value - + # uri is expected to be a DAV4Rack::Uri instance + def initialize(uri) @host = uri.host - @path = Addressable::URI.unencode uri.path - - if @script_name - if @path =~ /\A(?#{Regexp.escape @script_name}(?\/.*))\z/ - @path_info = $~[:path_info] - else - raise ArgumentError, 'invalid destination header value' - end + @path = uri.path + unless @path_info = uri.path_info + # nil path info means path is outside the realm of script_name + raise ArgumentError, "invalid destination header value: #{uri.to_s}" end end @@ -30,7 +19,7 @@ module DAV4Rack def validate(host: nil, resource_path: nil) if host and self.host and self.host != host DAV4Rack::HTTPStatus::BadGateway - elsif self.path == resource_path + elsif resource_path and self.path_info == resource_path DAV4Rack::HTTPStatus::Forbidden end end diff --git a/lib/dav4rack/request.rb b/lib/dav4rack/request.rb index 7851988b..3a867db7 100644 --- a/lib/dav4rack/request.rb +++ b/lib/dav4rack/request.rb @@ -3,6 +3,7 @@ require 'uri' require 'addressable/uri' require 'dav4rack/logger' +require 'dav4rack/uri' module DAV4Rack class Request < Rack::Request @@ -84,7 +85,7 @@ module DAV4Rack # Destination header def destination @destination ||= if h = get_header('HTTP_DESTINATION') - DestinationHeader.new h, script_name: script_name + DestinationHeader.new DAV4Rack::Uri.new(h, script_name: script_name) end end @@ -123,6 +124,13 @@ module DAV4Rack "#{script_name}#{expand_path path}" end + # returns the given path, but with the leading script_name removed. Will + # return nil if the path does not begin with the script_name + def path_info_for(full_path, script_name: self.script_name) + uri = DAV4Rack::Uri.new full_path, script_name: script_name + return uri.path_info + end + # expands '/foo/../bar' to '/bar' def expand_path(path) path.squeeze! '/' diff --git a/lib/dav4rack/resource.rb b/lib/dav4rack/resource.rb index 2981e852..6eb62d3e 100644 --- a/lib/dav4rack/resource.rb +++ b/lib/dav4rack/resource.rb @@ -208,8 +208,12 @@ module DAV4Rack NotImplemented end + # HTTP HEAD request. + # + # Like GET, but without content. Override if you set custom headers in GET + # to set them here as well. def head(request, response) - #no-op, but called by the controller + OK end # HTTP PUT request. diff --git a/lib/dav4rack/uri.rb b/lib/dav4rack/uri.rb new file mode 100644 index 00000000..20a5dbcf --- /dev/null +++ b/lib/dav4rack/uri.rb @@ -0,0 +1,42 @@ +require 'addressable/uri' + +module DAV4Rack + + # adds a bit of parsing logic around a header URI or path value + class Uri + + attr_reader :host, :path, :path_info, :script_name + + def initialize(uri_or_path, script_name: nil) + # more than one leading slash confuses Addressable::URI, resulting e.g. + # with //remote.php/dav/files in a path of /dav/files with a host + # remote.php. + @uri_or_path = uri_or_path.to_s.strip.sub %r{\A/+}, '/' + + @script_name = script_name + parse + end + + def to_s + @uri_or_path + end + + private + + def parse + uri = Addressable::URI.parse @uri_or_path + + @host = uri.host + @path = Addressable::URI.unencode uri.path + + if @script_name + if @path =~ /\A(?#{Regexp.escape @script_name}(?\/.*))\z/ + @path_info = $~[:path_info] + end + else + @path_info = @path + end + end + + end +end diff --git a/lib/dav4rack/version.rb b/lib/dav4rack/version.rb index 221a3e9a..d386f6c5 100644 --- a/lib/dav4rack/version.rb +++ b/lib/dav4rack/version.rb @@ -13,5 +13,5 @@ module DAV4Rack end end - VERSION = Version.new('1.0.0') + VERSION = Version.new('1.1.0') end diff --git a/test/unit/dmsf_workflow_step_test.rb b/test/unit/dmsf_workflow_step_test.rb index 77b2b55a..cd0b5e36 100644 --- a/test/unit/dmsf_workflow_step_test.rb +++ b/test/unit/dmsf_workflow_step_test.rb @@ -72,25 +72,25 @@ class DmsfWorkflowStepTest < RedmineDmsf::Test::UnitTest def test_validate_workflow_id_presence @wfs1.dmsf_workflow_id = nil assert !@wfs1.save - assert@wfs1.errors.count > 0 + assert @wfs1.errors.any? end def test_validate_step_presence @wfs1.step = nil assert !@wfs1.save - assert @wfs1.errors.count > 0 + assert @wfs1.errors.any? end def test_validate_user_id_presence @wfs1.user_id = nil assert !@wfs1.save - assert@wfs1.errors.count > 0 + assert @wfs1.errors.any? end def test_validate_operator_presence @wfs1.operator = nil assert !@wfs1.save - assert @wfs1.errors.count > 0 + assert @wfs1.errors.any? end def test_validate_user_id_uniqueness @@ -98,7 +98,7 @@ class DmsfWorkflowStepTest < RedmineDmsf::Test::UnitTest @wfs2.dmsf_workflow_id = @wfs1.dmsf_workflow_id @wfs2.step = @wfs1.step assert !@wfs2.save - assert @wfs2.errors.count > 0 + assert @wfs2.errors.any? end def test_validate_name_length