Commit d9774257 authored by Robert Speicher's avatar Robert Speicher

Increase the minimum length for commit SHA matching to 7

This is the git default and will help to prevent false positive matches.

Closes #12706
parent dec21517
......@@ -68,18 +68,18 @@ class Commit
# Pattern used to extract commit references from text
#
# The SHA can be between 6 and 40 hex characters.
# The SHA can be between 7 and 40 hex characters.
#
# This pattern supports cross-project references.
def self.reference_pattern
%r{
(?:#{Project.reference_pattern}#{reference_prefix})?
(?<commit>\h{6,40})
(?<commit>\h{7,40})
}x
end
def self.link_reference_pattern
super("commit", /(?<commit>\h{6,40})/)
super("commit", /(?<commit>\h{7,40})/)
end
def to_reference(from_project = nil)
......
......@@ -32,8 +32,8 @@ class CommitRange
PATTERN = /#{REF_PATTERN}\.{2,3}#{REF_PATTERN}/
# In text references, the beginning and ending refs can only be SHAs
# between 6 and 40 hex characters.
STRICT_PATTERN = /\h{6,40}\.{2,3}\h{6,40}/
# between 7 and 40 hex characters.
STRICT_PATTERN = /\h{7,40}\.{2,3}\h{7,40}/
def self.reference_prefix
'@'
......
......@@ -490,7 +490,7 @@ Rails.application.routes.draw do
end
resource :avatar, only: [:show, :destroy]
resources :commit, only: [:show], constraints: { id: /[[:alnum:]]{6,40}/ } do
resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
member do
get :branches
get :builds
......
......@@ -21,7 +21,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do
let(:reference) { commit.id }
# Let's test a variety of commit SHA sizes just to be paranoid
[6, 8, 12, 18, 20, 32, 40].each do |size|
[7, 8, 12, 18, 20, 32, 40].each do |size|
it "links to a valid reference of #{size} characters" do
doc = reference_filter("See #{reference[0...size]}")
......@@ -35,7 +35,7 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do
doc = reference_filter("See #{commit.id}")
expect(doc.text).to eq "See #{commit.short_id}"
doc = reference_filter("See #{commit.id[0...6]}")
doc = reference_filter("See #{commit.id[0...7]}")
expect(doc.text).to eq "See #{commit.short_id}"
end
......
......@@ -321,12 +321,12 @@ describe Projects::HooksController, 'routing' do
end
end
# project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /[[:alnum:]]{6,40}/, project_id: /[^\/]+/}
# project_commit GET /:project_id/commit/:id(.:format) commit#show {id: /\h{7,40}/, project_id: /[^\/]+/}
describe Projects::CommitController, 'routing' do
it 'to #show' do
expect(get('/gitlab/gitlabhq/commit/4246fb')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb')
expect(get('/gitlab/gitlabhq/commit/4246fb.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'diff')
expect(get('/gitlab/gitlabhq/commit/4246fb.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fb', format: 'patch')
expect(get('/gitlab/gitlabhq/commit/4246fbd')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd')
expect(get('/gitlab/gitlabhq/commit/4246fbd.diff')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'diff')
expect(get('/gitlab/gitlabhq/commit/4246fbd.patch')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd', format: 'patch')
expect(get('/gitlab/gitlabhq/commit/4246fbd13872934f72a8fd0d6fb1317b47b59cb5')).to route_to('projects/commit#show', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '4246fbd13872934f72a8fd0d6fb1317b47b59cb5')
end
end
......
......@@ -67,9 +67,9 @@ module FilterSpecHelper
if reference =~ /\A(.+)?.\d+\z/
# Integer-based reference with optional project prefix
reference.gsub(/\d+\z/) { |i| i.to_i + 1 }
elsif reference =~ /\A(.+@)?(\h{6,40}\z)/
elsif reference =~ /\A(.+@)?(\h{7,40}\z)/
# SHA-based reference with optional prefix
reference.gsub(/\h{6,40}\z/) { |v| v.reverse }
reference.gsub(/\h{7,40}\z/) { |v| v.reverse }
else
reference.gsub(/\w+\z/) { |v| v.reverse }
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