Skip to content

Escape backslashes when serializing marker values - #1374

Open
sueun-dev wants to merge 2 commits into
pypa:mainfrom
sueun-dev:fix-marker-value-backslash-roundtrip
Open

Escape backslashes when serializing marker values#1374
sueun-dev wants to merge 2 commits into
pypa:mainfrom
sueun-dev:fix-marker-value-backslash-roundtrip

Conversation

@sueun-dev

Copy link
Copy Markdown

Marker and requirement values are read back with ast.literal_eval (in _parser.process_python_str), which decodes backslash escapes. Value.serialize re-wraps the decoded value in quotes but doesn't re-escape it, so a value containing a backslash doesn't survive a str() round trip:

>>> from packaging.markers import Marker
>>> m = Marker(r'os_name == "C:\\temp"')   # value is C:\temp
>>> str(m)
'os_name == "C:\\temp"'                     # single, unescaped backslash
>>> Marker(str(m)) == m
False                                        # reparses \t as a tab -> C:<TAB>emp

Same for Requirement:

>>> from packaging.requirements import Requirement
>>> r = Requirement(r'demo; os_name == "C:\\temp"')
>>> Requirement(str(r)) == r
False

serialize is the inverse of the ast.literal_eval parser, so it has to escape everything the parser decodes. This is the backslash counterpart of the quote round trip fixed in #1213; the fix doubles backslashes before quoting and leaves the quote-delimiter selection alone.

Added regression tests for Value.serialize and for the Marker/Requirement round trip. Both fail before this change and pass after.

Marker and requirement values are read back with ``ast.literal_eval``
(``_parser.process_python_str``), which decodes backslash escapes, but
``Value.serialize`` re-wrapped the decoded value in quotes without
re-escaping. A value containing a backslash therefore did not survive a
``str()`` round trip: ``os_name == "C:\\temp"`` (value ``C:\temp``)
serialized to ``os_name == "C:\temp"``, which reparses with ``\t`` as a
tab, so ``Marker(str(m)) != m`` (and the same for ``Requirement``).

Double the backslashes on serialization so the value round-trips
unchanged. This completes the round-trip contract added for the quote
case in pypa#1213.
Comment thread src/packaging/_parser.py
Comment on lines +72 to +75
# ``process_python_str`` reads a value back with ``ast.literal_eval``,
# which decodes backslash escapes. Double the backslashes so a value
# that contains one survives the round trip instead of being re-decoded
# (e.g. ``C:\temp`` would otherwise reparse to ``C:<TAB>emp``).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# ``process_python_str`` reads a value back with ``ast.literal_eval``,
# which decodes backslash escapes. Double the backslashes so a value
# that contains one survives the round trip instead of being re-decoded
# (e.g. ``C:\temp`` would otherwise reparse to ``C:<TAB>emp``).
# ``process_python_str`` reads a value with ``ast.literal_eval``,
# which decodes backslash escapes.

But are there any string-valued markers that might be a path, though? os_name won't be a path. I'm not sure what chars are valid in a node name, that's the main arbitrary one I can think of. I'm not sure the value if there are no markers that can possibly have a backslash.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants