Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_parse_version_many does not match PEP 508 or documentation #803

Open
matt-phylum opened this issue May 20, 2024 · 2 comments
Open

_parse_version_many does not match PEP 508 or documentation #803

matt-phylum opened this issue May 20, 2024 · 2 comments

Comments

@matt-phylum
Copy link

PEP 508 says that a version_many looks like version_one (wsp* ',' version_one)* and the documentation comment in _parse_version_many says that a version_many looks like (SPECIFIER (WS? COMMA WS? SPECIFIER)*)? (essentially the same thing, but optional).

However, the implementation of _parse_version_many actually parses something like version_one (wsp* ',' version_one)* wsp* (',' wsp*)? or (SPECIFIER (WS? COMMA WS? SPECIFIER)* WS? (COMMA WS?)?)?. An extra comma, possibly surrounded by whitespace, is accepted.

It's probably too late to fix the implementation. Packages like this one contain malformed requirements that are accepted by the implementation but not the documentation. The documentation should probably be changed to reflect reality instead.

from packaging._tokenizer import DEFAULT_RULES, Tokenizer
from packaging._parser import _parse_version_many

def parse(input: str) -> str:
    tokenizer = Tokenizer(input, rules=DEFAULT_RULES)
    _parse_version_many(tokenizer)
    return (tokenizer.source[:tokenizer.position], tokenizer.source[tokenizer.position:])

print(repr(parse(">=1.0")))
print(repr(parse(">=1.0 ")))
print(repr(parse(">=1.0,")))
print(repr(parse(">=1.0 ,")))
print(repr(parse(">=1.0 , ")))

produces

('>=1.0', '')
('>=1.0 ', '')
('>=1.0,', '')
('>=1.0 ,', '')
('>=1.0 , ', '')

It looks like the extra, empty specifier is accidentally removed by SpecifierSet when it tries to handle converting an empty string into an empty set.

from packaging.specifiers import SpecifierSet

print(SpecifierSet("")._specs)
print(SpecifierSet(">=1.0")._specs)
print(SpecifierSet(">=1.0,")._specs)

produces

frozenset()
frozenset({<Specifier('>=1.0')>})
frozenset({<Specifier('>=1.0')>})

Together, these behaviors cause trailing commas in requirements to be ignored, allowing invalid requirements to be installed by setuptools.

from packaging.requirements import Requirement

print(Requirement("a>=1.0,"))

produces

a>=1.0
@uranusjr
Copy link
Member

The canonical spec source is on packaging.python.org: https://packaging.python.org/en/latest/specifications/version-specifiers/#compatible-release

So the most important thing would be to raise a PR to pypa/packaging.python.org if we don’t decide to change the implementation. (Personally I feel it’s not worthwhile to break people’s things for this.)

@matt-phylum
Copy link
Author

It looks like this is the relevant spec from packaging.python.org: https://packaging.python.org/en/latest/specifications/dependency-specifiers/ It's the same as the PEP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants