Skip to content
Snippets Groups Projects
Commit 0fdfd2dd authored by Stan Hu's avatar Stan Hu
Browse files

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_git!86

Closes #13826
parent ff345f8b
No related branches found
No related tags found
1 merge request!4270Fix Error 500 when viewing a blob with binary characters after the 1024-byte mark
Loading
@@ -18,6 +18,7 @@ v 8.9.0 (unreleased)
Loading
@@ -18,6 +18,7 @@ v 8.9.0 (unreleased)
- Reduce number of fog gem dependencies - Reduce number of fog gem dependencies
- Remove project notification settings associated with deleted projects - Remove project notification settings associated with deleted projects
- Fix 404 page when viewing TODOs that contain milestones or labels in different 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 - Redesign navigation for project pages
- Fix groups API to list only user's accessible projects - Fix groups API to list only user's accessible projects
- Redesign account and email confirmation emails - Redesign account and email confirmation emails
Loading
Loading
Loading
@@ -284,7 +284,7 @@ GEM
Loading
@@ -284,7 +284,7 @@ GEM
posix-spawn (~> 0.3) posix-spawn (~> 0.3)
gitlab_emoji (0.3.1) gitlab_emoji (0.3.1)
gemojione (~> 2.2, >= 2.2.1) gemojione (~> 2.2, >= 2.2.1)
gitlab_git (10.1.0) gitlab_git (10.1.3)
activesupport (~> 4.0) activesupport (~> 4.0)
charlock_holmes (~> 0.7.3) charlock_holmes (~> 0.7.3)
github-linguist (~> 4.7.0) github-linguist (~> 4.7.0)
Loading
@@ -408,7 +408,7 @@ GEM
Loading
@@ -408,7 +408,7 @@ GEM
mime-types (>= 1.16, < 4) mime-types (>= 1.16, < 4)
mail_room (0.7.0) mail_room (0.7.0)
method_source (0.8.2) method_source (0.8.2)
mime-types (2.99.1) mime-types (2.99.2)
mimemagic (0.3.0) mimemagic (0.3.0)
mini_portile2 (2.1.0) mini_portile2 (2.1.0)
minitest (5.7.0) minitest (5.7.0)
Loading
Loading
Loading
@@ -116,7 +116,7 @@ module BlobHelper
Loading
@@ -116,7 +116,7 @@ module BlobHelper
end end
   
def blob_text_viewable?(blob) def blob_text_viewable?(blob)
blob && blob.text? && !blob.lfs_pointer? blob && blob.text? && !blob.lfs_pointer? && !blob.only_display_raw?
end end
   
def blob_size(blob) def blob_size(blob)
Loading
Loading
Loading
@@ -24,7 +24,7 @@ class Blob < SimpleDelegator
Loading
@@ -24,7 +24,7 @@ class Blob < SimpleDelegator
end end
   
def only_display_raw? def only_display_raw?
size && size > 5.megabytes size && truncated?
end end
   
def svg? def svg?
Loading
Loading
Loading
@@ -446,7 +446,7 @@ class Repository
Loading
@@ -446,7 +446,7 @@ class Repository
   
def blob_at(sha, path) def blob_at(sha, path)
unless Gitlab::Git.blank_ref?(sha) unless Gitlab::Git.blank_ref?(sha)
Gitlab::Git::Blob.find(self, sha, path) Blob.decorate(Gitlab::Git::Blob.find(self, sha, path))
end end
end end
   
Loading
Loading
Loading
@@ -24,6 +24,7 @@
Loading
@@ -24,6 +24,7 @@
- diff_commit = commit_for_diff(diff_file) - diff_commit = commit_for_diff(diff_file)
- blob = project.repository.blob_for_diff(diff_commit, diff_file) - blob = project.repository.blob_for_diff(diff_commit, diff_file)
- next unless blob - next unless blob
- blob.load_all_data!(project.repository) unless blob.only_display_raw?
   
= render 'projects/diffs/file', i: index, project: project, = render 'projects/diffs/file', i: index, project: project,
diff_file: diff_file, diff_commit: diff_commit, blob: blob, diff_refs: diff_refs diff_file: diff_file, diff_commit: diff_commit, blob: blob, diff_refs: diff_refs
Loading
@@ -49,6 +49,8 @@
Loading
@@ -49,6 +49,8 @@
= render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i = render "projects/diffs/parallel_view", diff_file: diff_file, project: project, blob: blob, index: i
- else - else
= render "projects/diffs/text_file", diff_file: diff_file, index: i = 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? - elsif blob.image?
- old_file = project.repository.prev_blob_for_diff(diff_commit, diff_file) - 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 = render "projects/diffs/image", diff_file: diff_file, old_file: old_file, file: blob, index: i, diff_refs: diff_refs
Loading
Loading
Loading
@@ -38,6 +38,11 @@ describe Projects::BlobController do
Loading
@@ -38,6 +38,11 @@ describe Projects::BlobController do
let(:id) { 'invalid-branch/README.md' } let(:id) { 'invalid-branch/README.md' }
it { is_expected.to respond_with(:not_found) } it { is_expected.to respond_with(:not_found) }
end end
context "binary file" do
let(:id) { 'binary-encoding/encoding/binary-1.bin' }
it { is_expected.to respond_with(:success) }
end
end end
   
describe 'GET show with tree path' do describe 'GET show with tree path' do
Loading
Loading
Loading
@@ -2,6 +2,8 @@ require 'rails_helper'
Loading
@@ -2,6 +2,8 @@ require 'rails_helper'
   
describe Projects::CommitController do describe Projects::CommitController do
describe 'GET show' do describe 'GET show' do
render_views
let(:project) { create(:project) } let(:project) { create(:project) }
   
before do before do
Loading
@@ -27,6 +29,16 @@ describe Projects::CommitController do
Loading
@@ -27,6 +29,16 @@ describe Projects::CommitController do
end end
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:) def go(id:)
get :show, get :show,
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
Loading
Loading
Loading
@@ -16,6 +16,7 @@ module TestEnv
Loading
@@ -16,6 +16,7 @@ module TestEnv
'master' => '5937ac0', 'master' => '5937ac0',
"'test'" => 'e56497b', "'test'" => 'e56497b',
'orphaned-branch' => '45127a9', 'orphaned-branch' => '45127a9',
'binary-encoding' => '7b1cf43',
} }
   
# gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment