Merge branch 'add-validation-to-pypi-version' into 'master'

Add validation for pypi package

See merge request gitlab-org/gitlab!35080

app/models/packages/package.rb

@@ -37,6 +37,7 @@ class Packages::Package < ApplicationRecord
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.maven_version_regex }, if: -> { version? && maven? }
+  validates :version, format: { with: Gitlab::Regex.pypi_version_regex }, if: :pypi?
 
   enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5, composer: 6 }
 

changelogs/unreleased/35080-add-validation-to-pypi-package-version.yml

+---
+title: Add validation to pypi package version
+merge_request: 35080
+author: Bola Ahmed Buari
+type: added

doc/user/packages/pypi_repository/index.md

@@ -206,6 +206,26 @@ When uploading packages, note that:
 - The maximum allowed size is 50 Megabytes.
 - You cannot upload the same version of a package multiple times. If you try, you receive the error `Validation failed: File name has already been taken`.
 
+### Ensure your version string is valid
+
+If your version string (for example, `0.0.1`) is invalid, it will be rejected. GitLab uses the following regex to validate the version string.
+
+```ruby
+\A(?:
+    v?
+    (?:([0-9]+)!)?                                                 (?# epoch)
+    ([0-9]+(?:\.[0-9]+)*)                                          (?# release segment)
+    ([-_\.]?((a|b|c|rc|alpha|beta|pre|preview))[-_\.]?([0-9]+)?)?  (?# pre-release)
+    ((?:-([0-9]+))|(?:[-_\.]?(post|rev|r)[-_\.]?([0-9]+)?))?       (?# post release)
+    ([-_\.]?(dev)[-_\.]?([0-9]+)?)?                                (?# dev release)
+    (?:\+([a-z0-9]+(?:[-_\.][a-z0-9]+)*))?                         (?# local version)
+)\z}xi
+```
+
+You can play around with the regex and try your version strings on [this regular expression editor](https://rubular.com/r/FKM6d07ouoDaFV).
+
+For more details about the regex used, please check the [documentation here](https://www.python.org/dev/peps/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions))
+
 ### Upload packages with Twine
 
 If you were following the guide above, then the `MyPyPiPackage` package should

lib/gitlab/regex.rb

@@ -51,6 +51,21 @@ module Gitlab
         maven_app_name_regex
       end
 
+      def pypi_version_regex
+        # See the official regex: https://github.com/pypa/packaging/blob/16.7/packaging/version.py#L159
+
+        @pypi_version_regex ||= %r{
+          \A(?:
+            v?
+            (?:([0-9]+)!)?                                                 (?# epoch)
+            ([0-9]+(?:\.[0-9]+)*)                                          (?# release segment)
+            ([-_\.]?((a|b|c|rc|alpha|beta|pre|preview))[-_\.]?([0-9]+)?)?  (?# pre-release)
+            ((?:-([0-9]+))|(?:[-_\.]?(post|rev|r)[-_\.]?([0-9]+)?))?       (?# post release)
+            ([-_\.]?(dev)[-_\.]?([0-9]+)?)?                                (?# dev release)
+            (?:\+([a-z0-9]+(?:[-_\.][a-z0-9]+)*))?                         (?# local version)
+            )\z}xi.freeze
+      end
+
       def unbounded_semver_regex
         # See the official regex: https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
 

spec/factories/packages.rb

@@ -56,14 +56,20 @@ FactoryBot.define do
     end
 
     factory :pypi_package do
-      pypi_metadatum
-
       sequence(:name) { |n| "pypi-package-#{n}"}
       sequence(:version) { |n| "1.0.#{n}" }
       package_type { :pypi }
 
-      after :create do |package|
+      transient do
+        without_loaded_metadatum { false }
+      end
+
+      after :create do |package, evaluator|
         create :package_file, :pypi, package: package, file_name: "#{package.name}-#{package.version}.tar.gz"
+
+        unless evaluator.without_loaded_metadatum
+          create :pypi_metadatum, package: package
+        end
       end
     end
 
@@ -297,7 +303,7 @@ FactoryBot.define do
   end
 
   factory :pypi_metadatum, class: 'Packages::Pypi::Metadatum' do
-    association :package, package_type: :pypi
+    package { create(:pypi_package, without_loaded_metadatum: true) }
     required_python { '>=2.7' }
   end
 

spec/lib/gitlab/regex_spec.rb

@@ -311,6 +311,73 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('%2e%2e%2f1.2.3') }
   end
 
+  describe '.pypi_version_regex' do
+    subject { described_class.pypi_version_regex }
+
+    it { is_expected.to match('0.1') }
+    it { is_expected.to match('2.0') }
+    it { is_expected.to match('1.2.0')}
+    it { is_expected.to match('0100!0.0') }
+    it { is_expected.to match('00!1.2') }
+    it { is_expected.to match('1.0a') }
+    it { is_expected.to match('1.0-a') }
+    it { is_expected.to match('1.0.a1') }
+    it { is_expected.to match('1.0a1') }
+    it { is_expected.to match('1.0-a1') }
+    it { is_expected.to match('1.0alpha1') }
+    it { is_expected.to match('1.0b1') }
+    it { is_expected.to match('1.0beta1') }
+    it { is_expected.to match('1.0rc1') }
+    it { is_expected.to match('1.0pre1') }
+    it { is_expected.to match('1.0preview1') }
+    it { is_expected.to match('1.0.dev1') }
+    it { is_expected.to match('1.0.DEV1') }
+    it { is_expected.to match('1.0.post1') }
+    it { is_expected.to match('1.0.rev1') }
+    it { is_expected.to match('1.0.r1') }
+    it { is_expected.to match('1.0c2') }
+    it { is_expected.to match('2012.15') }
+    it { is_expected.to match('1.0+5') }
+    it { is_expected.to match('1.0+abc.5') }
+    it { is_expected.to match('1!1.1') }
+    it { is_expected.to match('1.0c3') }
+    it { is_expected.to match('1.0rc2') }
+    it { is_expected.to match('1.0c1') }
+    it { is_expected.to match('1.0b2-346') }
+    it { is_expected.to match('1.0b2.post345') }
+    it { is_expected.to match('1.0b2.post345.dev456') }
+    it { is_expected.to match('1.2.rev33+123456') }
+    it { is_expected.to match('1.1.dev1') }
+    it { is_expected.to match('1.0b1.dev456') }
+    it { is_expected.to match('1.0a12.dev456') }
+    it { is_expected.to match('1.0b2') }
+    it { is_expected.to match('1.0.dev456') }
+    it { is_expected.to match('1.0c1.dev456') }
+    it { is_expected.to match('1.0.post456') }
+    it { is_expected.to match('1.0.post456.dev34') }
+    it { is_expected.to match('1.2+123abc') }
+    it { is_expected.to match('1.2+abc') }
+    it { is_expected.to match('1.2+abc123') }
+    it { is_expected.to match('1.2+abc123def') }
+    it { is_expected.to match('1.2+1234.abc') }
+    it { is_expected.to match('1.2+123456') }
+    it { is_expected.to match('1.2.r32+123456') }
+    it { is_expected.to match('1!1.2.rev33+123456') }
+    it { is_expected.to match('1.0a12') }
+    it { is_expected.to match('1.2.3-45+abcdefgh') }
+    it { is_expected.to match('v1.2.3') }
+    it { is_expected.not_to match('1.2.3-45-abcdefgh') }
+    it { is_expected.not_to match('..1.2.3') }
+    it { is_expected.not_to match('  1.2.3') }
+    it { is_expected.not_to match("1.2.3  \r\t") }
+    it { is_expected.not_to match("\r\t 1.2.3") }
+    it { is_expected.not_to match('1./2.3') }
+    it { is_expected.not_to match('1.2.3-4/../../') }
+    it { is_expected.not_to match('1.2.3-4%2e%2e%') }
+    it { is_expected.not_to match('../../../../../1.2.3') }
+    it { is_expected.not_to match('%2e%2e%2f1.2.3') }
+  end
+
   describe '.semver_regex' do
     subject { described_class.semver_regex }
 

spec/models/packages/package_spec.rb

@@ -169,6 +169,73 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
       end
 
+      context 'pypi package' do
+        subject { create(:pypi_package) }
+
+        it { is_expected.to allow_value('0.1').for(:version) }
+        it { is_expected.to allow_value('2.0').for(:version) }
+        it { is_expected.to allow_value('1.2.0').for(:version) }
+        it { is_expected.to allow_value('0100!0.0').for(:version) }
+        it { is_expected.to allow_value('00!1.2').for(:version) }
+        it { is_expected.to allow_value('1.0a').for(:version) }
+        it { is_expected.to allow_value('1.0-a').for(:version) }
+        it { is_expected.to allow_value('1.0.a1').for(:version) }
+        it { is_expected.to allow_value('1.0a1').for(:version) }
+        it { is_expected.to allow_value('1.0-a1').for(:version) }
+        it { is_expected.to allow_value('1.0alpha1').for(:version) }
+        it { is_expected.to allow_value('1.0b1').for(:version) }
+        it { is_expected.to allow_value('1.0beta1').for(:version) }
+        it { is_expected.to allow_value('1.0rc1').for(:version) }
+        it { is_expected.to allow_value('1.0pre1').for(:version) }
+        it { is_expected.to allow_value('1.0preview1').for(:version) }
+        it { is_expected.to allow_value('1.0.dev1').for(:version) }
+        it { is_expected.to allow_value('1.0.DEV1').for(:version) }
+        it { is_expected.to allow_value('1.0.post1').for(:version) }
+        it { is_expected.to allow_value('1.0.rev1').for(:version) }
+        it { is_expected.to allow_value('1.0.r1').for(:version) }
+        it { is_expected.to allow_value('1.0c2').for(:version) }
+        it { is_expected.to allow_value('2012.15').for(:version) }
+        it { is_expected.to allow_value('1.0+5').for(:version) }
+        it { is_expected.to allow_value('1.0+abc.5').for(:version) }
+        it { is_expected.to allow_value('1!1.1').for(:version) }
+        it { is_expected.to allow_value('1.0c3').for(:version) }
+        it { is_expected.to allow_value('1.0rc2').for(:version) }
+        it { is_expected.to allow_value('1.0c1').for(:version) }
+        it { is_expected.to allow_value('1.0b2-346').for(:version) }
+        it { is_expected.to allow_value('1.0b2.post345').for(:version) }
+        it { is_expected.to allow_value('1.0b2.post345.dev456').for(:version) }
+        it { is_expected.to allow_value('1.2.rev33+123456').for(:version) }
+        it { is_expected.to allow_value('1.1.dev1').for(:version) }
+        it { is_expected.to allow_value('1.0b1.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0a12.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0b2').for(:version) }
+        it { is_expected.to allow_value('1.0.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0c1.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0.post456').for(:version) }
+        it { is_expected.to allow_value('1.0.post456.dev34').for(:version) }
+        it { is_expected.to allow_value('1.2+123abc').for(:version) }
+        it { is_expected.to allow_value('1.2+abc').for(:version) }
+        it { is_expected.to allow_value('1.2+abc123').for(:version) }
+        it { is_expected.to allow_value('1.2+abc123def').for(:version) }
+        it { is_expected.to allow_value('1.2+1234.abc').for(:version) }
+        it { is_expected.to allow_value('1.2+123456').for(:version) }
+        it { is_expected.to allow_value('1.2.r32+123456').for(:version) }
+        it { is_expected.to allow_value('1!1.2.rev33+123456').for(:version) }
+        it { is_expected.to allow_value('1.0a12').for(:version) }
+        it { is_expected.to allow_value('1.2.3-45+abcdefgh').for(:version) }
+        it { is_expected.to allow_value('v1.2.3').for(:version) }
+        it { is_expected.not_to allow_value('1.2.3-45-abcdefgh').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  \r\t").for(:version) }
+        it { is_expected.not_to allow_value("\r\t 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-4/../../').for(:version) }
+        it { is_expected.not_to allow_value('1.2.3-4%2e%2e%').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
+
       it_behaves_like 'validating version to be SemVer compliant for', :npm_package
       it_behaves_like 'validating version to be SemVer compliant for', :nuget_package
     end