diff --git a/src/univers/datetime.py b/src/univers/datetime.py index 264c164d..cee2c270 100644 --- a/src/univers/datetime.py +++ b/src/univers/datetime.py @@ -105,6 +105,9 @@ def __gt__(self, other): def __ge__(self, other): return self.parsed_stamp >= other.parsed_stamp + def __str__(self): + return self.original + @classmethod def is_valid(cls, string): return bool(cls.VERSION_PATTERN.fullmatch(string)) diff --git a/src/univers/intdot.py b/src/univers/intdot.py index b48af4d5..ebd805af 100644 --- a/src/univers/intdot.py +++ b/src/univers/intdot.py @@ -36,6 +36,9 @@ def __gt__(self, other): def __ge__(self, other): return self.__cmp__(other) >= 0 + def __str__(self): + return self.original + @classmethod def is_valid(cls, string): return re.compile(IntdotVersion.VERSION_PATTERN).match(string) diff --git a/src/univers/utils.py b/src/univers/utils.py index 63952ce4..d8cb07d4 100644 --- a/src/univers/utils.py +++ b/src/univers/utils.py @@ -9,6 +9,13 @@ def remove_spaces(string): return "".join(string.split()) +ASCII_WHITESPACE = frozenset(" \t\n\r\f\v") + + +def contains_ascii_whitespace(string): + return any(character in ASCII_WHITESPACE for character in string) + + def cmp(x, y): """ Replacement for built-in Python 2 function cmp that was removed in Python 3 diff --git a/src/univers/version_constraint.py b/src/univers/version_constraint.py index 2613a487..b30265f0 100644 --- a/src/univers/version_constraint.py +++ b/src/univers/version_constraint.py @@ -9,6 +9,7 @@ import attr +from univers.utils import contains_ascii_whitespace from univers.utils import remove_spaces from univers.versions import Version @@ -166,7 +167,10 @@ def from_string(cls, string, version_class): Return a single VersionConstraint built from a constraint ``string`` and a ``version_class`` Version class. """ - constraint_string = remove_spaces(string) + if contains_ascii_whitespace(string): + raise ValueError(f"Invalid ASCII whitespace in constraint: {string!r}") + + constraint_string = string # A version range specifier contains only printable ASCII letters, digits and # punctuation. @@ -195,7 +199,7 @@ def split(string): For example:: >>> assert VersionConstraint.split(">=2.3") == (">=", "2.3",) - >>> assert VersionConstraint.split(" < = 2 . 3 ") == ("<=", "2.3",) + >>> assert VersionConstraint.split("<=2.3") == ("<=", "2.3",) >>> assert VersionConstraint.split("2.3") == ("=", "2.3",) >>> assert VersionConstraint.split("*2.3") == ("*", "",) >>> assert VersionConstraint.split("*") == ("*", "",) @@ -208,7 +212,10 @@ def split(string): ... except ValueError: ... pass """ - constraint_string = remove_spaces(string) + if contains_ascii_whitespace(string): + raise ValueError(f"Invalid ASCII whitespace in constraint: {string!r}") + + constraint_string = string # special case for star if constraint_string.startswith("*"): @@ -292,13 +299,13 @@ def validate(cls, constraints): # occur only once in any ```` of a range specifier, # irrespective of its comparators. Tools must report an error for duplicated # versions. - if len(set(c.version for c in constraints)) != len(constraints): - raise ValueError(f"{constraints!r} cannot contain duplicated Version") + for index, constraint in enumerate(constraints): + if any(constraint.version == other.version for other in constraints[index + 1 :]): + raise ValueError(f"{constraints!r} cannot contain duplicated Version") - # Constraints are sorted by version**. The canonical ordering is the versions - # order. The ordering of ```` is not significant otherwise - # but this sort order is needed when check if a version is contained in a range. - constraints.sort() + sorted_constraints = sorted(constraints) + if list(constraints) != sorted_constraints: + raise ValueError(f"{constraints!r} must use canonical ordering by version") return validate_comparators(constraints) diff --git a/src/univers/version_range.py b/src/univers/version_range.py index c50bc87c..97c237a6 100644 --- a/src/univers/version_range.py +++ b/src/univers/version_range.py @@ -18,6 +18,7 @@ from univers import maven from univers import versions from univers.conan.version_range import VersionRange as conan_version_range +from univers.utils import contains_ascii_whitespace from univers.utils import remove_spaces from univers.version_constraint import VersionConstraint from univers.version_constraint import contains_version @@ -108,8 +109,10 @@ def from_string(cls, vers, simplify=False, validate=False): f"{vers!r} is not a valid argument, a valid ``vers`` string argument is required." ) - # Spaces are not significant and removed in a canonical form. - vers = remove_spaces(vers) + if contains_ascii_whitespace(vers): + raise ValueError(f"Invalid ASCII whitespace in VERS string: {vers!r}") + + original_vers = vers # A version range specifier contains only printable ASCII letters, digits and # punctuation. @@ -134,7 +137,6 @@ def from_string(cls, vers, simplify=False, validate=False): version_class = range_class.version_class - constraints = remove_spaces(constraints) if not constraints: raise ValueError(f"{vers!r} specifies no version range constraints.") @@ -149,7 +151,12 @@ def from_string(cls, vers, simplify=False, validate=False): parsed_constraints = [] - constraints = constraints.strip("|") + if constraints.startswith("|") or constraints.endswith("|"): + raise ValueError(f"{vers!r} contains a leading or trailing pipe '|'.") + + if "||" in constraints: + raise ValueError(f"{vers!r} contains consecutive pipes '||'.") + for const in constraints.split("|"): constraint = VersionConstraint.from_string( string=const, @@ -157,17 +164,19 @@ def from_string(cls, vers, simplify=False, validate=False): ) parsed_constraints.append(constraint) - # Constraints are sorted by version**. The canonical ordering is the versions - # order. The ordering of ```` is not significant otherwise - # but this sort order is needed when check if a version is contained in a range. - parsed_constraints.sort() + VersionConstraint.validate(parsed_constraints) if simplify: parsed_constraints = VersionConstraint.simplify(parsed_constraints) if validate: VersionConstraint.validate(parsed_constraints) - return range_class(parsed_constraints) + version_range = range_class(parsed_constraints) + canonical_vers = str(version_range) + if canonical_vers != original_vers: + raise ValueError(f"{original_vers!r} is not canonical. Expected {canonical_vers!r}.") + + return version_range @classmethod def from_versions(cls, sequence): diff --git a/tests/data/schema/range/pypi_range_containment.json b/tests/data/schema/range/pypi_range_containment.json index d4b232bf..3998f5c8 100644 --- a/tests/data/schema/range/pypi_range_containment.json +++ b/tests/data/schema/range/pypi_range_containment.json @@ -44,7 +44,7 @@ "test_group": "advanced", "test_type": "containment", "input": { - "vers": "vers:pypi/>=3.0.0|2.0.3", + "vers": "vers:pypi/2.0.3|>=3.0.0", "version": "2.0.3" }, "expected_output": true @@ -74,7 +74,7 @@ "test_group": "advanced", "test_type": "containment", "input": { - "vers": "vers:pypi/>=3.0.0|!=2.0.3", + "vers": "vers:pypi/!=2.0.3|>=3.0.0", "version": "2.0.3" }, "expected_output": false @@ -94,7 +94,7 @@ "test_group": "advanced", "test_type": "containment", "input": { - "vers": "vers:pypi/0.0.2|0.0.6|>=3.0.0|0.0.1|0.0.4|0.0.5|0.0.3", + "vers": "vers:pypi/0.0.1|0.0.2|0.0.3|0.0.4|0.0.5|0.0.6|>=3.0.0", "version": "2.0.3" }, "expected_output": false diff --git a/tests/data/schema/range/pypi_range_roundtrip.json b/tests/data/schema/range/pypi_range_roundtrip.json index da9a6dd2..583a08a5 100644 --- a/tests/data/schema/range/pypi_range_roundtrip.json +++ b/tests/data/schema/range/pypi_range_roundtrip.json @@ -13,7 +13,7 @@ "test_group": "advanced", "test_type": "roundtrip", "input": { - "vers": "vers:pypi/0.0.2|0.0.6|0.0.0|0.0.1|0.0.4|0.0.5|0.0.3" + "vers": "vers:pypi/0.0.0|0.0.1|0.0.2|0.0.3|0.0.4|0.0.5|0.0.6" }, "expected_output": "vers:pypi/0.0.0|0.0.1|0.0.2|0.0.3|0.0.4|0.0.5|0.0.6" }, @@ -22,27 +22,27 @@ "test_group": "advanced", "test_type": "roundtrip", "input": { - "vers": "vers:pypi/>0.0.0|>=0.0.1|0.0.2|<0.0.3|0.0.4|<0.0.5|>=0.0.6" + "vers": "vers:pypi/0.0.2|0.0.4|>=0.0.6" }, - "expected_output": "vers:pypi/>0.0.0|>=0.0.1|0.0.2|<0.0.3|0.0.4|<0.0.5|>=0.0.6" + "expected_output": "vers:pypi/0.0.2|0.0.4|>=0.0.6" }, { "description": "Roundtrip test for pypi VERS range.", "test_group": "advanced", "test_type": "roundtrip", "input": { - "vers": "vers:pypi/>0.0.0|>=0.0.1|>=0.0.1|0.0.2|0.0.3|0.0.4|<0.0.5|<=0.0.6|!=0.7|8.0|>12|<15.3" + "vers": "vers:pypi/!=0.7|8.0|>12|<15.3" }, - "expected_output": "vers:pypi/>0.0.0|>=0.0.1|>=0.0.1|0.0.2|0.0.3|0.0.4|<0.0.5|<=0.0.6|!=0.7|8.0|>12|<15.3" + "expected_output": "vers:pypi/!=0.7|8.0|>12|<15.3" }, { "description": "Roundtrip test for pypi VERS range.", "test_group": "advanced", "test_type": "roundtrip", "input": { - "vers": "vers:pypi/>0.0.0|>=0.0.1|0.0.2|0.0.3|0.0.4|<0.0.5|>=0.0.6|!=0.8" + "vers": "vers:pypi/>0.0.0|<0.0.5|>=0.0.6|!=0.8" }, - "expected_output": "vers:pypi/>0.0.0|>=0.0.1|0.0.2|0.0.3|0.0.4|<0.0.5|>=0.0.6|!=0.8" + "expected_output": "vers:pypi/>0.0.0|<0.0.5|>=0.0.6|!=0.8" }, { "description": "Roundtrip test for pypi VERS range.", @@ -166,7 +166,7 @@ "test_group": "advanced", "test_type": "roundtrip", "input": { - "vers": "vers:pypi/0.0.2|0.0.6|>=0.0.0|0.0.1|0.0.4|0.0.5|0.0.3" + "vers": "vers:pypi/>=0.0.0|0.0.1|0.0.2|0.0.3|0.0.4|0.0.5|0.0.6" }, "expected_output": "vers:pypi/>=0.0.0|0.0.1|0.0.2|0.0.3|0.0.4|0.0.5|0.0.6" } diff --git a/tests/data/schema/range/vers_parse_canonical.json b/tests/data/schema/range/vers_parse_canonical.json new file mode 100644 index 00000000..124ba971 --- /dev/null +++ b/tests/data/schema/range/vers_parse_canonical.json @@ -0,0 +1,64 @@ +{ + "$schema": "https://packageurl.org/schemas/vers-test.schema-0.1.json", + "tests": [ + { + "description": "Canonical parse succeeds for an already normalized VERS.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/>=1.0.0|<2.0.0", + "expected_output": { + "scheme": "npm", + "version_constraints": [ + [ + ">=", + "1.0.0" + ], + [ + "<", + "2.0.0" + ] + ] + } + }, + { + "description": "Parsing fails when ASCII whitespace is present.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/>=1.0.0| <2.0.0", + "expected_failure": true, + "expected_failure_reason": "non-canonical VERS: whitespace is not permitted" + }, + { + "description": "Parsing fails when constraints start with a pipe.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/|>=1.0.0|<2.0.0", + "expected_failure": true, + "expected_failure_reason": "non-canonical VERS: leading pipe is not permitted" + }, + { + "description": "Parsing fails when constraints end with a pipe.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/>=1.0.0|<2.0.0|", + "expected_failure": true, + "expected_failure_reason": "non-canonical VERS: trailing pipe is not permitted" + }, + { + "description": "Parsing fails when constraints contain consecutive pipes.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/>=1.0.0||<2.0.0", + "expected_failure": true, + "expected_failure_reason": "non-canonical VERS: consecutive pipes are not permitted" + }, + { + "description": "Parsing fails when constraints are not in canonical order.", + "test_group": "base", + "test_type": "parse", + "input": "vers:npm/>=2.0.0|<1.0.0", + "expected_failure": true, + "expected_failure_reason": "non-canonical VERS: constraints are not sorted by version" + } + ] +} \ No newline at end of file diff --git a/tests/test_version_constraint.py b/tests/test_version_constraint.py index 9997b7c7..a5638295 100644 --- a/tests/test_version_constraint.py +++ b/tests/test_version_constraint.py @@ -96,3 +96,12 @@ def test_invalid_vers_comparator_prefixes(spec): string=spec, version_class=versions.SemverVersion, ) + + +@pytest.mark.parametrize("spec", [">= 2.3", "\t>=2.3", ">=2.3\n"]) +def test_invalid_vers_constraint_with_ascii_whitespace(spec): + with pytest.raises(ValueError, match="ASCII whitespace"): + VersionConstraint.from_string( + string=spec, + version_class=versions.SemverVersion, + ) diff --git a/tests/test_version_range.py b/tests/test_version_range.py index b2cb3c4a..927c2020 100644 --- a/tests/test_version_range.py +++ b/tests/test_version_range.py @@ -59,6 +59,52 @@ TEST_DATA_PYPI_CONTAINMENT = TEST_DATA_PARENT / "pypi_range_containment.json" TEST_DATA_PYPI_ROUNDTRIP = TEST_DATA_PARENT / "pypi_range_roundtrip.json" +TEST_DATA_VERS_PARSE_CANONICAL = TEST_DATA_PARENT / "vers_parse_canonical.json" + + +def assert_parse_failure_reason(error_message, expected_reason): + message = error_message.lower() + reason = expected_reason.lower() + + if "whitespace" in reason: + assert "whitespace" in message + return + + if "leading pipe" in reason or "trailing pipe" in reason: + assert "leading or trailing pipe" in message + return + + if "consecutive pipes" in reason: + assert "consecutive pipes" in message + return + + if "sorted by version" in reason: + assert "canonical ordering" in message + return + + assert reason in message + + +@pytest.mark.parametrize("test_case", json.load(open(TEST_DATA_VERS_PARSE_CANONICAL))["tests"]) +def test_vers_spec_parse_cases(test_case): + vers_string = test_case["input"] + + if test_case.get("expected_failure"): + with pytest.raises(ValueError) as error: + VersionRange.from_string(vers_string) + + expected_failure_reason = test_case.get("expected_failure_reason") + if expected_failure_reason: + assert_parse_failure_reason(str(error.value), expected_failure_reason) + return + + version_range = VersionRange.from_string(vers_string) + expected_output = test_case["expected_output"] + + assert version_range.scheme == expected_output["scheme"] + assert [ + [constraint.comparator, str(constraint.version)] for constraint in version_range.constraints + ] == expected_output["version_constraints"] class TestVersionRangeContainment(SchemaDrivenVersTest): @@ -244,6 +290,23 @@ def test_alpine_version_range_containment(): assert AlpineLinuxVersion("3.19.0") not in vr +@pytest.mark.parametrize( + "vers, expected_error", + [ + ("vers:pypi/>= 1.0", "ASCII whitespace"), + ("vers:pypi/|>=1.0", "leading or trailing pipe"), + ("vers:pypi/>=1.0|", "leading or trailing pipe"), + ("vers:pypi/>=1.0||<2.0", "consecutive pipes"), + ("vers:pypi/>=2.0|>=1.0", "canonical ordering"), + ("vers:pypi/>=1.0|<=1.0", "duplicated Version"), + ("vers:pypi/>=1.0|>2.0", "cannot be followed by"), + ], +) +def test_version_range_from_string_rejects_non_canonical_vers(vers, expected_error): + with pytest.raises(ValueError, match=expected_error): + VersionRange.from_string(vers) + + VERSION_RANGE_TESTS_BY_SCHEME = { "nginx": ["0.8.40+", "0.7.52-0.8.39", "0.9.10", "1.5.0+, 1.4.1+"], "npm": [ @@ -320,13 +383,13 @@ def test_mattermost_version_range(): constraints=[ VersionConstraint(comparator="=", version=SemverVersion("5.0")), ] - ) == VersionRange.from_string("vers:mattermost/5.0") + ) == VersionRange.from_string("vers:mattermost/5.0.0") assert MattermostVersionRange( constraints=[ VersionConstraint(comparator=">=", version=SemverVersion("5.0")), ] - ) == VersionRange.from_string("vers:mattermost/>=5.0") + ) == VersionRange.from_string("vers:mattermost/>=5.0.0") def test_build_range_from_snyk_advisory_string(): @@ -453,8 +516,8 @@ def test_version_range_datetime(): def test_version_range_lexicographic(): assert LexicographicVersion("1.2.3") in VersionRange.from_string( - "vers:lexicographic/<1.2.4|>0.9" + "vers:lexicographic/>0.9|<1.2.4" ) assert LexicographicVersion(-123) in VersionRange.from_string("vers:lexicographic/<~") assert LexicographicVersion(None) in VersionRange.from_string("vers:lexicographic/*") - assert LexicographicVersion("ABC") in VersionRange.from_string("vers:lexicographic/>abc|<=None") + assert LexicographicVersion("ABC") in VersionRange.from_string("vers:lexicographic/<=None|>abc")