Fix saving diffs that are not valid UTF-835539-can-t-create-a-merge-request-containing-a-binary-file-with-non-utf-8-characters

Previously, we used Psych, which would:

1. Check if a string was encoded as binary, and not ASCII-compatible.
2. Add the !binary tag in that case.
3. Convert to base64.

We need to do the same thing, using a new column in place of the tag.

5 files changed, 40 insertions, 4 deletions

diff --git a/spec/features/projects/ref_switcher_spec.rb b/spec/features/projects/ref_switcher_spec.rb
index 31c7b492ab7..9f5544ac43e 100644
--- a/spec/features/projects/ref_switcher_spec.rb
+++ b/spec/features/projects/ref_switcher_spec.rb
@@ -19,14 +19,14 @@ feature 'Ref switcher', feature: true, js: true do
       input.set 'binary'
       wait_for_requests
 
-      expect(find('.dropdown-content ul')).to have_selector('li', count: 6)
+      expect(find('.dropdown-content ul')).to have_selector('li', count: 7)
 
       page.within '.dropdown-content ul' do
         input.native.send_keys :enter
       end
     end
 
-    expect(page).to have_title 'binary-encoding'
+    expect(page).to have_title 'add-pdf-text-binary'
   end
 
   it "user selects ref with special characters" do
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 0f2db3380a7..11f4c16ff96 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -195,6 +195,7 @@ MergeRequestDiffFile:
 - a_mode
 - b_mode
 - too_large
+- binary
 Ci::Pipeline:
 - id
 - project_id
diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb
index 7276f5b5061..239620ef4fc 100644
--- a/spec/models/merge_request_diff_file_spec.rb
+++ b/spec/models/merge_request_diff_file_spec.rb
@@ -1,8 +1,33 @@
 require 'rails_helper'
 
 describe MergeRequestDiffFile, type: :model do
+  describe '#diff' 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
+      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
+
   describe '#utf8_diff' do
-    it 'does not raise error when a hash value is in binary' do
+    it 'does not raise error when the diff is binary' do
       subject.diff = "\x05\x00\x68\x65\x6c\x6c\x6f"
 
       expect { subject.utf8_diff }.not_to raise_error
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index edc2f4bb9f0..0e77752bccc 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -105,6 +105,15 @@ describe MergeRequestDiff, models: true do
 
       expect(mr_diff.empty?).to be_truthy
     end
+
+    it 'saves binary diffs correctly' do
+      path = 'files/images/icn-time-tracking.pdf'
+      mr_diff = create(:merge_request, source_branch: 'add-pdf-text-binary', target_branch: 'master').merge_request_diff
+      diff_file = mr_diff.merge_request_diff_files.find_by(new_path: path)
+
+      expect(diff_file).to be_binary
+      expect(diff_file.diff).to eq(mr_diff.compare.diffs(paths: [path]).to_a.first.diff)
+    end
   end
 
   describe '#commit_shas' do
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 0a194ca4c90..c32c05b03e2 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -41,7 +41,8 @@ module TestEnv
     'csv'                                => '3dd0896',
     'v1.1.0'                             => 'b83d6e3',
     'add-ipython-files'                  => '93ee732',
-    'add-pdf-file'                       => 'e774ebd'
+    'add-pdf-file'                       => 'e774ebd',
+    'add-pdf-text-binary'                => '79faa7b'
   }.freeze
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily