Commit ca87a513 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin

Return an error for an invalid ref_name

* Contributes to https://gitlab.com/gitlab-org/gitlab/-/issues/351520
* Sentry error: https://sentry.gitlab.net/gitlab/gitlabcom/issues/3173142/

**Problem**

Git ref cannot start with `-`. Currently we return an ArgumentError
from the Gitaly.

**Solution**

Verify the ref_name with regexp

Changelog: fixed
parent e1a881c1
...@@ -32,7 +32,7 @@ module API ...@@ -32,7 +32,7 @@ module API
success Entities::Commit success Entities::Commit
end end
params do params do
optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used' optional :ref_name, type: String, desc: 'The name of a repository branch or tag, if not given the default branch is used', regexp: /\A#{Gitlab::PathRegex.git_reference_regex}\z|\A\z/
optional :since, type: DateTime, desc: 'Only commits after or on this date will be returned' optional :since, type: DateTime, desc: 'Only commits after or on this date will be returned'
optional :until, type: DateTime, desc: 'Only commits before or on this date will be returned' optional :until, type: DateTime, desc: 'Only commits before or on this date will be returned'
optional :path, type: String, desc: 'The file path' optional :path, type: String, desc: 'The file path'
......
...@@ -234,7 +234,7 @@ module Gitlab ...@@ -234,7 +234,7 @@ module Gitlab
@git_reference_regex ||= single_line_regexp %r{ @git_reference_regex ||= single_line_regexp %r{
(?! (?!
(?# doesn't begins with) (?# doesn't begins with)
\/| (?# rule #6) \/|-| (?# rule #6)
(?# doesn't contain) (?# doesn't contain)
.*(?: .*(?:
[\/.]\.| (?# rule #1,3) [\/.]\.| (?# rule #1,3)
......
...@@ -530,6 +530,18 @@ RSpec.describe Gitlab::PathRegex do ...@@ -530,6 +530,18 @@ RSpec.describe Gitlab::PathRegex do
it { is_expected.not_to match('snippets/1.wiki.git') } it { is_expected.not_to match('snippets/1.wiki.git') }
end end
describe '.git_reference_regex' do
subject { %r{\A#{described_class.git_reference_regex}\z} }
it { is_expected.to match('main') }
it { is_expected.to match('v1.2.3') }
it { is_expected.to match('refs/heads/main') }
it { is_expected.to match('1-2-3') }
it { is_expected.to match('1-----') }
it { is_expected.not_to match('-main') }
it { is_expected.not_to match('') }
end
describe '.full_snippets_repository_path_regex' do describe '.full_snippets_repository_path_regex' do
subject { described_class.full_snippets_repository_path_regex } subject { described_class.full_snippets_repository_path_regex }
......
...@@ -127,6 +127,14 @@ RSpec.describe API::Commits do ...@@ -127,6 +127,14 @@ RSpec.describe API::Commits do
it_behaves_like 'project commits' it_behaves_like 'project commits'
end end
context 'with incorrect ref_name parameter' do
let(:route) { "/projects/#{project_id}/repository/commits?ref_name=-main" }
it_behaves_like '400 response' do
let(:request) { get api(route, user) }
end
end
context "path optional parameter" do context "path optional parameter" do
it "returns project commits matching provided path parameter" do it "returns project commits matching provided path parameter" do
path = 'files/ruby/popen.rb' path = 'files/ruby/popen.rb'
......
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