Commit 04038e1d authored by Bola Ahmed Buari's avatar Bola Ahmed Buari Committed by Nick Thomas

Add validation to Conan recipe

The username, package name, channel and  version that makes up the Conan recipe is validated using the official conan.io regex from https://docs.conan.io/en/latest/reference/conanfile/attributes.html

This [prevent problems when working with the GitLab registry.

The regex matches a minimum of 2 and a maximum of 50 characters,  starts with alphanumeric or underscore, then alphanumeric, underscore, +, ., - characters.

Tests are added to check the cases listed above.

Fix #214471
parent 2d41405b
...@@ -20,10 +20,9 @@ class Packages::Package < ApplicationRecord ...@@ -20,10 +20,9 @@ class Packages::Package < ApplicationRecord
delegate :recipe, :recipe_path, to: :conan_metadatum, prefix: :conan delegate :recipe, :recipe_path, to: :conan_metadatum, prefix: :conan
validates :project, presence: true validates :project, presence: true
validates :name, presence: true
validates :name, validates :name, format: { with: Gitlab::Regex.package_name_regex }, unless: :conan?
presence: true,
format: { with: Gitlab::Regex.package_name_regex }
validates :name, validates :name,
uniqueness: { scope: %i[project_id version package_type] }, unless: :conan? uniqueness: { scope: %i[project_id version package_type] }, unless: :conan?
...@@ -32,6 +31,8 @@ class Packages::Package < ApplicationRecord ...@@ -32,6 +31,8 @@ class Packages::Package < ApplicationRecord
validate :valid_npm_package_name, if: :npm? validate :valid_npm_package_name, if: :npm?
validate :package_already_taken, if: :npm? validate :package_already_taken, if: :npm?
validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm? validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm?
validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5, composer: 6 } enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5, composer: 6 }
......
---
title: Add validation to Conan recipe that conforms with Conan.io
merge_request: 29739
author: Bola Ahmed Buari
type: fixed
...@@ -20,7 +20,7 @@ module EE ...@@ -20,7 +20,7 @@ module EE
end end
def conan_recipe_component_regex def conan_recipe_component_regex
@conan_recipe_component_regex ||= %r{\A(\w[.+-]?)+\z}.freeze @conan_recipe_component_regex ||= %r{\A[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}\z}.freeze
end end
def package_name_regex def package_name_regex
......
...@@ -33,13 +33,20 @@ describe Gitlab::Regex do ...@@ -33,13 +33,20 @@ describe Gitlab::Regex do
describe '.conan_recipe_component_regex' do describe '.conan_recipe_component_regex' do
subject { described_class.conan_recipe_component_regex } subject { described_class.conan_recipe_component_regex }
let(:fifty_one_characters) { 'f_a' * 17}
it { is_expected.to match('foobar') } it { is_expected.to match('foobar') }
it { is_expected.to match('foo_bar') } it { is_expected.to match('foo_bar') }
it { is_expected.to match('foo+bar') } it { is_expected.to match('foo+bar') }
it { is_expected.to match('_foo+bar-baz+1.0') }
it { is_expected.to match('1.0.0') } it { is_expected.to match('1.0.0') }
it { is_expected.not_to match('-foo_bar') }
it { is_expected.not_to match('+foo_bar') }
it { is_expected.not_to match('.foo_bar') }
it { is_expected.not_to match('foo@bar') } it { is_expected.not_to match('foo@bar') }
it { is_expected.not_to match('foo/bar') } it { is_expected.not_to match('foo/bar') }
it { is_expected.not_to match('!!()()') } it { is_expected.not_to match('!!()()') }
it { is_expected.not_to match(fifty_one_characters) }
end end
describe '.feature_flag_regex' do describe '.feature_flag_regex' do
......
...@@ -8,12 +8,22 @@ RSpec.describe Packages::ConanMetadatum, type: :model do ...@@ -8,12 +8,22 @@ RSpec.describe Packages::ConanMetadatum, type: :model do
end end
describe 'validations' do describe 'validations' do
let(:fifty_one_characters) { 'f_a' * 17}
it { is_expected.to validate_presence_of(:package) } it { is_expected.to validate_presence_of(:package) }
it { is_expected.to validate_presence_of(:package_username) } it { is_expected.to validate_presence_of(:package_username) }
it { is_expected.to validate_presence_of(:package_channel) } it { is_expected.to validate_presence_of(:package_channel) }
describe '#package_username' do describe '#package_username' do
it { is_expected.to allow_value("my-package+username").for(:package_username) } it { is_expected.to allow_value("my-package+username").for(:package_username) }
it { is_expected.to allow_value("my_package.username").for(:package_username) }
it { is_expected.to allow_value("_my-package.username123").for(:package_username) }
it { is_expected.to allow_value("my").for(:package_username) }
it { is_expected.not_to allow_value('+my_package').for(:package_username) }
it { is_expected.not_to allow_value('.my_package').for(:package_username) }
it { is_expected.not_to allow_value('-my_package').for(:package_username) }
it { is_expected.not_to allow_value('m').for(:package_username) }
it { is_expected.not_to allow_value(fifty_one_characters).for(:package_username) }
it { is_expected.not_to allow_value("my/package").for(:package_username) } it { is_expected.not_to allow_value("my/package").for(:package_username) }
it { is_expected.not_to allow_value("my(package)").for(:package_username) } it { is_expected.not_to allow_value("my(package)").for(:package_username) }
it { is_expected.not_to allow_value("my@package").for(:package_username) } it { is_expected.not_to allow_value("my@package").for(:package_username) }
...@@ -22,6 +32,14 @@ RSpec.describe Packages::ConanMetadatum, type: :model do ...@@ -22,6 +32,14 @@ RSpec.describe Packages::ConanMetadatum, type: :model do
describe '#package_channel' do describe '#package_channel' do
it { is_expected.to allow_value("beta").for(:package_channel) } it { is_expected.to allow_value("beta").for(:package_channel) }
it { is_expected.to allow_value("stable+1.0").for(:package_channel) } it { is_expected.to allow_value("stable+1.0").for(:package_channel) }
it { is_expected.to allow_value("my").for(:package_channel) }
it { is_expected.to allow_value("my_channel.beta").for(:package_channel) }
it { is_expected.to allow_value("_my-channel.beta123").for(:package_channel) }
it { is_expected.not_to allow_value('+my_channel').for(:package_channel) }
it { is_expected.not_to allow_value('.my_channel').for(:package_channel) }
it { is_expected.not_to allow_value('-my_channel').for(:package_channel) }
it { is_expected.not_to allow_value('m').for(:package_channel) }
it { is_expected.not_to allow_value(fifty_one_characters).for(:package_channel) }
it { is_expected.not_to allow_value("my/channel").for(:package_channel) } it { is_expected.not_to allow_value("my/channel").for(:package_channel) }
it { is_expected.not_to allow_value("my(channel)").for(:package_channel) } it { is_expected.not_to allow_value("my(channel)").for(:package_channel) }
it { is_expected.not_to allow_value("my@channel").for(:package_channel) } it { is_expected.not_to allow_value("my@channel").for(:package_channel) }
......
...@@ -78,6 +78,20 @@ RSpec.describe Packages::Package, type: :model do ...@@ -78,6 +78,20 @@ RSpec.describe Packages::Package, type: :model do
it { is_expected.to allow_value("my/domain/com/my-app").for(:name) } it { is_expected.to allow_value("my/domain/com/my-app").for(:name) }
it { is_expected.to allow_value("my.app-11.07.2018").for(:name) } it { is_expected.to allow_value("my.app-11.07.2018").for(:name) }
it { is_expected.not_to allow_value("my(dom$$$ain)com.my-app").for(:name) } it { is_expected.not_to allow_value("my(dom$$$ain)com.my-app").for(:name) }
context 'conan package' do
subject { create(:conan_package) }
let(:fifty_one_characters) {'f_b' * 17}
it { is_expected.to allow_value('foo+bar').for(:name) }
it { is_expected.to allow_value('foo_bar').for(:name) }
it { is_expected.to allow_value('foo.bar').for(:name) }
it { is_expected.not_to allow_value(fifty_one_characters).for(:name) }
it { is_expected.not_to allow_value('+foobar').for(:name) }
it { is_expected.not_to allow_value('.foobar').for(:name) }
it { is_expected.not_to allow_value('%foo%bar').for(:name) }
end
end end
describe '#version' do describe '#version' do
...@@ -93,6 +107,22 @@ RSpec.describe Packages::Package, type: :model do ...@@ -93,6 +107,22 @@ RSpec.describe Packages::Package, type: :model do
it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) } it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) }
it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) } it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
end end
context 'conan package' do
subject { create(:conan_package) }
let(:fifty_one_characters) {'1.2' * 17}
it { is_expected.to allow_value('1.2').for(:version) }
it { is_expected.to allow_value('1.2.3-beta').for(:version) }
it { is_expected.to allow_value('1.2.3-pre1+build2').for(:version) }
it { is_expected.not_to allow_value('1').for(:version) }
it { is_expected.not_to allow_value(fifty_one_characters).for(:version) }
it { is_expected.not_to allow_value('1./2.3').for(:version) }
it { is_expected.not_to allow_value('.1.2.3').for(:version) }
it { is_expected.not_to allow_value('+1.2.3').for(:version) }
it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
end
end end
describe '#package_already_taken' do describe '#package_already_taken' do
......
...@@ -31,7 +31,7 @@ describe Packages::Npm::CreateTagService do ...@@ -31,7 +31,7 @@ describe Packages::Npm::CreateTagService do
end end
context 'on different package type' do context 'on different package type' do
let!(:package2) { create(:conan_package, project: package.project, name: package.name, version: package.version) } let!(:package2) { create(:conan_package, project: package.project, name: 'conan_package_name', version: package.version) }
it_behaves_like 'it creates the tag' it_behaves_like 'it creates the tag'
end 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