summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2018-02-21 13:42:11 +0000
committerSean McGivern <sean@gitlab.com>2018-02-22 12:26:23 +0000
commitcdf3ae04f84abe039b79ab31192bb7c462bf7ce5 (patch)
tree6631529a0cc5f189d25928551d2b20137dc5f3ad
parent9ff91bc481683e3619e0745b03161a12a5afd497 (diff)
downloadgitlab-ce-cdf3ae04f84abe039b79ab31192bb7c462bf7ce5.tar.gz
Fix 500 error when diff context line has broken encoding42332-actionview-template-error-366-524-out-of-range
Rugged sometimes chops a context line in between bytes, resulting in the context line having an invalid encoding: https://github.com/libgit2/rugged/issues/716 When that happens, we will try to detect the encoding for the diff, and sometimes we'll get it wrong. If that difference in encoding results in a difference in string lengths between the diff and the underlying blobs, we'd fail to highlight any inline diffs, and return a 500 status to the user. As we're using the underlying blobs, the encoding is 'correct' anyway, so if the string range is invalid, we can just discard the inline diff highlighting. We still report to Sentry to ensure that we can investigate further in future.
-rw-r--r--changelogs/unreleased/42332-actionview-template-error-366-524-out-of-range.yml5
-rw-r--r--lib/gitlab/diff/highlight.rb12
-rw-r--r--spec/lib/gitlab/diff/highlight_spec.rb22
3 files changed, 38 insertions, 1 deletions
diff --git a/changelogs/unreleased/42332-actionview-template-error-366-524-out-of-range.yml b/changelogs/unreleased/42332-actionview-template-error-366-524-out-of-range.yml
new file mode 100644
index 00000000000..626c761bfbd
--- /dev/null
+++ b/changelogs/unreleased/42332-actionview-template-error-366-524-out-of-range.yml
@@ -0,0 +1,5 @@
+---
+title: Fix 500 error being shown when diff has context marker with invalid encoding
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/diff/highlight.rb b/lib/gitlab/diff/highlight.rb
index 0f897e6316c..269016daac2 100644
--- a/lib/gitlab/diff/highlight.rb
+++ b/lib/gitlab/diff/highlight.rb
@@ -27,7 +27,17 @@ module Gitlab
rich_line = highlight_line(diff_line) || diff_line.text
if line_inline_diffs = inline_diffs[i]
- rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs)
+ begin
+ rich_line = InlineDiffMarker.new(diff_line.text, rich_line).mark(line_inline_diffs)
+ # This should only happen when the encoding of the diff doesn't
+ # match the blob, which is a bug. But we shouldn't fail to render
+ # completely in that case, even though we want to report the error.
+ rescue RangeError => e
+ if Gitlab::Sentry.enabled?
+ Gitlab::Sentry.context
+ Raven.capture_exception(e)
+ end
+ end
end
diff_line.text = rich_line
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index cd602ccab8e..73d60c021c8 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -72,6 +72,28 @@ describe Gitlab::Diff::Highlight do
expect(subject[5].text).to eq(code)
expect(subject[5].text).to be_html_safe
end
+
+ context 'when the inline diff marker has an invalid range' do
+ before do
+ allow_any_instance_of(Gitlab::Diff::InlineDiffMarker).to receive(:mark).and_raise(RangeError)
+ end
+
+ it 'keeps the original rich line' do
+ code = %q{+ raise RuntimeError, "System commands must be given as an array of strings"}
+
+ expect(subject[5].text).to eq(code)
+ expect(subject[5].text).not_to be_html_safe
+ end
+
+ it 'reports to Sentry if configured' do
+ allow(Gitlab::Sentry).to receive(:enabled?).and_return(true)
+
+ expect(Gitlab::Sentry).to receive(:context)
+ expect(Raven).to receive(:capture_exception)
+
+ subject
+ end
+ end
end
end
end