summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Thomas <nick@gitlab.com>2019-09-12 12:53:11 +0000
committerNick Thomas <nick@gitlab.com>2019-09-12 12:53:11 +0000
commitd02ee542b4a6ae769545d1637fc06a38ad8b298f (patch)
treeb079e25e74d336def54a928c417fe8e0c9586c4e
parent6d437bc751867d23d13864976a90f426b3188743 (diff)
parentce46b40252f5667c6dc5817acee62f100b693c53 (diff)
downloadgitlab-ce-d02ee542b4a6ae769545d1637fc06a38ad8b298f.tar.gz
Merge branch '61841-fix-encoding-error-in-mr-diffs' into 'master'
Fix encoding error in MR diffs Closes #61841 See merge request gitlab-org/gitlab-ce!32862
-rw-r--r--app/models/merge_request_diff_file.rb2
-rw-r--r--changelogs/unreleased/61841-fix-encoding-error-in-mr-diffs.yml5
-rw-r--r--spec/models/merge_request_diff_file_spec.rb59
3 files changed, 52 insertions, 14 deletions
diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb
index 01ee82ae398..a532c1e6356 100644
--- a/app/models/merge_request_diff_file.rb
+++ b/app/models/merge_request_diff_file.rb
@@ -17,7 +17,7 @@ class MergeRequestDiffFile < ApplicationRecord
if merge_request_diff&.stored_externally?
merge_request_diff.opening_external_diff do |file|
file.seek(external_diff_offset)
- file.read(external_diff_size)
+ force_encode_utf8(file.read(external_diff_size))
end
else
super
diff --git a/changelogs/unreleased/61841-fix-encoding-error-in-mr-diffs.yml b/changelogs/unreleased/61841-fix-encoding-error-in-mr-diffs.yml
new file mode 100644
index 00000000000..8f689d89671
--- /dev/null
+++ b/changelogs/unreleased/61841-fix-encoding-error-in-mr-diffs.yml
@@ -0,0 +1,5 @@
+---
+title: Fix encoding error in MR diffs when using external diffs
+merge_request: 32862
+author: Hiroyuki Sato
+type: fixed
diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb
index 97b30bb8607..84f9c9d06ba 100644
--- a/spec/models/merge_request_diff_file_spec.rb
+++ b/spec/models/merge_request_diff_file_spec.rb
@@ -4,26 +4,59 @@ require 'spec_helper'
describe MergeRequestDiffFile do
describe '#diff' do
- let(:unpacked) { 'unpacked' }
- let(:packed) { [unpacked].pack('m0') }
+ context 'when diff is not stored' do
+ let(:unpacked) { 'unpacked' }
+ let(:packed) { [unpacked].pack('m0') }
- before do
- subject.diff = packed
- end
-
- context 'when the diff is marked as binary' do
before do
- subject.binary = true
+ subject.diff = packed
+ end
+
+ context 'when the diff is marked as binary' do
+ before do
+ subject.binary = true
+ end
+
+ it 'unpacks from base 64' do
+ expect(subject.diff).to eq(unpacked)
+ end
+ end
+
+ context 'when the diff is not marked as binary' do
+ it 'returns the raw diff' do
+ expect(subject.diff).to eq(packed)
+ end
end
+ end
- it 'unpacks from base 64' do
- expect(subject.diff).to eq(unpacked)
+ context 'when diff is stored in DB' do
+ let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first }
+
+ it 'returns UTF-8 string' do
+ expect(file.diff.encoding).to eq Encoding::UTF_8
end
end
- context 'when the diff is not marked as binary' do
- it 'returns the raw diff' do
- expect(subject.diff).to eq(packed)
+ context 'when diff is stored in external storage' do
+ let(:file) { create(:merge_request).merge_request_diff.merge_request_diff_files.first }
+ let(:test_dir) { 'tmp/tests/external-diffs' }
+
+ around do |example|
+ FileUtils.mkdir_p(test_dir)
+
+ begin
+ example.run
+ ensure
+ FileUtils.rm_rf(test_dir)
+ end
+ end
+
+ before do
+ stub_external_diffs_setting(enabled: true, storage_path: test_dir)
+ end
+
+ it 'returns UTF-8 string' do
+ expect(file.diff.encoding).to eq Encoding::UTF_8
end
end
end