Commit 0d352010 authored by Igor Drozdov's avatar Igor Drozdov

Highlight huge diffs as plain text without loading a blob

Feature flag: limited_diff_highlighting

When a huge file changes, its whole content highlighted
as a plain text anyway:
https://gitlab.com/gitlab-org/gitlab/-/issues/300228
parent 933bc278
---
name: limited_diff_highlighting
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53768
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323566
milestone: '13.12'
type: development
group: group::source code
default_enabled: true
...@@ -87,6 +87,7 @@ module Gitlab ...@@ -87,6 +87,7 @@ module Gitlab
def highlight_line(diff_line) def highlight_line(diff_line)
return unless diff_file && diff_file.diff_refs return unless diff_file && diff_file.diff_refs
return diff_line_highlighting(diff_line, plain: true) if blobs_too_large?
if Feature.enabled?(:diff_line_syntax_highlighting, project, default_enabled: :yaml) if Feature.enabled?(:diff_line_syntax_highlighting, project, default_enabled: :yaml)
diff_line_highlighting(diff_line) diff_line_highlighting(diff_line)
...@@ -95,9 +96,10 @@ module Gitlab ...@@ -95,9 +96,10 @@ module Gitlab
end end
end end
def diff_line_highlighting(diff_line) def diff_line_highlighting(diff_line, plain: false)
rich_line = syntax_highlighter(diff_line).highlight( rich_line = syntax_highlighter(diff_line).highlight(
diff_line.text(prefix: false), diff_line.text(prefix: false),
plain: plain,
context: { line_number: diff_line.line } context: { line_number: diff_line.line }
) )
...@@ -158,6 +160,13 @@ module Gitlab ...@@ -158,6 +160,13 @@ module Gitlab
blob.load_all_data! blob.load_all_data!
blob.present.highlight.lines blob.present.highlight.lines
end end
def blobs_too_large?
return false unless Feature.enabled?(:limited_diff_highlighting, project, default_enabled: :yaml)
return true if Gitlab::Highlight.too_large?(diff_file.old_blob&.size)
Gitlab::Highlight.too_large?(diff_file.new_blob&.size)
end
end end
end end
end end
...@@ -10,6 +10,10 @@ module Gitlab ...@@ -10,6 +10,10 @@ module Gitlab
.highlight(blob_content, continue: false, plain: plain) .highlight(blob_content, continue: false, plain: plain)
end end
def self.too_large?(size)
size.to_i > Gitlab.config.extra['maximum_text_highlight_size_kilobytes']
end
attr_reader :blob_name attr_reader :blob_name
def initialize(blob_name, blob_content, language: nil) def initialize(blob_name, blob_content, language: nil)
...@@ -22,7 +26,7 @@ module Gitlab ...@@ -22,7 +26,7 @@ module Gitlab
def highlight(text, continue: false, plain: false, context: {}) def highlight(text, continue: false, plain: false, context: {})
@context = context @context = context
plain ||= text.length > maximum_text_highlight_size plain ||= self.class.too_large?(text.length)
highlighted_text = highlight_text(text, continue: continue, plain: plain) highlighted_text = highlight_text(text, continue: continue, plain: plain)
highlighted_text = link_dependencies(text, highlighted_text) if blob_name highlighted_text = link_dependencies(text, highlighted_text) if blob_name
...@@ -80,10 +84,6 @@ module Gitlab ...@@ -80,10 +84,6 @@ module Gitlab
Gitlab::DependencyLinker.link(blob_name, text, highlighted_text) Gitlab::DependencyLinker.link(blob_name, text, highlighted_text)
end end
def maximum_text_highlight_size
Gitlab.config.extra['maximum_text_highlight_size_kilobytes']
end
def add_highlight_timeout_metric def add_highlight_timeout_metric
return unless Feature.enabled?(:track_highlight_timeouts) return unless Feature.enabled?(:track_highlight_timeouts)
......
...@@ -5,7 +5,8 @@ require 'spec_helper' ...@@ -5,7 +5,8 @@ require 'spec_helper'
RSpec.describe Gitlab::Diff::Highlight do RSpec.describe Gitlab::Diff::Highlight do
include RepoHelpers include RepoHelpers
let(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let(:commit) { project.commit(sample_commit.id) } let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.raw_diffs.first } let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
...@@ -156,5 +157,34 @@ RSpec.describe Gitlab::Diff::Highlight do ...@@ -156,5 +157,34 @@ RSpec.describe Gitlab::Diff::Highlight do
it_behaves_like 'without inline diffs' it_behaves_like 'without inline diffs'
end end
end end
context 'when blob is too large' do
let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
before do
allow(Gitlab::Highlight).to receive(:too_large?).and_return(true)
end
it 'blobs are highlighted as plain text without loading all data' do
expect(diff_file.blob).not_to receive(:load_all_data!)
expect(subject[2].rich_text).to eq(%Q{ <span id="LC7" class="line" lang=""> def popen(cmd, path=nil)</span>\n})
expect(subject[2].rich_text).to be_html_safe
end
context 'when limited_diff_highlighting is disabled' do
before do
stub_feature_flags(limited_diff_highlighting: false)
stub_feature_flags(diff_line_syntax_highlighting: false)
end
it 'blobs are highlighted as plain text with loading all data' do
expect(diff_file.blob).to receive(:load_all_data!).twice
code = %Q{ <span id="LC7" class="line" lang=""> def popen(cmd, path=nil)</span>\n}
expect(subject[2].rich_text).to eq(code)
end
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment