From a5d47bb063624bb30064c8e44c05de05bb63b25a Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Mon, 2 Oct 2017 13:51:51 +0100 Subject: [PATCH] Revert "Increase diff limits to 100 KB for collapse and 200 KB overall" This reverts commit 1d3c33b57eeb39df76e78fd37c86532c02aa22ac. --- lib/gitlab/git/diff.rb | 48 +++++---------------- spec/lib/gitlab/git/diff_collection_spec.rb | 3 +- spec/lib/gitlab/git/diff_spec.rb | 38 ++-------------- 3 files changed, 15 insertions(+), 74 deletions(-) diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb index 096301d300f..ca94b4baa59 100644 --- a/lib/gitlab/git/diff.rb +++ b/lib/gitlab/git/diff.rb @@ -24,41 +24,13 @@ module Gitlab SERIALIZE_KEYS = %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large).freeze + # The maximum size of a diff to display. + SIZE_LIMIT = 100.kilobytes + + # The maximum size before a diff is collapsed. + COLLAPSE_LIMIT = 10.kilobytes + class << self - # The maximum size of a diff to display. - def size_limit - if RequestStore.active? - RequestStore['gitlab_git_diff_size_limit'] ||= find_size_limit - else - find_size_limit - end - end - - # The maximum size before a diff is collapsed. - def collapse_limit - if RequestStore.active? - RequestStore['gitlab_git_diff_collapse_limit'] ||= find_collapse_limit - else - find_collapse_limit - end - end - - def find_size_limit - if Feature.enabled?('gitlab_git_diff_size_limit_increase') - 200.kilobytes - else - 100.kilobytes - end - end - - def find_collapse_limit - if Feature.enabled?('gitlab_git_diff_size_limit_increase') - 100.kilobytes - else - 10.kilobytes - end - end - def between(repo, head, base, options = {}, *paths) straight = options.delete(:straight) || false @@ -172,7 +144,7 @@ module Gitlab def too_large? if @too_large.nil? - @too_large = @diff.bytesize >= self.class.size_limit + @too_large = @diff.bytesize >= SIZE_LIMIT else @too_large end @@ -190,7 +162,7 @@ module Gitlab def collapsed? return @collapsed if defined?(@collapsed) - @collapsed = !expanded && @diff.bytesize >= self.class.collapse_limit + @collapsed = !expanded && @diff.bytesize >= COLLAPSE_LIMIT end def collapse! @@ -275,14 +247,14 @@ module Gitlab hunk.each_line do |line| size += line.content.bytesize - if size >= self.class.size_limit + if size >= SIZE_LIMIT too_large! return true end end end - if !expanded && size >= self.class.collapse_limit + if !expanded && size >= COLLAPSE_LIMIT collapse! return true end diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb index 3494f0cc98d..ee657101f4c 100644 --- a/spec/lib/gitlab/git/diff_collection_spec.rb +++ b/spec/lib/gitlab/git/diff_collection_spec.rb @@ -341,8 +341,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do end context 'when diff is quite large will collapse by default' do - let(:iterator) { [{ diff: 'a' * (Gitlab::Git::Diff.collapse_limit + 1) }] } - let(:max_files) { 100 } + let(:iterator) { [{ diff: 'a' * 20480 }] } context 'when no collapse is set' do let(:expanded) { true } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index d39b33a0c05..4a7b06003fc 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -31,36 +31,6 @@ EOT [".gitmodules"]).patches.first end - describe 'size limit feature toggles' do - context 'when the feature gitlab_git_diff_size_limit_increase is enabled' do - before do - stub_feature_flags(gitlab_git_diff_size_limit_increase: true) - end - - it 'returns 200 KB for size_limit' do - expect(described_class.size_limit).to eq(200.kilobytes) - end - - it 'returns 100 KB for collapse_limit' do - expect(described_class.collapse_limit).to eq(100.kilobytes) - end - end - - context 'when the feature gitlab_git_diff_size_limit_increase is disabled' do - before do - stub_feature_flags(gitlab_git_diff_size_limit_increase: false) - end - - it 'returns 100 KB for size_limit' do - expect(described_class.size_limit).to eq(100.kilobytes) - end - - it 'returns 10 KB for collapse_limit' do - expect(described_class.collapse_limit).to eq(10.kilobytes) - end - end - end - describe '.new' do context 'using a Hash' do context 'with a small diff' do @@ -77,7 +47,7 @@ EOT context 'using a diff that is too large' do it 'prunes the diff' do - diff = described_class.new(diff: 'a' * (described_class.size_limit + 1)) + diff = described_class.new(diff: 'a' * 204800) expect(diff.diff).to be_empty expect(diff).to be_too_large @@ -115,8 +85,8 @@ EOT # The patch total size is 200, with lines between 21 and 54. # This is a quick-and-dirty way to test this. Ideally, a new patch is # added to the test repo with a size that falls between the real limits. - allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(150) - allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(100) + stub_const("#{described_class}::SIZE_LIMIT", 150) + stub_const("#{described_class}::COLLAPSE_LIMIT", 100) end it 'prunes the diff as a large diff instead of as a collapsed diff' do @@ -356,7 +326,7 @@ EOT describe '#collapsed?' do it 'returns true for a diff that is quite large' do - diff = described_class.new({ diff: 'a' * (described_class.collapse_limit + 1) }, expanded: false) + diff = described_class.new({ diff: 'a' * 20480 }, expanded: false) expect(diff).to be_collapsed end