Merge branch 'sh-handle-null-bytes-in-merge-request-diffs' into 'master'
Fix error creating a merge request when diff includes a null byte Closes #57710 See merge request gitlab-org/gitlab-ce!26190
This commit is contained in:
commit
a885e2d0a6
|
@ -296,6 +296,11 @@ class MergeRequestDiff < ActiveRecord::Base
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def encode_in_base64?(diff_text)
|
||||||
|
(diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?) ||
|
||||||
|
diff_text.include?("\0")
|
||||||
|
end
|
||||||
|
|
||||||
def create_merge_request_diff_files(diffs)
|
def create_merge_request_diff_files(diffs)
|
||||||
rows =
|
rows =
|
||||||
if has_attribute?(:external_diff) && Gitlab.config.external_diffs.enabled
|
if has_attribute?(:external_diff) && Gitlab.config.external_diffs.enabled
|
||||||
|
@ -348,7 +353,7 @@ class MergeRequestDiff < ActiveRecord::Base
|
||||||
diff_hash.tap do |hash|
|
diff_hash.tap do |hash|
|
||||||
diff_text = hash[:diff]
|
diff_text = hash[:diff]
|
||||||
|
|
||||||
if diff_text.encoding == Encoding::BINARY && !diff_text.ascii_only?
|
if encode_in_base64?(diff_text)
|
||||||
hash[:binary] = true
|
hash[:binary] = true
|
||||||
hash[:diff] = [diff_text].pack('m0')
|
hash[:diff] = [diff_text].pack('m0')
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix error creating a merge request when diff includes a null byte
|
||||||
|
merge_request: 26190
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -1,6 +1,8 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe MergeRequestDiff do
|
describe MergeRequestDiff do
|
||||||
|
include RepoHelpers
|
||||||
|
|
||||||
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
|
let(:diff_with_commits) { create(:merge_request).merge_request_diff }
|
||||||
|
|
||||||
describe 'validations' do
|
describe 'validations' do
|
||||||
|
@ -194,6 +196,25 @@ describe MergeRequestDiff do
|
||||||
expect(diff_file).to be_binary
|
expect(diff_file).to be_binary
|
||||||
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff)
|
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'with diffs that contain a null byte' do
|
||||||
|
let(:filename) { 'test-null.txt' }
|
||||||
|
let(:content) { "a" * 10000 + "\x00" }
|
||||||
|
let(:project) { create(:project, :repository) }
|
||||||
|
let(:branch) { 'null-data' }
|
||||||
|
let(:target_branch) { 'master' }
|
||||||
|
|
||||||
|
it 'saves diffs correctly' do
|
||||||
|
create_file_in_repo(project, target_branch, branch, filename, content)
|
||||||
|
|
||||||
|
mr_diff = create(:merge_request, target_project: project, source_project: project, source_branch: branch, target_branch: target_branch).merge_request_diff
|
||||||
|
diff_file = mr_diff.merge_request_diff_files.find_by(new_path: filename)
|
||||||
|
|
||||||
|
expect(diff_file).to be_binary
|
||||||
|
expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [filename]).to_a.first.diff)
|
||||||
|
expect(diff_file.diff).to include(content)
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -115,4 +115,18 @@ eos
|
||||||
commits: commits
|
commits: commits
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def create_file_in_repo(
|
||||||
|
project, start_branch, branch_name, filename, content,
|
||||||
|
commit_message: 'Add new content')
|
||||||
|
Files::CreateService.new(
|
||||||
|
project,
|
||||||
|
project.owner,
|
||||||
|
commit_message: commit_message,
|
||||||
|
start_branch: start_branch,
|
||||||
|
branch_name: branch_name,
|
||||||
|
file_path: filename,
|
||||||
|
file_content: content
|
||||||
|
).execute
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue