Commit b9857d8b authored by Douwe Maan's avatar Douwe Maan

Properly compare diff refs and diff positions when shas are truncated

parent c19d5ac1
......@@ -26,7 +26,8 @@ class Commit
DIFF_HARD_LIMIT_LINES = 50000
# The SHA can be between 7 and 40 hex characters.
COMMIT_SHA_PATTERN = '\h{7,40}'.freeze
MIN_SHA_LENGTH = 7
COMMIT_SHA_PATTERN = /\h{#{MIN_SHA_LENGTH},40}/.freeze
def banzai_render_context(field)
context = { pipeline: :single_line, project: self.project }
......@@ -53,7 +54,7 @@ class Commit
# Truncate sha to 8 characters
def truncate_sha(sha)
sha[0..7]
sha[0..MIN_SHA_LENGTH]
end
def max_diff_options
......@@ -100,7 +101,7 @@ class Commit
def self.reference_pattern
@reference_pattern ||= %r{
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit>\h{7,40})
(?<commit>#{COMMIT_SHA_PATTERN})
}x
end
......@@ -216,9 +217,9 @@ class Commit
@raw.respond_to?(method, include_private) || super
end
# Truncate sha to 8 characters
# Truncate sha to 7 characters
def short_id
@raw.short_id(7)
@raw.short_id(MIN_SHA_LENGTH)
end
def diff_refs
......
......@@ -13,9 +13,9 @@ module Gitlab
def ==(other)
other.is_a?(self.class) &&
base_sha == other.base_sha &&
start_sha == other.start_sha &&
head_sha == other.head_sha
shas_equal?(base_sha, other.base_sha) &&
shas_equal?(start_sha, other.start_sha) &&
shas_equal?(head_sha, other.head_sha)
end
alias_method :eql?, :==
......@@ -47,6 +47,22 @@ module Gitlab
CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
end
end
private
def shas_equal?(sha1, sha2)
return true if sha1 == sha2
return false if sha1.nil? || sha2.nil?
return false unless sha1.class == sha2.class
length = [sha1.length, sha2.length].min
# If either of the shas is below 7 characters in length, we cannot be sure
# if they actually refer to the same commit because of hash collision.
return false if length < Commit::MIN_SHA_LENGTH
sha1[0, length] == sha2[0, length]
end
end
end
end
......@@ -49,12 +49,13 @@ module Gitlab
coder['attributes'] = self.to_h
end
def key
@key ||= [base_sha, start_sha, head_sha, Digest::SHA1.hexdigest(old_path || ""), Digest::SHA1.hexdigest(new_path || ""), old_line, new_line]
end
def ==(other)
other.is_a?(self.class) && key == other.key
other.is_a?(self.class) &&
other.diff_refs == diff_refs &&
other.old_path == old_path &&
other.new_path == new_path &&
other.old_line == old_line &&
other.new_line == new_line
end
def to_h
......
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