diff options
author | Stan Hu <stanhu@gmail.com> | 2016-10-08 08:48:23 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2016-10-10 23:05:09 +0300 |
commit | d4fab17d7c8c2b233248295755a6277fdee09c9f (patch) | |
tree | 4ee583bb9ef541d5e6975d9e36b5b22ffec8b763 /app/models/merge_request_diff.rb | |
parent | a5cd9c9a596c4160bbdc7645f57266655488386c (diff) |
Fix Error 500 when viewing old merge requests with bad diff data
Customers running old versions of GitLab may have MergeRequestDiffs with
the text ["--broken diff"] due to text generated by gitlab_git 1.0.3.
To avoid the Error 500, verify that each element is a type that gitlab_git
will accept before attempting to create a DiffCollection.
Closes #20776
Diffstat (limited to 'app/models/merge_request_diff.rb')
-rw-r--r-- | app/models/merge_request_diff.rb | 14 |
1 files changed, 13 insertions, 1 deletions
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 36b8b70870b..3f7e96186a1 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -6,6 +6,9 @@ class MergeRequestDiff < ActiveRecord::Base # Prevent store of diff if commits amount more then 500 COMMITS_SAFE_SIZE = 100 + # Valid types of serialized diffs allowed by Gitlab::Git::Diff + VALID_CLASSES = [Hash, Rugged::Patch, Rugged::Diff::Delta] + belongs_to :merge_request state_machine :state, initial: :empty do @@ -170,6 +173,15 @@ class MergeRequestDiff < ActiveRecord::Base private + # Old GitLab implementations may have generated diffs as ["--broken-diff"]. + # Avoid an error 500 by ignoring bad elements. See: + # https://gitlab.com/gitlab-org/gitlab-ce/issues/20776 + def valid_raw_diff?(raw) + return false unless raw.respond_to?(:each) + + raw.any? { |element| VALID_CLASSES.include?(element.class) } + end + def dump_commits(commits) commits.map(&:to_hash) end @@ -200,7 +212,7 @@ class MergeRequestDiff < ActiveRecord::Base end def load_diffs(raw, options) - if raw.respond_to?(:each) + if valid_raw_diff?(raw) if paths = options[:paths] raw = raw.select do |diff| paths.include?(diff[:old_path]) || paths.include?(diff[:new_path]) |