diff --git a/app/models/repository.rb b/app/models/repository.rb
index ba34d570dbd197e607fe3df10681b64838670c86..0c797dd581402b07beb4991d28e4350d98e9a79d 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -789,7 +789,7 @@ class Repository
}
options.merge!(get_committer_and_author(user, email: author_email, name: author_name))
- Rugged::Commit.create(rugged, options)
+ create_commit(options)
end
end
# rubocop:enable Metrics/ParameterLists
@@ -836,7 +836,7 @@ class Repository
tree: merge_index.write_tree(rugged),
)
- commit_id = Rugged::Commit.create(rugged, actual_options)
+ commit_id = create_commit(actual_options)
merge_request.update(in_progress_merge_commit_sha: commit_id)
commit_id
end
@@ -859,12 +859,11 @@ class Repository
committer = user_to_committer(user)
- Rugged::Commit.create(rugged,
- message: commit.revert_message(user),
- author: committer,
- committer: committer,
- tree: revert_tree_id,
- parents: [start_commit.sha])
+ create_commit(message: commit.revert_message(user),
+ author: committer,
+ committer: committer,
+ tree: revert_tree_id,
+ parents: [start_commit.sha])
end
end
@@ -883,16 +882,15 @@ class Repository
committer = user_to_committer(user)
- Rugged::Commit.create(rugged,
- message: commit.message,
- author: {
- email: commit.author_email,
- name: commit.author_name,
- time: commit.authored_date
- },
- committer: committer,
- tree: cherry_pick_tree_id,
- parents: [start_commit.sha])
+ create_commit(message: commit.message,
+ author: {
+ email: commit.author_email,
+ name: commit.author_name,
+ time: commit.authored_date
+ },
+ committer: committer,
+ tree: cherry_pick_tree_id,
+ parents: [start_commit.sha])
end
end
@@ -900,7 +898,7 @@ class Repository
GitOperationService.new(user, self).with_branch(branch_name) do
committer = user_to_committer(user)
- Rugged::Commit.create(rugged, params.merge(author: committer, committer: committer))
+ create_commit(params.merge(author: committer, committer: committer))
end
end
@@ -1142,6 +1140,12 @@ class Repository
Gitlab::Metrics.add_event(event, { path: path_with_namespace }.merge(tags))
end
+ def create_commit(params = {})
+ params[:message].delete!("\r")
+
+ Rugged::Commit.create(rugged, params)
+ end
+
def repository_storage_path
@project.repository_storage_path
end
diff --git a/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml b/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml
new file mode 100644
index 0000000000000000000000000000000000000000..c33fa944a83f54e012fee8346ff81f4c65d8c9e6
--- /dev/null
+++ b/changelogs/unreleased/31671-merge-request-message-contains-carriage-returns.yml
@@ -0,0 +1,4 @@
+---
+title: Remove carriage returns from commit messages
+merge_request: 11077
+author:
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index acd0037ee4f3b153ff9f20fbeee722babe70744f..e7485c220399d94415e604dd528e5f90ce72d46a 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -876,27 +876,6 @@ module Gitlab
rugged.remotes[remote_name].push(refspecs)
end
- # Merge the +source_name+ branch into the +target_name+ branch. This is
- # equivalent to `git merge --no_ff +source_name+`, since a merge commit
- # is always created.
- def merge(source_name, target_name, options = {})
- our_commit = rugged.branches[target_name].target
- their_commit = rugged.branches[source_name].target
-
- raise "Invalid merge target" if our_commit.nil?
- raise "Invalid merge source" if their_commit.nil?
-
- merge_index = rugged.merge_commits(our_commit, their_commit)
- return false if merge_index.conflicts?
-
- actual_options = options.merge(
- parents: [our_commit, their_commit],
- tree: merge_index.write_tree(rugged),
- update_ref: "refs/heads/#{target_name}"
- )
- Rugged::Commit.create(rugged, actual_options)
- end
-
AUTOCRLF_VALUES = {
"true" => true,
"false" => false,
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 5216764a82dd6e79fe6130f4bfe18a9e20798c2e..dd6514b3b50aed3da5033f2a1d4fc6d8adaff879 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1098,21 +1098,33 @@ describe Repository, models: true do
end
describe '#merge' do
- it 'merges the code and return the commit id' do
+ let(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) }
+
+ let(:commit_options) do
+ author = repository.user_to_committer(user)
+ { message: 'Test \r\n\r\n message', committer: author, author: author }
+ end
+
+ it 'merges the code and returns the commit id' do
expect(merge_commit).to be_present
expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
end
it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do
- merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
-
- merge_commit_id = repository.merge(user,
- merge_request.diff_head_sha,
- merge_request,
- commit_options)
+ merge_commit_id = merge(repository, user, merge_request, commit_options)
expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id)
end
+
+ it 'removes carriage returns from commit message' do
+ merge_commit_id = merge(repository, user, merge_request, commit_options)
+
+ expect(repository.commit(merge_commit_id).message).to eq(commit_options[:message].delete("\r"))
+ end
+
+ def merge(repository, user, merge_request, options = {})
+ repository.merge(user, merge_request.diff_head_sha, merge_request, options)
+ end
end
describe '#revert' do