diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1e08562b..78330d29 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,6 +19,7 @@ Behavior adaptations: * Adjust arbitrary equality intersection preservation in ``SpecifierSet`` (:pull:`951`) * Return ``False`` instead of raising for ``.contains`` with invalid version (:pull:`932`) * Support arbitrary equality on arbitrary strings for ``Specifier`` and ``SpecifierSet``'s ``filter`` and ``contains`` method. (:pull:`954`) +* Only try to parse as ``Version`` on certain marker keys, return ``False`` on unequal ordered comparisons (:pull:`939`) Fixes: diff --git a/src/packaging/markers.py b/src/packaging/markers.py index 2ae53358..ca3706fe 100644 --- a/src/packaging/markers.py +++ b/src/packaging/markers.py @@ -29,6 +29,12 @@ Operator = Callable[[str, Union[str, AbstractSet[str]]], bool] EvaluateContext = Literal["metadata", "lock_file", "requirement"] MARKERS_ALLOWING_SET = {"extras", "dependency_groups"} +MARKERS_REQUIRING_VERSION = { + "implementation_version", + "platform_release", + "python_full_version", + "python_version", +} class InvalidMarker(ValueError): @@ -177,25 +183,26 @@ def _format_marker( _operators: dict[str, Operator] = { "in": lambda lhs, rhs: lhs in rhs, "not in": lambda lhs, rhs: lhs not in rhs, - "<": operator.lt, - "<=": operator.le, + "<": lambda _lhs, _rhs: False, + "<=": operator.eq, "==": operator.eq, "!=": operator.ne, - ">=": operator.ge, - ">": operator.gt, + ">=": operator.eq, + ">": lambda _lhs, _rhs: False, } -def _eval_op(lhs: str, op: Op, rhs: str | AbstractSet[str]) -> bool: - if isinstance(rhs, str): +def _eval_op(lhs: str, op: Op, rhs: str | AbstractSet[str], *, key: str) -> bool: + op_str = op.serialize() + if key in MARKERS_REQUIRING_VERSION: try: - spec = Specifier(f"{op.serialize()}{rhs}") + spec = Specifier(f"{op_str}{rhs}") except InvalidSpecifier: pass else: return spec.contains(lhs, prereleases=True) - oper: Operator | None = _operators.get(op.serialize()) + oper: Operator | None = _operators.get(op_str) if oper is None: raise UndefinedComparison(f"Undefined {op!r} on {lhs!r} and {rhs!r}.") @@ -242,9 +249,10 @@ def _evaluate_markers( lhs_value = lhs.value environment_key = rhs.value rhs_value = environment[environment_key] + assert isinstance(lhs_value, str), "lhs must be a string" lhs_value, rhs_value = _normalize(lhs_value, rhs_value, key=environment_key) - groups[-1].append(_eval_op(lhs_value, op, rhs_value)) + groups[-1].append(_eval_op(lhs_value, op, rhs_value, key=environment_key)) elif marker == "or": groups.append([]) elif marker == "and": diff --git a/tests/test_markers.py b/tests/test_markers.py index c7aa068a..5a9ca9e6 100644 --- a/tests/test_markers.py +++ b/tests/test_markers.py @@ -80,10 +80,25 @@ def test_base_class(self) -> None: class TestOperatorEvaluation: def test_prefers_pep440(self) -> None: - assert Marker('"2.7.9" < "foo"').evaluate(dict(foo="2.7.10")) + assert Marker('"2.7.9" < python_full_version').evaluate( + dict(python_full_version="2.7.10") + ) + assert not Marker('"2.7.9" < python_full_version').evaluate( + dict(python_full_version="2.7.8") + ) - def test_falls_back_to_python(self) -> None: - assert Marker('"b" > "a"').evaluate(dict(a="a")) + def test_new_string_rules(self) -> None: + assert not Marker('"b" < python_full_version').evaluate( + dict(python_full_version="c") + ) + assert not Marker('"b" < python_full_version').evaluate( + dict(python_full_version="a") + ) + assert not Marker('"b" > "a"').evaluate(dict(a="a")) + assert not Marker('"b" < "a"').evaluate(dict(a="a")) + assert not Marker('"b" >= "a"').evaluate(dict(a="a")) + assert not Marker('"b" <= "a"').evaluate(dict(a="a")) + assert Marker('"a" <= "a"').evaluate(dict(a="a")) def test_fails_when_undefined(self) -> None: with pytest.raises(UndefinedComparison): @@ -443,3 +458,24 @@ def test_extras_and_dependency_groups_disallowed(self, variable: str) -> None: with pytest.raises(KeyError): marker.evaluate(context="requirement") + + @pytest.mark.parametrize( + ("marker_string", "environment", "expected"), + [ + ('extra == "v2"', None, False), + ('extra == "v2"', {"extra": ""}, False), + ('extra == "v2"', {"extra": "v2"}, True), + ('extra == "v2"', {"extra": "v2a3"}, False), + ('extra == "v2a3"', {"extra": "v2"}, False), + ('extra == "v2a3"', {"extra": "v2a3"}, True), + ], + ) + def test_version_like_equality( + self, marker_string: str, environment: dict[str, str] | None, expected: bool + ) -> None: + """ + Test for issue #938: Extras are meant to be literal strings, even if + they look like versions, and therefore should not be parsed as version. + """ + marker = Marker(marker_string) + assert marker.evaluate(environment) is expected