Decode Python string escapes in PEP 508 marker literals (#19401)

[jonyoder]

Decodes Python string-escape sequences in PEP 508 marker string literals, and tightens validation to reject truncated \u/\U escapes and invalid octal digits that were previously accepted.

---

Stale issue claim — corrected

#19401 says "Product impact today: none. PPM consumes only this module's license/ package." That is false. PPM's src/ imports this module's marker package at src/pyresolve/target.go via marker.EnvironmentFromTarget, and marker evaluation ships on a customer path in PPM #20799 (get pypi --file-in --resolve-dependencies). The live defect: a requires_dist marker comparing against a string with an escape (e.g. "\n", a Windows path with \\) compared against the raw backslash sequence, not the decoded value.

This does not reach PPM immediately — PPM currently pins go-python-packaging v0.9.0 and MVS resolves there until a release is cut and PPM's pin moves.

What changed

Upstream (pypa/packaging) tokenizes quoted strings permissively (no backslash handling in the tokenizer) and decodes via process_python_str (_parser.py, effectively ast.literal_eval), converting SyntaxError/ValueError into InvalidRequirement: Invalid quoted string (_parser.py:379). This repo's tokenizer already matched the permissive rule (#18640); this PR adds the missing decode step and closes validation gaps (truncated \u, truncated \U, invalid octal) upstream also rejects.

Implemented as a single left-to-right pass (decodeQuotedStringContents, marker.go) — not a validate-then-decode split. A two-pass design is the exact bug class #18640's review caught late: an interior \\ pair immediately followed by x gets misread as a truncated \x escape by a second, independent scan. The new decoder consumes exactly one escape unit per step and never re-enters the escape switch on a byte already consumed as part of a prior escape. Test case C:\\x41 (marker_test.go) locks this in: decodes to the 6 literal characters C:\x41, not C:A.

Handles: \\ \' \" \n \t \r \0 \a \b \f \v, \xHH (2 hex digits), \uHHHH (4 hex digits), \UHHHHHHHH (8 hex digits, rejects > U+10FFFF), and Python octal \OOO (1-3 digits). Fully custom decoder, not strconv.Unquote — Go's octal/\x semantics and quote-escaping rules diverge from Python's in ways that made bending strconv.Unquote messier than a ~60-line purpose-built decoder.

Decode-site choice: decoding lives in marker.go, not in Token.Unquoted(). Grepped the whole module for .Unquoted( callers: exactly one production call site (marker.go:322, inside parseMarkerVar); the rest are test call sites in tokenizer_test.go. marker, requirement, and reqtxt reach markers only through pep508.ParseMarker/ParseFullMarker/ParseRequirement, which funnel through that one call site — none call Unquoted() directly. With a single caller, there's no benefit to pushing Python string-literal semantics into the tokenizer layer, which otherwise knows nothing about Python string grammar. Token.Unquoted()'s behavior and doc comment are otherwise unchanged (still returns raw, quote-stripped text); TestUnquoted_DoesNotDecodeEscapes locks this in.

Two independent fixes, each verified RED before GREEN

Fix A — decoding (RED captured against a variant that validated but returned the raw undecoded string):

--- FAIL: TestDecodeQuotedStringContents_Accepts
    --- FAIL: .../doubled_backslash_then_U   expected "C:\U" actual "C:\\U"
    --- FAIL: .../newline                     expected "\n" actual "\\n"
    ... 21/21 accept subtests failed
--- FAIL: TestParseMarker_QuotedStringLiteralDecodesEscapes
    expected: Literal{Value:"line\nbreak"}  actual: Literal{Value:"line\\nbreak"}

GREEN after the decoder: TestDecodeQuotedStringContents_Accepts (21/21) and TestParseMarker_QuotedStringLiteralDecodesEscapes pass.

Fix B — tightened validation (RED captured against a variant with full decoding but relaxed \u/\U width checks and permissive \8/\9):

--- FAIL: TestDecodeQuotedStringContents_Rejects
    --- FAIL: .../truncated_unicode_empty, .../truncated_unicode_short
    --- FAIL: .../truncated_unicode_wide_empty, .../truncated_unicode_wide_short
    --- FAIL: .../unicode_wide_out_of_range
    --- FAIL: .../invalid_octal_digit_8, .../invalid_octal_digit_9
--- FAIL: TestParseMarker_QuotedStringRejectsMalformedUnicodeEscape

GREEN after the tightened checks: TestDecodeQuotedStringContents_Rejects (11/11) and TestParseMarker_QuotedStringRejectsMalformedUnicodeEscape pass.

Neither temporary variant is in the committed diff.

Existing-caller safety

Checked pre-existing marker strings with backslashes elsewhere in the module (grammar_conformance_test.go, from #18640) — all still decode/validate the same way after this change. Full module test suite is green, not just internal/pep508.

Verification

• go test ./... -count=1 -timeout 900s — all packages ok, 0 failures.
• go test ./... -race — all packages ok, 0 failures.
• golangci-lint run ./... at CI's pinned v2.11.2 (confirmed against .github/workflows/ci.yml) — 0 issues.
• gofmt -l on the 4 changed files — empty.

Scope

Touches only internal/pep508/marker.go, internal/pep508/marker_test.go, internal/pep508/tokenizer.go (doc comments only — no rule changes), internal/pep508/tokenizer_test.go. Does not touch the URL/WS token rules or requirement.go (L3/#19402's territory).

NEWS

No entry — library-internal, pre-GA, this repo has no NEWS/CHANGELOG (uses chore(release): commits). Deliberately omitted, not forgotten.

Addresses rstudio/package-manager#19401.

---

🤖 Generated with Claude Code