Commit 5f651cef authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch 'osw-configurable-single-diff-file-limit' into 'master'

Make single diff patch limit configurable

Closes #48027

See merge request gitlab-org/gitlab-ce!21886
......@@ -254,7 +254,8 @@ def visible_attributes
......@@ -182,6 +182,12 @@ class ApplicationSetting < ActiveRecord::Base
numericality: { less_than_or_equal_to: :gitaly_timeout_default },
if: :gitaly_timeout_default
validates :diff_max_patch_bytes,
presence: true,
numericality: { only_integer: true,
greater_than_or_equal_to: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES,
less_than_or_equal_to: Gitlab::Git::Diff::MAX_PATCH_BYTES_UPPER_BOUND }
validates :user_default_internal_regex, js_regex: true, allow_nil: true
SUPPORTED_KEY_TYPES.each do |type|
......@@ -293,7 +299,8 @@ def self.defaults
user_default_external: false,
user_default_internal_regex: nil,
user_show_add_ssh_key_message: true,
usage_stats_set_by_user_id: nil
usage_stats_set_by_user_id: nil,
diff_max_patch_bytes: Gitlab::Git::Diff::DEFAULT_MAX_PATCH_BYTES
= form_for @application_setting, url: admin_application_settings_path(anchor: 'js-merge-request-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
= f.label :diff_max_patch_bytes, 'Maximum diff patch size (Bytes)', class: 'label-light'
= f.number_field :diff_max_patch_bytes, class: 'form-control'
Diff files surpassing this limit will be presented as 'too large'
and won't be expandable.
= link_to icon('question-circle'),
anchor: 'maximum-diff-patch-size')
= f.submit _('Save changes'), class: 'btn btn-success'
......@@ -24,6 +24,17 @@
= render 'account_and_limit'{ class: ('expanded' if expanded_by_default?) }
= _('Diff limits')
%button.btn.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
= _('Diff content limits')
= render 'diff_limits'{ class: ('expanded' if expanded_by_default?) }
title: Make single diff patch limit configurable
merge_request: 21886
type: added
# frozen_string_literal: true
# See
# for more information on how to write migrations for GitLab.
class AddDiffMaxPatchBytesToApplicationSettings < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
def up
default: 100.kilobytes,
allow_null: false)
def down
remove_column(:application_settings, :diff_max_patch_bytes)
......@@ -11,7 +11,7 @@
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20180917172041) do
ActiveRecord::Schema.define(version: 20180924141949) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -171,6 +171,7 @@
t.boolean "user_show_add_ssh_key_message", default: true, null: false
t.integer "usage_stats_set_by_user_id"
t.integer "receive_max_input_size"
t.integer "diff_max_patch_bytes", default: 102400, null: false
create_table "audit_events", force: :cascade do |t|
......@@ -47,6 +47,7 @@ Learn how to install, configure, update, and maintain your GitLab instance.
- [Enforcing Terms of Service](../user/admin_area/settings/
- [Third party offers](../user/admin_area/settings/
- [Compliance]( A collection of features from across the application that you may configure to help ensure that your GitLab instance and DevOps workflow meet compliance standards.
- [Diff limits](../user/admin_area/ Configure the diff rendering size limits of branch comparison pages.
#### Customizing GitLab's appearance
......@@ -2,13 +2,10 @@
Currently we rely on different sources to present diffs, these include:
- Rugged gem
- Gitaly service
- Database (through `merge_request_diff_files`)
- Redis (cached highlighted diffs)
We're constantly moving Rugged calls to Gitaly and the progress can be followed through [Gitaly repo](
## Architecture overview
### Merge request diffs
......@@ -19,8 +16,9 @@ we fetch the comparison information using `Gitlab::Git::Compare`, which fetches
The diffs fetching process _limits_ single file diff sizes and the overall size of the whole diff through a series of constant values. Raw diff files are
then persisted on `merge_request_diff_files` table.
Even though diffs higher than 10kb are collapsed (`Gitlab::Git::Diff::COLLAPSE_LIMIT`), we still keep them on Postgres. However, diff files over _safety limits_
(see the [Diff limits section](#diff-limits)) are _not_ persisted.
Even though diffs larger than 10% of the value of `ApplicationSettings#diff_max_patch_bytes` are collapsed,
we still keep them on Postgres. However, diff files larger than defined _safety limits_
(see the [Diff limits section](#diff-limits)) are _not_ persisted in the database.
In order to present diffs information on the Merge Request diffs page, we:
......@@ -102,23 +100,20 @@ Gitaly will only return the safe amount of data to be persisted on `merge_reques
Limits that act onto each diff file of a collection. Files number, lines number and files size are considered.
Gitlab::Git::Diff::COLLAPSE_LIMIT = 10.kilobytes
#### Expandable patches (collapsed)
File diff will be collapsed (but be expandable) if it is larger than 10 kilobytes.
Diff patches are collapsed when surpassing 10% of the value set in `ApplicationSettings#diff_max_patch_bytes`.
That is, it's equivalent to 10kb if the maximum allowed value is 100kb.
The diff will still be persisted and expandable if the patch size doesn't
surpass `ApplicationSettings#diff_max_patch_bytes`.
*Note:* Although this nomenclature (Collapsing) is also used on Gitaly, this limit is only used on GitLab (hardcoded - not sent to Gitaly).
Gitaly will only return `Diff.Collapsed` (RPC) when surpassing collection limits.
Gitlab::Git::Diff::SIZE_LIMIT = 100.kilobytes
File diff will not be rendered if it's larger than 100 kilobytes.
#### Not expandable patches (too large)
*Note:* This limit is currently hardcoded and applied on Gitaly and the RPC returns `Diff.TooLarge` when this limit is surpassed.
Although we're still also applying it on GitLab, we should remove the redundancy from GitLab once we're confident with the Gitaly integration.
The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`.
Users will see a `This source diff could not be displayed because it is too large` message.
Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000
# Diff limits administration
NOTE: **Note:**
Merge requests and branch comparison views will be affected.
CAUTION: **Caution:**
These settings are currently under experimental state. They'll
increase the resource consumption of your instance and should
be edited mindfully.
1. Access **Admin area > Settings > General**
1. Expand **Diff limits**
### Maximum diff patch size
This is the content size each diff file (patch) is allowed to reach before
it's collapsed, without the possibility of being expanded. A link redirecting
to the blob view will be presented for the patches that surpass this limit.
Patches surpassing 10% of this content size will be automatically collapsed,
but expandable (a link to expand the diff will be presented).
......@@ -19,13 +19,17 @@ class Diff
alias_method :expanded?, :expanded
SERIALIZE_KEYS = %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large).freeze
# The default maximum content size to display a diff patch.
# If this value ever changes, make sure to create a migration to update
# current records, and default of `ApplicationSettings#diff_max_patch_bytes`.
# The maximum size of a diff to display.
SIZE_LIMIT = 100.kilobytes
# This is a limitation applied on the source (Gitaly), therefore we don't allow
# persisting limits over that.
# The maximum size before a diff is collapsed.
COLLAPSE_LIMIT = 10.kilobytes
SERIALIZE_KEYS = %i(diff new_path old_path a_mode b_mode new_file renamed_file deleted_file too_large).freeze
class << self
def between(repo, head, base, options = {}, *paths)
......@@ -105,6 +109,26 @@ def filter_diff_options(options, default_options = {})
def binary_message(old_path, new_path)
"Binary files #{old_path} and #{new_path} differ\n"
# Returns the limit of bytes a single diff file can reach before it
# appears as 'collapsed' for end-users.
# By convention, it's 10% of the persisted `diff_max_patch_bytes`.
# Example: If we have 100k for the `diff_max_patch_bytes`, it will be 10k by
# default.
# Patches surpassing this limit should still be persisted in the database.
def patch_safe_limit_bytes
patch_hard_limit_bytes / 10
# Returns the limit for a single diff file (patch).
# Patches surpassing this limit shouldn't be persisted in the database
# and will be presented as 'too large' for end-users.
def patch_hard_limit_bytes
def initialize(raw_diff, expanded: true)
......@@ -150,7 +174,7 @@ def line_count
def too_large?
if @too_large.nil?
@too_large = @diff.bytesize >= SIZE_LIMIT
@too_large = @diff.bytesize >= self.class.patch_hard_limit_bytes
......@@ -168,7 +192,7 @@ def too_large!
def collapsed?
return @collapsed if defined?(@collapsed)
@collapsed = !expanded && @diff.bytesize >= COLLAPSE_LIMIT
@collapsed = !expanded && @diff.bytesize >= self.class.patch_safe_limit_bytes
def collapse!
......@@ -219,30 +243,6 @@ def prune_diff_if_eligible
# If the patch surpasses any of the diff limits it calls the appropiate
# prune method and returns true. Otherwise returns false.
def prune_large_patch(patch)
size = 0
patch.each_hunk do |hunk|
hunk.each_line do |line|
size += line.content.bytesize
if size >= SIZE_LIMIT
return true # rubocop:disable Cop/AvoidReturnFromBlocks
if !expanded && size >= COLLAPSE_LIMIT
return true
......@@ -19,7 +19,7 @@ def self.limits(options = {})
limits[:safe_max_files] = [limits[:max_files], DEFAULT_LIMITS[:max_files]].min
limits[:safe_max_lines] = [limits[:max_lines], DEFAULT_LIMITS[:max_lines]].min
limits[:safe_max_bytes] = limits[:safe_max_files] * 5.kilobytes # Average 5 KB per file
limits[:max_patch_bytes] = Gitlab::Git::Diff::SIZE_LIMIT
limits[:max_patch_bytes] = Gitlab::Git::Diff.patch_hard_limit_bytes
......@@ -2292,6 +2292,12 @@ msgstr ""
msgid "Details"
msgstr ""
msgid "Diff content limits"
msgstr ""
msgid "Diff limits"
msgstr ""
msgid "Diffs|No file name available"
msgstr ""
......@@ -592,4 +592,19 @@ def expect_invalid
it { eq(result) }
context 'diff limit settings' do
describe '#diff_max_patch_bytes' do
context 'validations' do
it { validate_presence_of(:diff_max_patch_bytes) }
it do validate_numericality_of(:diff_max_patch_bytes)
......@@ -66,7 +66,8 @@
enforce_terms: true,
terms: 'Hello world!',
performance_bar_allowed_group_path: group.full_path,
instance_statistics_visibility_private: true
instance_statistics_visibility_private: true,
diff_max_patch_bytes: 150_000
expect(response).to have_gitlab_http_status(200)
expect(json_response['default_projects_limit']).to eq(3)
......@@ -92,6 +93,7 @@
expect(json_response['terms']).to eq('Hello world!')
expect(json_response['performance_bar_allowed_group_id']).to eq(
expect(json_response['instance_statistics_visibility_private']).to be(true)
expect(json_response['diff_max_patch_bytes']).to eq(150_000)
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