Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 1 addition & 18 deletions internal/pep508/requirement.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 48 additions & 14 deletions internal/pep508/requirement_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---
Expand All @@ -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) {
Expand Down
18 changes: 8 additions & 10 deletions internal/pep508/tokenizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading