From d170133bded904603833f1b9e2727def3805962d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 20 Apr 2017 00:37:44 +0000 Subject: Refactor changing files in web UI --- lib/api/commits.rb | 4 ++-- lib/api/files.rb | 4 ++-- lib/api/v3/commits.rb | 4 ++-- lib/api/v3/files.rb | 4 ++-- lib/gitlab/git/index.rb | 49 ++++++++++++++++++++++++++++++++++--------------- lib/gitlab/regex.rb | 16 ---------------- 6 files changed, 42 insertions(+), 39 deletions(-) (limited to 'lib') diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 66b37fd2bcc..621b9dcecd9 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -62,7 +62,7 @@ module API post ":id/repository/commits" do authorize! :push_code, user_project - attrs = declared_params.merge(start_branch: declared_params[:branch], target_branch: declared_params[:branch]) + attrs = declared_params.merge(start_branch: declared_params[:branch], branch_name: declared_params[:branch]) result = ::Files::MultiService.new(user_project, current_user, attrs).execute @@ -140,7 +140,7 @@ module API commit_params = { commit: commit, start_branch: params[:branch], - target_branch: params[:branch] + branch_name: params[:branch] } result = ::Commits::CherryPickService.new(user_project, current_user, commit_params).execute diff --git a/lib/api/files.rb b/lib/api/files.rb index 33fc970dc09..e6ea12c5ab7 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -5,7 +5,7 @@ module API { file_path: attrs[:file_path], start_branch: attrs[:branch], - target_branch: attrs[:branch], + branch_name: attrs[:branch], commit_message: attrs[:commit_message], file_content: attrs[:content], file_content_encoding: attrs[:encoding], @@ -130,7 +130,7 @@ module API authorize! :push_code, user_project file_params = declared_params(include_missing: false) - result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute + result = ::Files::DeleteService.new(user_project, current_user, commit_params(file_params)).execute if result[:status] != :success render_api_error!(result[:message], 400) diff --git a/lib/api/v3/commits.rb b/lib/api/v3/commits.rb index 3414a2883e5..674de592f0a 100644 --- a/lib/api/v3/commits.rb +++ b/lib/api/v3/commits.rb @@ -53,7 +53,7 @@ module API attrs = declared_params.dup branch = attrs.delete(:branch_name) - attrs.merge!(branch: branch, start_branch: branch, target_branch: branch) + attrs.merge!(start_branch: branch, branch_name: branch) result = ::Files::MultiService.new(user_project, current_user, attrs).execute @@ -131,7 +131,7 @@ module API commit_params = { commit: commit, start_branch: params[:branch], - target_branch: params[:branch] + branch_name: params[:branch] } result = ::Commits::CherryPickService.new(user_project, current_user, commit_params).execute diff --git a/lib/api/v3/files.rb b/lib/api/v3/files.rb index 13542b0c71c..c76acc86504 100644 --- a/lib/api/v3/files.rb +++ b/lib/api/v3/files.rb @@ -6,7 +6,7 @@ module API { file_path: attrs[:file_path], start_branch: attrs[:branch], - target_branch: attrs[:branch], + branch_name: attrs[:branch], commit_message: attrs[:commit_message], file_content: attrs[:content], file_content_encoding: attrs[:encoding], @@ -123,7 +123,7 @@ module API file_params = declared_params(include_missing: false) file_params[:branch] = file_params.delete(:branch_name) - result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute + result = ::Files::DeleteService.new(user_project, current_user, commit_params(file_params)).execute if result[:status] == :success status(200) diff --git a/lib/gitlab/git/index.rb b/lib/gitlab/git/index.rb index af1744c9c46..1add037fa5f 100644 --- a/lib/gitlab/git/index.rb +++ b/lib/gitlab/git/index.rb @@ -1,8 +1,12 @@ module Gitlab module Git class Index + IndexError = Class.new(StandardError) + DEFAULT_MODE = 0o100644 + ACTIONS = %w(create create_dir update move delete).freeze + attr_reader :repository, :raw_index def initialize(repository) @@ -23,9 +27,8 @@ module Gitlab def create(options) options = normalize_options(options) - file_entry = get(options[:file_path]) - if file_entry - raise Gitlab::Git::Repository::InvalidBlobName.new("Filename already exists") + if get(options[:file_path]) + raise IndexError, "A file with this name already exists" end add_blob(options) @@ -34,13 +37,12 @@ module Gitlab def create_dir(options) options = normalize_options(options) - file_entry = get(options[:file_path]) - if file_entry - raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists as a file") + if get(options[:file_path]) + raise IndexError, "A file with this name already exists" end if dir_exists?(options[:file_path]) - raise Gitlab::Git::Repository::InvalidBlobName.new("Directory already exists") + raise IndexError, "A directory with this name already exists" end options = options.dup @@ -55,7 +57,7 @@ module Gitlab file_entry = get(options[:file_path]) unless file_entry - raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist") + raise IndexError, "A file with this name doesn't exist" end add_blob(options, mode: file_entry[:mode]) @@ -66,7 +68,11 @@ module Gitlab file_entry = get(options[:previous_path]) unless file_entry - raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist") + raise IndexError, "A file with this name doesn't exist" + end + + if get(options[:file_path]) + raise IndexError, "A file with this name already exists" end raw_index.remove(options[:previous_path]) @@ -77,9 +83,8 @@ module Gitlab def delete(options) options = normalize_options(options) - file_entry = get(options[:file_path]) - unless file_entry - raise Gitlab::Git::Repository::InvalidBlobName.new("File doesn't exist") + unless get(options[:file_path]) + raise IndexError, "A file with this name doesn't exist" end raw_index.remove(options[:file_path]) @@ -95,10 +100,20 @@ module Gitlab end def normalize_path(path) + unless path + raise IndexError, "You must provide a file path" + end + pathname = Gitlab::Git::PathHelper.normalize_path(path.dup) - if pathname.each_filename.include?('..') - raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path') + pathname.each_filename do |segment| + if segment == '..' + raise IndexError, 'Path cannot include directory traversal' + end + + unless segment =~ Gitlab::Regex.file_name_regex + raise IndexError, "Path #{Gitlab::Regex.file_name_regex_message}" + end end pathname.to_s @@ -106,6 +121,10 @@ module Gitlab def add_blob(options, mode: nil) content = options[:content] + unless content + raise IndexError, "You must provide content" + end + content = Base64.decode64(content) if options[:encoding] == 'base64' detect = CharlockHolmes::EncodingDetector.new.detect(content) @@ -119,7 +138,7 @@ module Gitlab raw_index.add(path: options[:file_path], oid: oid, mode: mode || DEFAULT_MODE) rescue Rugged::IndexError => e - raise Gitlab::Git::Repository::InvalidBlobName.new(e.message) + raise IndexError, e.message end end end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index e599dd4a656..08b061d5e31 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -73,22 +73,6 @@ module Gitlab "can contain only letters, digits, '_', '-', '@', '+' and '.'." end - def file_path_regex - @file_path_regex ||= /\A[[[:alnum:]]_\-\.\/\@]*\z/.freeze - end - - def file_path_regex_message - "can contain only letters, digits, '_', '-', '@' and '.'. Separate directories with a '/'." - end - - def directory_traversal_regex - @directory_traversal_regex ||= /\.{2}/.freeze - end - - def directory_traversal_regex_message - "cannot include directory traversal." - end - def archive_formats_regex # |zip|tar| tar.gz | tar.bz2 | @archive_formats_regex ||= /(zip|tar|tar\.gz|tgz|gz|tar\.bz2|tbz|tbz2|tb2|bz2)/.freeze -- cgit v1.2.1