diff --git a/internal/pep508/requirement.go b/internal/pep508/requirement.go index 7f7a49f..751d1a7 100644 --- a/internal/pep508/requirement.go +++ b/internal/pep508/requirement.go @@ -143,25 +143,8 @@ func parseRequirementDetails(t *Tokenizer, req *RawRequirement) error { if t.peek(End) { return nil } - // Consume horizontal whitespace and any immediately following line - // breaks. PEP 508 requires whitespace after a URL; a newline is valid - // (common in "defensively multiline" Requires-Dist metadata) and must - // be consumed here so a following "; marker" clause is recognized. if !t.consume(WS) { - // No horizontal whitespace found; check for a line break. - if t.pos < len(t.source) && (t.source[t.pos] == '\n' || t.source[t.pos] == '\r') { - // Consume the line break(s). - for t.pos < len(t.source) && (t.source[t.pos] == '\n' || t.source[t.pos] == '\r') { - t.pos++ - } - } else { - return t.NewSyntaxError("Expected whitespace after URL") - } - } else { - // Consumed horizontal whitespace; also consume any following line breaks. - for t.pos < len(t.source) && (t.source[t.pos] == '\n' || t.source[t.pos] == '\r') { - t.pos++ - } + return t.NewSyntaxError("Expected whitespace after URL") } if t.peek(End) { return nil diff --git a/internal/pep508/requirement_test.go b/internal/pep508/requirement_test.go index 801f194..b166257 100644 --- a/internal/pep508/requirement_test.go +++ b/internal/pep508/requirement_test.go @@ -171,20 +171,41 @@ func TestParseRequirement_URLContainingSemicolon_NoMarker(t *testing.T) { assert.Nil(t, req.Marker) } -// --- URL terminates at any whitespace, not just space/tab --- - -func TestParseRequirement_URLThenNewlineThenMarker_Splits(t *testing.T) { - // A defensively-multiline value: the "@ url" clause ends in a newline - // rather than a space before "; marker". The URL rule must treat the - // newline as a terminator (it is the exact complement of the WS rule, - // \s+) so the marker clause is not absorbed into the URL token. - req := parseRequirementString(t, "foo @ https://x/y\n; python_version > \"3\"") - assert.Equal(t, "https://x/y", req.URL) - require.NotNil(t, req.Marker) - cmp, ok := req.Marker.(*CompareExpr) - require.True(t, ok) - assert.Equal(t, EnvVar{Name: "python_version"}, cmp.Lhs) - assert.Equal(t, Literal{Value: "3"}, cmp.Rhs) +// --- URL terminates at horizontal whitespace (space/tab) only, matching +// upstream's own URL rule ([^ \t]+) - NOT at a line break. This was checked +// against a real packaging 4eb0753dba8fcaaac8eb75463374e448f0931558 install: +// Requirement("name @ https://example.com\n").url == "https://example.com\n" +// (the line break is embedded in the url, not rejected). An older version of +// this test asserted the opposite (line break treated as a terminator, so +// a following "; marker" clause still parsed) - that pinned a divergence +// from upstream, introduced when WS was narrowed to horizontal-only ([ \t]+) +// without updating the URL rule to match; it is corrected below. + +func TestParseRequirement_URLForm_BareLineBreak_EmbeddedInURL(t *testing.T) { + // With nothing following the line break, the whole thing (URL bytes + // plus break) has no space/tab in it, so it is ONE greedy URL token and + // the requirement parses successfully - matching upstream exactly. + for _, lb := range []string{"\n", "\r", "\r\n"} { + t.Run(escapeForName(lb), func(t *testing.T) { + req := parseRequirementString(t, "foo @ https://x/y"+lb) + assert.Equal(t, "https://x/y"+lb, req.URL) + assert.Nil(t, req.Marker) + }) + } +} + +func TestParseRequirement_URLForm_LineBreakBeforeMarker_SwallowsSemicolonIntoURL(t *testing.T) { + // The line break is not a delimiter, so it - and the following ";" - + // are absorbed into the URL token, right up to the next space. That + // consumes the marker clause's required ";", leaving the marker's own + // text (which contains spaces) as unparsable trailing garbage. + for _, lb := range []string{"\n", "\r", "\r\n"} { + t.Run(escapeForName(lb), func(t *testing.T) { + tok := NewTokenizer("foo @ https://x/y" + lb + `; python_version > "3"`) + _, err := ParseRequirement(tok) + require.Error(t, err, "line break then marker should not parse (semicolon is swallowed into the URL)") + }) + } } // --- grammar asymmetry: url form requires wsp+ before ";" marker --- @@ -205,6 +226,19 @@ func TestParseRequirement_URLForm_SpaceBeforeMarker_Splits(t *testing.T) { require.NotNil(t, req.Marker) } +// A trailing horizontal space or tab after the URL (nothing else follows) +// still terminates the URL correctly and parses, mirroring upstream's +// test_trailing_horizontal_whitespace (which only covers the name form). +func TestParseRequirement_URLForm_AllowsTrailingHorizontalWhitespace(t *testing.T) { + for _, ws := range []string{" ", "\t", " \t"} { + t.Run(escapeForName(ws), func(t *testing.T) { + req := parseRequirementString(t, "foo @ https://x/y"+ws) + assert.Equal(t, "https://x/y", req.URL) + assert.Nil(t, req.Marker) + }) + } +} + // --- combinations --- func TestParseRequirement_ExtrasAndVersionSpecAndMarker(t *testing.T) { diff --git a/internal/pep508/tokenizer.go b/internal/pep508/tokenizer.go index eeae072..0a959d6 100644 --- a/internal/pep508/tokenizer.go +++ b/internal/pep508/tokenizer.go @@ -228,16 +228,14 @@ var tokenRules = map[Kind]*regexp.Regexp{ // leftover "=2.0" rather than at the fusion point, which is a worse error // message for the same correct verdict. Specifier: regexp.MustCompile(`\A(?:===\s*[^\s,;)]+|(?:==|~=|!=|<=|>=|<|>)\s*[^\s,;)<>=~]+)`), - // URL: a greedy run of non-whitespace characters - the exact - // complement of the WS rule above (\s+) - deliberately not bounded by - // ";" (see the URL Kind doc comment). Upstream packaging's rule - // excludes only space ([^ ]+); this port is stricter, treating all - // whitespace (including tab and newlines) as a terminator. That's - // safe because a valid URL never contains whitespace, and it avoids - // a stray newline (e.g. in a defensively-multiline Requires-Dist - // value) being absorbed into the URL along with a following "; marker" - // clause. - URL: regexp.MustCompile(`\A\S+`), + // URL: a greedy run of non-horizontal-whitespace characters, matching + // upstream packaging's own URL rule ([^ \t]+) exactly - it stops only + // at a space or tab, not at a line break. It is deliberately not + // bounded by ";" either (see the URL Kind doc comment): a URL can + // itself contain a semicolon (e.g. a query string), so PEP 508 relies + // on mandatory whitespace - not a ban on ";" - to separate a URL from + // a following "; marker" clause. + URL: regexp.MustCompile(`\A[^ \t]+`), } // Tokenizer performs context-sensitive lexing over a PEP 508 source string.