Merge branch 'stanhu/gitlab-ce-fix-error-500-false-binary-negative'

10 files changed, 27 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index eb583af9f95..3674cd6055b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,7 @@ v 8.9.0 (unreleased)
   - Remove project notification settings associated with deleted projects
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects
   - Add a metric for the number of new Redis connections created by a transaction
+  - 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
   - Add Environments and Deployments
diff --git a/Gemfile.lock b/Gemfile.lock
index d517fcb8ed3..c1c8c175b1d 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -277,7 +277,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)
@@ -400,7 +400,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