From d97742570fce0512eddb6dc6c924d541aa4f57e4 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 29 Jan 2016 19:49:07 -0500 Subject: 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 --- app/models/commit.rb | 6 +++--- app/models/commit_range.rb | 4 ++-- config/routes.rb | 2 +- spec/lib/banzai/filter/commit_reference_filter_spec.rb | 4 ++-- spec/routing/project_routing_spec.rb | 8 ++++---- spec/support/filter_spec_helper.rb | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index 0ba7b584d91..23b771aebb7 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -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})? - (?\h{6,40}) + (?\h{7,40}) }x end def self.link_reference_pattern - super("commit", /(?\h{6,40})/) + super("commit", /(?\h{7,40})/) end def to_reference(from_project = nil) diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index 14e7971fa06..289dbc57287 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -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 '@' diff --git a/config/routes.rb b/config/routes.rb index fdfdb449085..54cc338b605 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 473534ba68a..63a32d9d455 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -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 diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 22937226fce..538f44e4f3f 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -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 diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index d6e03cbef3d..ef5ea7d626e 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -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 -- cgit v1.2.3