diff --git a/changelogs/unreleased/sh-validate-ref-name-in-commit.yml b/changelogs/unreleased/sh-validate-ref-name-in-commit.yml new file mode 100644 index 00000000000..399529556bc --- /dev/null +++ b/changelogs/unreleased/sh-validate-ref-name-in-commit.yml @@ -0,0 +1,5 @@ +--- +title: Validate refs used in controllers don't have spaces +merge_request: 24037 +author: +type: other diff --git a/lib/extracts_path.rb b/lib/extracts_path.rb index b2c8d46ede1..44a9c7ea536 100644 --- a/lib/extracts_path.rb +++ b/lib/extracts_path.rb @@ -113,6 +113,9 @@ module ExtractsPath @id = get_id @ref, @path = extract_ref(@id) @repo = @project.repository + @ref.strip! + + raise InvalidPathError if @ref.match?(/\s/) @commit = @repo.commit(@ref) diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index e1d8719d8d6..7f7cabe3b0c 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -74,6 +74,26 @@ describe Projects::TreeController do end end + describe 'GET show with whitespace in ref' do + render_views + + let(:id) { "this ref/api/responses" } + + it 'does not call make a Gitaly request' do + allow(::Gitlab::GitalyClient).to receive(:call).and_call_original + expect(::Gitlab::GitalyClient).not_to receive(:call).with(anything, :commit_service, :find_commit, anything, anything) + + get(:show, + params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: id + }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + describe 'GET show with blob path' do render_views diff --git a/spec/lib/extracts_path_spec.rb b/spec/lib/extracts_path_spec.rb index e0691aba600..21ba72953fb 100644 --- a/spec/lib/extracts_path_spec.rb +++ b/spec/lib/extracts_path_spec.rb @@ -44,6 +44,36 @@ describe ExtractsPath do end end + context 'ref contains trailing space' do + let(:ref) { 'master ' } + + it 'strips surrounding space' do + assign_ref_vars + + expect(@ref).to eq('master') + end + end + + context 'ref contains leading space' do + let(:ref) { ' master ' } + + it 'strips surrounding space' do + assign_ref_vars + + expect(@ref).to eq('master') + end + end + + context 'ref contains space in the middle' do + let(:ref) { 'master plan ' } + + it 'returns 404' do + expect(self).to receive(:render_404) + + assign_ref_vars + end + end + context 'path contains space' do let(:params) { { path: 'with space', ref: '38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e' } }