Commit f9902c18 authored by Pavlo Strokov's avatar Pavlo Strokov

Add verification of commit id before calling remote

It makes no sense to make a call to remote service
in case commit id contains unsupported characters.
parent c9d7d0be
...@@ -57,11 +57,8 @@ module Gitlab ...@@ -57,11 +57,8 @@ module Gitlab
# Already a commit? # Already a commit?
return commit_id if commit_id.is_a?(Gitlab::Git::Commit) return commit_id if commit_id.is_a?(Gitlab::Git::Commit)
# Some weird thing?
return unless commit_id.is_a?(String)
# This saves us an RPC round trip. # This saves us an RPC round trip.
return if commit_id.include?(':') return unless valid?(commit_id)
commit = find_commit(repo, commit_id) commit = find_commit(repo, commit_id)
...@@ -431,6 +428,15 @@ module Gitlab ...@@ -431,6 +428,15 @@ module Gitlab
def fetch_body_from_gitaly def fetch_body_from_gitaly
self.class.get_message(@repository, id) self.class.get_message(@repository, id)
end end
def self.valid?(commit_id)
commit_id.is_a?(String) && !(
commit_id.start_with?('-') ||
commit_id.include?(':') ||
commit_id.include?("\x00") ||
commit_id.match?(/\s/)
)
end
end end
end end
end end
......
...@@ -161,6 +161,26 @@ describe Gitlab::Git::Commit, :seed_helper do ...@@ -161,6 +161,26 @@ describe Gitlab::Git::Commit, :seed_helper do
expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil expect(described_class.find(repository, "+123_4532530XYZ")).to be_nil
end end
it "returns nil for id started with dash" do
expect(described_class.find(repository, "-HEAD")).to be_nil
end
it "returns nil for id containing colon" do
expect(described_class.find(repository, "HEAD:")).to be_nil
end
it "returns nil for id containing space" do
expect(described_class.find(repository, "HE AD")).to be_nil
end
it "returns nil for id containing tab" do
expect(described_class.find(repository, "HE\tAD")).to be_nil
end
it "returns nil for id containing NULL" do
expect(described_class.find(repository, "HE\x00AD")).to be_nil
end
context 'with broken repo' do context 'with broken repo' do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '', 'group/project') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '', 'group/project') }
......
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