Add valid? function to semver_dialects gem in order to check semantic versions
Why are we doing this work
The comparison done by the semver_dialects
gem does not take into account if the version
that's being compared against is valid SemVer. This can be helpful to a caller who may
choose to have a different result if the version is not valid SemVer.
Relevant links
- Discussion
-
Introductory blog post
-
This blog post mentions some important details as to why the version validation is so lax. The following from the blog is especially important:
In practice it seems as if many ecosystems required features that go beyond SemVer which led to the development of many SemVer versioning flavours as well as a variety of different native constraint matching syntaxes, some of which deviate from the official SemVer specification. Depending on the ecosystem you are working with, the same semantic version string may be treated/interpreted differently: for example both Maven and pip/PyPI treat versions 1.2.3.SP differently because pip/PyPI lacks the notion of an SP post release.
...
We cannot generally assume that all provided versions we would like to process fully adhere to the SemVer spec which requires a version prefix (core) to consist of three segments: major, minor and patch. Hence, per default, we remove redundant, trailing zeros from the prefix to ensure that 2.0.0, 2.0 and 2 are considered identical.
This implies that this is a lot more difficult to validate a version than simply using the official SemVer regex.
-
Proposals
Proposal A
Gem
- Return
nil
from.parse
inversion_parser.rb
Click to expand
diff --git a/lib/semver_dialects/semantic_version/version_parser.rb b/lib/semver_dialects/semantic_version/version_parser.rb
index ef263f2..b7cd2a1 100644
--- a/lib/semver_dialects/semantic_version/version_parser.rb
+++ b/lib/semver_dialects/semantic_version/version_parser.rb
@@ -8,11 +8,13 @@ module VersionParser
return VersionInterval.new(IntervalType::LEFT_OPEN | IntervalType::RIGHT_OPEN, BelowAll.new(), AboveAll.new())
end
- version_items = versionstring.split(" ")
interval = VersionInterval.new(IntervalType::LEFT_OPEN | IntervalType::RIGHT_OPEN, BelowAll.new(), AboveAll.new())
- version_items.each do
- |version_item|
+
+ version_items = versionstring.split(" ")
+ versionstring.split(" ").each do |version_item|
matches = version_item.match /(?<op>[><=]+) *(?<version>[a-zA-Z0-9\-_\.\*]+)/
+ return unless matches
+
version_string = matches[:version]
case matches[:op]
when ">="
diff --git a/spec/unit/version_parser_spec.rb b/spec/unit/version_parser_spec.rb
index c257460..d8bf756 100644
--- a/spec/unit/version_parser_spec.rb
+++ b/spec/unit/version_parser_spec.rb
@@ -5,6 +5,10 @@ require_relative '../../lib/semver_dialects/semantic_version/version_parser.rb'
RSpec.describe VersionParser do
context 'Basic functionality when' do
+ it 'is invalid input' do
+ expect(VersionParser.parse('invalid-version')).to be_nil
+ end
+
it 'running on a simple example' do
expect(VersionParser.parse('<=3.2.9.RELEASE').to_s == '(-inf,3.2.9.RELEASE]')
expect(VersionParser.parse('>=2.3.0 <2.3.2').to_s == '[2.3.0,2.3.2)')
- Extract regular expression to constant
Click to expand
diff --git a/lib/semver_dialects/semantic_version/version_parser.rb b/lib/semver_dialects/semantic_version/version_parser.rb
index b7cd2a1..2849398 100644
--- a/lib/semver_dialects/semantic_version/version_parser.rb
+++ b/lib/semver_dialects/semantic_version/version_parser.rb
@@ -2,6 +2,8 @@ require_relative "version_cut"
require_relative "version_interval"
module VersionParser
+ REGEXP = /(?<op>[><=]+) *(?<version>[a-zA-Z0-9\-_\.\*]+)/
+
def self.parse(versionstring)
if (versionstring == "=*")
# special case = All Versions
@@ -12,7 +14,7 @@ module VersionParser
version_items = versionstring.split(" ")
versionstring.split(" ").each do |version_item|
- matches = version_item.match /(?<op>[><=]+) *(?<version>[a-zA-Z0-9\-_\.\*]+)/
+ matches = version_item.match(REGEXP)
return unless matches
version_string = matches[:version]
- Define utility method that uses the new constant
Click to expand
diff --git a/lib/semver_dialects.rb b/lib/semver_dialects.rb
index b8f9de3..59f6010 100644
--- a/lib/semver_dialects.rb
+++ b/lib/semver_dialects.rb
@@ -38,6 +38,10 @@ module SemverDialects
end
end
+ def self.valid?(version)
+ !!version.match(VersionParser::REGEXP)
+ end
+
def self.version_sat?(typ, raw_ver, raw_constraint)
version_constraint = version_translate(typ, raw_constraint)
raise SemverDialects::Error, 'malformed constraint' if version_constraint.nil? || version_constraint.empty?
- Add unit tests for the new utility method
Rails App
- Check input version is valid and update tests
Click to expand
diff --git a/ee/app/models/package_metadata/package.rb b/ee/app/models/package_metadata/package.rb
index 5538c17de524..79fffdc55f3e 100644
--- a/ee/app/models/package_metadata/package.rb
+++ b/ee/app/models/package_metadata/package.rb
@@ -20,6 +20,9 @@ def license_ids_for(version:)
# might be nil, in which case we return an empty array.
return [] if licenses.blank?
+ # if the version isn't semver, then return an empty array.
+ return [] unless SemverDialects::VersionChecker.valid?(version)
+
matching_other_license_ids(version: version) || default_license_ids
end
diff --git a/ee/spec/lib/gitlab/license_scanning/package_licenses_spec.rb b/ee/spec/lib/gitlab/license_scanning/package_licenses_spec.rb
index e73b38aae843..6c501a206319 100644
--- a/ee/spec/lib/gitlab/license_scanning/package_licenses_spec.rb
+++ b/ee/spec/lib/gitlab/license_scanning/package_licenses_spec.rb
@@ -273,19 +273,10 @@
[Hashie::Mash.new({ name: "beego", purl_type: "golang", version: "invalid-version" })]
end
- # this test shows that the current matching behaviour is incorrect, because the default
- # license is returned, when 'unknown' should actually be returned.
- # We need to add a new `valid?` method to the semver_dialects gem to handle invalid versions.
- #
- # See https://gitlab.com/gitlab-org/vulnerability-research/foss/semver_dialects/-/issues/3
- # for more details.
- #
- # TODO: once we have a `valid?` method in the semver_dialects gem, remove this test
- # and add a test to the table in the `returns 'unknown' as the license` example above.
it "returns the default licenses" do
expect(fetch).to eq([
"name" => "beego", "purl_type" => "golang", "version" => "invalid-version", "path" => "",
- "licenses" => [{ "name" => "Default License 2.1", "spdx_identifier" => "DEFAULT-2.1" }]
+ "licenses" => [{ "name" => "unknown", "spdx_identifier" => "unknown" }]
])
end
end
Implementation plan
TBD