Consistently create, update, and delete files, taking CRLF settings into account

2 files changed, 39 insertions, 103 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index cd2568ad445..b747b71ae68 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -752,24 +752,28 @@ class Repository
     message:, branch_name:,
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
-    check_tree_entry_for_dir(branch_name, path)
 
-    if start_branch_name
-      start_project.repository.
-        check_tree_entry_for_dir(start_branch_name, path)
+    entry = tree_entry_at(start_branch_name || branch_name, path)
+    if entry
+      if entry[:type] == :blob
+        raise Gitlab::Git::Repository::InvalidBlobName.new(
+          "Directory already exists as a file")
+      else
+        raise Gitlab::Git::Repository::InvalidBlobName.new(
+          "Directory already exists")
+      end
     end
 
-    commit_file(
-      user,
-      "#{path}/.gitkeep",
-      '',
+    multi_action(
+      user: user,
       message: message,
       branch_name: branch_name,
-      update: false,
       author_email: author_email,
       author_name: author_name,
       start_branch_name: start_branch_name,
-      start_project: start_project)
+      start_project: start_project,
+      actions: [{ action: :create_dir,
+                  file_path: path }])
   end
   # rubocop:enable Metrics/ParameterLists
 
@@ -779,18 +783,8 @@ class Repository
     message:, branch_name:, update: true,
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
-    unless update
-      error_message = "Filename already exists; update not allowed"
 
-      if tree_entry_at(branch_name, path)
-        raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
-      end
-
-      if start_branch_name &&
-          start_project.repository.tree_entry_at(start_branch_name, path)
-        raise Gitlab::Git::Repository::InvalidBlobName.new(error_message)
-      end
-    end
+    action = update ? :update : :create
 
     multi_action(
       user: user,
@@ -800,7 +794,7 @@ class Repository
       author_name: author_name,
       start_branch_name: start_branch_name,
       start_project: start_project,
-      actions: [{ action: :create,
+      actions: [{ action: action,
                   file_path: path,
                   content: content }])
   end
@@ -839,6 +833,7 @@ class Repository
     message:, branch_name:,
     author_email: nil, author_name: nil,
     start_branch_name: nil, start_project: project)
+
     multi_action(
       user: user,
       message: message,
@@ -861,21 +856,22 @@ class Repository
       branch_name,
       start_branch_name: start_branch_name,
       start_project: start_project) do |start_commit|
-      index = rugged.index
 
-      parents = if start_commit
-                  index.read_tree(start_commit.raw_commit.tree)
-                  [start_commit.sha]
-                else
-                  []
-                end
+      index = Gitlab::Git::Index.new(raw_repository)
 
-      actions.each do |act|
-        git_action(index, act)
+      if start_commit
+        index.read_tree(start_commit.raw_commit.tree)
+        parents = [start_commit.sha]
+      else
+        parents = []
+      end
+
+      actions.each do |options|
+        index.__send__(options.delete(:action), options)
       end
 
       options = {
-        tree: index.write_tree(rugged),
+        tree: index.write_tree,
         message: message,
         parents: parents
       }
@@ -1174,22 +1170,6 @@ class Repository
       raw_repository.send(:tree_entry, commit(branch_name), path)
   end
 
-  def check_tree_entry_for_dir(branch_name, path)
-    return unless branch_exists?(branch_name)
-
-    entry = tree_entry_at(branch_name, path)
-
-    return unless entry
-
-    if entry[:type] == :blob
-      raise Gitlab::Git::Repository::InvalidBlobName.new(
-        "Directory already exists as a file")
-    else
-      raise Gitlab::Git::Repository::InvalidBlobName.new(
-        "Directory already exists")
-    end
-  end
-
   private
 
   def blob_data_at(sha, path)
@@ -1200,58 +1180,6 @@ class Repository
     blob.data
   end
 
-  def git_action(index, action)
-    path = normalize_path(action[:file_path])
-
-    if action[:action] == :move
-      previous_path = normalize_path(action[:previous_path])
-    end
-
-    case action[:action]
-    when :create, :update, :move
-      mode =
-        case action[:action]
-        when :update
-          index.get(path)[:mode]
-        when :move
-          index.get(previous_path)[:mode]
-        end
-      mode ||= 0o100644
-
-      index.remove(previous_path) if action[:action] == :move
-
-      content = if action[:encoding] == 'base64'
-                  Base64.decode64(action[:content])
-                else
-                  action[:content]
-                end
-
-      detect = CharlockHolmes::EncodingDetector.new.detect(content) if content
-
-      unless detect && detect[:type] == :binary
-        # When writing to the repo directly as we are doing here,
-        # the `core.autocrlf` config isn't taken into account.
-        content.gsub!("\r\n", "\n") if self.autocrlf
-      end
-
-      oid = rugged.write(content, :blob)
-
-      index.add(path: path, oid: oid, mode: mode)
-    when :delete
-      index.remove(path)
-    end
-  end
-
-  def normalize_path(path)
-    pathname = Gitlab::Git::PathHelper.normalize_path(path)
-
-    if pathname.each_filename.include?('..')
-      raise Gitlab::Git::Repository::InvalidBlobName.new('Invalid path')
-    end
-
-    pathname.to_s
-  end
-
   def refs_directory_exists?
     return false unless path_with_namespace
 
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index af6da5b9d56..809fa32eca5 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -2,6 +2,8 @@ module Files
   class MultiService < Files::BaseService
     class FileChangedError < StandardError; end
 
+    ACTIONS = %w[create update delete move].freeze
+
     def commit
       repository.multi_action(
         user: current_user,
@@ -19,15 +21,23 @@ module Files
 
     def validate
       super
-
       params[:actions].each_with_index do |action, index|
         unless action[:file_path].present?
           raise_error("You must specify a file_path.")
         end
 
+        action[:file_path].slice!(0) if action[:file_path] && action[:file_path].start_with?('/')
+        action[:previous_path].slice!(0) if action[:previous_path] && action[:previous_path].start_with?('/')
+
         regex_check(action[:file_path])
         regex_check(action[:previous_path]) if action[:previous_path]
 
+        if ACTIONS.include?(action[:action].to_s)
+          action[:action] = action[:action].to_sym
+        else
+          raise_error("Unknown action type `#{action[:action]}`.")
+        end
+
         if project.empty_repo? && action[:action] != :create
           raise_error("No files to #{action[:action]}.")
         end
@@ -43,8 +53,6 @@ module Files
           validate_delete(action)
         when :move
           validate_move(action, index)
-        else
-          raise_error("Unknown action type `#{action[:action]}`.")
         end
       end
     end