diff options
| -rw-r--r-- | app/models/merge_request_diff_file.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/61841-fix-encoding-error-in-mr-diffs.yml | 5 | ||||
| -rw-r--r-- | spec/models/merge_request_diff_file_spec.rb | 59 |
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 |