Fix Error 500 when viewing a blob with binary characters after the 1024-byte mark

Here was the problem:

1. When determining whether a given blob is viewable text, gitlab_git reads the first 1024 bytes and checks with Linguist whether it is a text or binary file.
2. If the blob is text, GitLab will attempt to display it.
3. However, if the text has binary characters after the first 1024 bytes, then GitLab will attempt to load the entire contents, but the encoding will be ASCII-8BIT since there are binary characters.
4. The Error 500 results when GitLab attempts to display a mix UTF-8 and ASCII-8BIT.

To fix this, we load as much data as we are willing to display so that the detection will work properly. Requires
an update to gitlab_git: gitlab-org/gitlab_git!86

Closes #13826

10 files changed, 27 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 8f897b4a34c..acb349594aa 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -18,6 +18,7 @@ v 8.9.0 (unreleased)
   - Reduce number of fog gem dependencies
   - Remove project notification settings associated with deleted projects
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects
+  - Fix Error 500 when viewing a blob with binary characters after the 1024-byte mark
   - Redesign navigation for project pages
   - Fix groups API to list only user's accessible projects
   - Redesign account and email confirmation emails
diff --git a/Gemfile.lock b/Gemfile.lock
index dfc15700494..15b3158c63e 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -284,7 +284,7 @@ GEM
       posix-spawn (~> 0.3)
     gitlab_emoji (0.3.1)
       gemojione (~> 2.2, >= 2.2.1)
-    gitlab_git (10.1.0)
+    gitlab_git (10.1.3)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -408,7 +408,7 @@ GEM
       mime-types (>= 1.16, < 4)
     mail_room (0.7.0)
     method_source (0.8.2)
-    mime-types (2.99.1)
+    mime-types (2.99.2)
     mimemagic (0.3.0)
     mini_portile2 (2.1.0)
     minitest (5.7.0)
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index cec2dc753fe..85559fbc5f5 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -116,7 +116,7 @@ module BlobHelper
   end
 
   def blob_text_viewable?(blob)
-    blob && blob.text? && !blob.lfs_pointer?
+    blob && blob.text? && !blob.lfs_pointer? && !blob.only_display_raw?
   end
 
   def blob_size(blob)
diff --git a/app/models/blob.rb b/app/models/blob.rb
index 0fea6b7f576..4279ea2ce57 100644
--- a/app/models/blob.rb
+++ b/app/models/blob.rb
@@ -24,7 +24,7 @@ class Blob < SimpleDelegator
   end
 
   def only_display_raw?
-    size && size > 5.megabytes
+    size && truncated?
   end
 
   def svg?
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1ab163510bf..e5b277cb198 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -446,7 +446,7 @@ class Repository
 
   def blob_at(sha, path)
     unless Gitlab::Git.blank_ref?(sha)
-      Gitlab::Git::Blob.find(self, sha, path)
+      Blob.decorate(Gitlab::Git::Blob.find(self, sha, path))
     end
   end
 
diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml
index d9c4b410d32..6c11afbe420 100644
--- a/app/views/projects/diffs/_diffs.html.haml
+++ b/app/views/projects/diffs/_diffs.html.haml
@@ -24,6 +24,7 @@
     - diff_commit = commit_for_diff(diff_file)
     - blob = project.repository.blob_for_diff(diff_commit, diff_file)
     - next unless blob
+    - blob.load_all_data!(project.repository) unless blob.only_display_raw?
 
     = render 'projects/diffs/file', i: index, project: project,
       diff_file: diff_file, diff_commit: diff_commit, blob: blob, diff_refs: diff_refs
diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml
index e5983c58039..2395ea3c275 100644
--- a/app/views/projects/diffs/_file.html.haml
+++ b/app/views/projects/diffs/_file.html.haml
@@ -49,6 +49,8 @@
         = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
       - else
         = render "projects/diffs/text_file", diff_file: diff_file, index: i
+    - elsif blob.only_display_raw?
+      .nothing-here-block This file is too large to display.
     - elsif blob.image?
       - old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file)
       = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
diff --git a/spec/controllers/blob_controller_spec.rb b/spec/controllers/blob_controller_spec.rb
index eb91e577b87..465013231f9 100644
--- a/spec/controllers/blob_controller_spec.rb
+++ b/spec/controllers/blob_controller_spec.rb
@@ -38,6 +38,11 @@ describe Projects::BlobController do
       let(:id) { 'invalid-branch/README.md' }
       it { is_expected.to respond_with(:not_found) }
     end
+
+    context "binary file" do
+      let(:id) { 'binary-encoding/encoding/binary-1.bin' }
+      it { is_expected.to respond_with(:success) }
+    end
   end
 
   describe 'GET show with tree path' do
diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb
index 438e776ec4b..6e3db10e451 100644
--- a/spec/controllers/projects/commit_controller_spec.rb
+++ b/spec/controllers/projects/commit_controller_spec.rb
@@ -2,6 +2,8 @@ require 'rails_helper'
 
 describe Projects::CommitController do
   describe 'GET show' do
+    render_views
+
     let(:project) { create(:project) }
 
     before do
@@ -27,6 +29,16 @@ describe Projects::CommitController do
       end
     end
 
+    it 'handles binary files' do
+      get(:show,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: TestEnv::BRANCH_SHA['binary-encoding'],
+          format: "html")
+
+      expect(response).to be_success
+    end
+
     def go(id:)
       get :show,
         namespace_id: project.namespace.to_param,
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 71664bb192e..498bd4bf800 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -16,6 +16,7 @@ module TestEnv
     'master'           => '5937ac0',
     "'test'"           => 'e56497b',
     'orphaned-branch'  => '45127a9',
+    'binary-encoding'  => '7b1cf43',
   }
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily