diff --git a/internal/normalize_url.go b/internal/normalize_url.go index a08b473..6ef640e 100644 --- a/internal/normalize_url.go +++ b/internal/normalize_url.go @@ -5,19 +5,12 @@ package internal import ( "net/url" - "regexp" "strings" ) const ( - defaultHTTPPort = ":80" - defaultHTTPSPort = ":443" -) - -// Regular expressions used by the normalizations. -var ( - rxPort = regexp.MustCompile(`(:\d+)/?$`) - rxDupSlashes = regexp.MustCompile(`/{2,}`) + defaultHTTPPort = "80" + defaultHTTPSPort = "443" ) // NormalizeURL will normalize the specified URL @@ -55,20 +48,66 @@ func lowercaseHost(u *url.URL) { } } +// removeDefaultPort drops :80 from an http URL and :443 from an https one. +// +// The port stays when dropping it would leave an authority url.Parse no longer accepts, so the +// shortened host is parsed before being kept. url.Parse reads "https://:a:443" as the host ":a" +// on port 443, and ":a" on its own is an invalid port, so "https://:a" no longer parses. +// +// A degenerate authority can spell a default port twice - url.Parse reads "http://:80:80" as the +// host ":80" on port 80 - so removal repeats until nothing more comes off. Each pass shortens the +// host, so the loop ends, and normalizing the result again changes nothing. func removeDefaultPort(u *url.URL) { - if len(u.Host) > 0 { - scheme := strings.ToLower(u.Scheme) - u.Host = rxPort.ReplaceAllStringFunc(u.Host, func(val string) string { - if (scheme == "http" && val == defaultHTTPPort) || (scheme == "https" && val == defaultHTTPSPort) { - return "" - } - return val - }) + for { + port := u.Port() + if port == "" || port != defaultPortForScheme(strings.ToLower(u.Scheme)) { + return + } + + host := strings.TrimSuffix(u.Host, ":"+port) + if _, err := url.Parse("//" + host); err != nil { + return + } + + u.Host = host + } +} + +func defaultPortForScheme(scheme string) string { + switch scheme { + case "http": + return defaultHTTPPort + case "https": + return defaultHTTPSPort + default: + return "" } } +// removeDuplicateSlashes collapses every run of slashes in the path to a single one, however +// long the run is: "/a//b///c" becomes "/a/b/c". +// +// A path holding no "//" is left as it is, which is the common case and costs one scan and no +// allocation. func removeDuplicateSlashes(u *url.URL) { - if len(u.Path) > 0 { - u.Path = rxDupSlashes.ReplaceAllString(u.Path, "/") + const doubleSlash = "//" + + start := strings.Index(u.Path, doubleSlash) + if start < 0 { + return } + + var collapsed strings.Builder + collapsed.Grow(len(u.Path)) + collapsed.WriteString(u.Path[:start+1]) // everything up to and including the first slash of the run + + for i := start + 1; i < len(u.Path); i++ { + c := u.Path[i] + if c == '/' && u.Path[i-1] == '/' { + continue + } + collapsed.WriteByte(c) + } + + u.Path = collapsed.String() } diff --git a/internal/normalize_url_bench_test.go b/internal/normalize_url_bench_test.go new file mode 100644 index 0000000..df21cb9 --- /dev/null +++ b/internal/normalize_url_bench_test.go @@ -0,0 +1,38 @@ +// SPDX-FileCopyrightText: Copyright (c) 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package internal + +import ( + "net/url" + "testing" +) + +func BenchmarkNormalizeURL(b *testing.B) { + // the shapes NormalizeURL branches on: nothing to do, a default port with an upper-cased + // host, a fragment, a query, and a path holding runs of slashes + benchURLs := []string{ + "https://example.com/v1/pets/{petId}/photos", + mixedCaseDefaultPort, + "file:///base/path.json#/definitions/a", + "http://a/b/c/d;p?q", + "https://example.com/a//b///c////d?x=1#/frag", + } + + parsed := make([]url.URL, 0, len(benchURLs)) + for _, raw := range benchURLs { + u, err := url.Parse(raw) + if err != nil { + b.Fatal(err) + } + parsed = append(parsed, *u) + } + + b.ReportAllocs() + for b.Loop() { + for i := range parsed { + u := parsed[i] + NormalizeURL(&u) + } + } +} diff --git a/internal/normalize_url_fuzz_test.go b/internal/normalize_url_fuzz_test.go new file mode 100644 index 0000000..b143361 --- /dev/null +++ b/internal/normalize_url_fuzz_test.go @@ -0,0 +1,66 @@ +// SPDX-FileCopyrightText: Copyright (c) 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package internal + +import ( + "net/url" + "testing" + + "github.com/go-openapi/testify/v2/require" +) + +// FuzzNormalizeURL pins the postcondition callers rely on: a URL that parsed before still +// parses after being normalized, and normalizing what came out changes nothing further. +// +// Both matter to go-openapi/spec, which turns a normalized URL back into a string, hands it +// to MustCreateRef and gets a panic when it no longer parses. +func FuzzNormalizeURL(f *testing.F) { + for _, seed := range normalizeURLSeeds() { + f.Add(seed) + } + + f.Fuzz(func(t *testing.T, input string) { + u, err := url.Parse(input) + if err != nil { + return + } + + NormalizeURL(u) + once := u.String() + + again, err := url.Parse(once) + require.NoErrorf(t, err, "normalizing %q yielded %q, which no longer parses", input, once) + + NormalizeURL(again) + require.EqualTf(t, once, again.String(), "normalizing %q is not idempotent", input) + }) +} + +// normalizeURLSeeds covers what the normalizations branch on: the schemes carrying a default +// port, ports that are not default, hosts needing lower-casing, duplicate slashes, escaped +// paths and fragments, IPv6 literals, userinfo, and the degenerate authorities url.Parse accepts. +func normalizeURLSeeds() []string { + return []string{ + "", + "/folder/file", + mixedCaseDefaultPort, + "HTTP://xYz.cOm:80/folder//file", + "http://xyz.com:8080/folder", + "https://xyz.com/a//b///c////d", + "https://xyz.com////", + "postGRES://xYz.cOm:5432/folder//file", + userinfoDefaultPort + "?q=1#/a~1b", + "https://[2001:DB8::1]:443/folder", + ipv6NonDefaultPort, + degenerateHostHTTPS, + degenerateHostHTTP, + "https://:443", + "file:///base/path.json#/definitions/a%20b", + "file://", + "https://localhost/%F0%9F%8C%AD#/%F0%9F%8D%94", + "mailto:someone@example.com", + "//host/path", + "http:g", + } +} diff --git a/internal/normalize_url_test.go b/internal/normalize_url_test.go index bf6d88a..a64b8d1 100644 --- a/internal/normalize_url_test.go +++ b/internal/normalize_url_test.go @@ -11,13 +11,32 @@ import ( "github.com/go-openapi/testify/v2/require" ) +// URLs spelled in more than one test in this package. +const ( + // url.Parse reads these as the host ":a" on a default port, so dropping the port would + // leave "https://:a", which no longer parses. + degenerateHostHTTPS = "https://:a:443" + degenerateHostHTTP = "http://:a:80" + + ipv6NonDefaultPort = "https://[2001:db8::1]:8443/folder" + + // a default port, an upper-cased scheme and host, and a duplicate slash, all at once. + mixedCaseDefaultPort = "HTTPs://xYz.cOm:443/folder//file" + + // userinfo holds a colon of its own, which the port removal must not read as a port. + //nolint:gosec // test URLs carrying userinfo, not credentials + userinfoDefaultPort = "https://user:pw@xYz.cOm:443/folder" + //nolint:gosec // test URLs carrying userinfo, not credentials + userinfoNormalized = "https://user:pw@xyz.com/folder" +) + func TestUrlnorm(t *testing.T) { testCases := []struct { url string expected string }{ { - url: "HTTPs://xYz.cOm:443/folder//file", + url: mixedCaseDefaultPort, expected: "https://xyz.com/folder/file", }, { @@ -28,6 +47,45 @@ func TestUrlnorm(t *testing.T) { url: "postGRES://xYz.cOm:5432/folder//file", expected: "postgres://xyz.com:5432/folder/file", }, + { + url: userinfoDefaultPort, + expected: userinfoNormalized, + }, + { + url: "https://[2001:DB8::1]:443/folder", + expected: "https://[2001:db8::1]/folder", + }, + { + url: ipv6NonDefaultPort, + expected: ipv6NonDefaultPort, + }, + { + url: degenerateHostHTTPS, + expected: degenerateHostHTTPS, + }, + { + url: degenerateHostHTTP, + expected: degenerateHostHTTP, + }, + { + // a run of slashes collapses to one, however long the run is + url: "https://xyz.com/a//b///c////d", + expected: "https://xyz.com/a/b/c/d", + }, + { + url: "https://xyz.com////", + expected: "https://xyz.com/", + }, + { + url: "https://:]:443", + expected: "https://:]:443", + }, + { + // the host is ":80" on port 80, so the removal has to run twice. Emptying the host + // drops the "//" as well, which url.URL.String has always done. + url: "http://:80:80", + expected: "http:", + }, } for _, toPin := range testCases { @@ -37,6 +95,10 @@ func TestUrlnorm(t *testing.T) { require.NoError(t, err) NormalizeURL(u) - assert.EqualT(t, testCase.expected, u.String()) + normalized := u.String() + assert.EqualT(t, testCase.expected, normalized) + + _, err = url.Parse(normalized) + require.NoErrorf(t, err, "normalizing %q yielded a URL that no longer parses", testCase.url) } } diff --git a/internal/testdata/fuzz/FuzzNormalizeURL/1c6127448518abde b/internal/testdata/fuzz/FuzzNormalizeURL/1c6127448518abde new file mode 100644 index 0000000..68db737 --- /dev/null +++ b/internal/testdata/fuzz/FuzzNormalizeURL/1c6127448518abde @@ -0,0 +1,2 @@ +go test fuzz v1 +string("https://:]:443") diff --git a/internal/testdata/fuzz/FuzzNormalizeURL/3ed1453c138071f6 b/internal/testdata/fuzz/FuzzNormalizeURL/3ed1453c138071f6 new file mode 100644 index 0000000..a3fda5b --- /dev/null +++ b/internal/testdata/fuzz/FuzzNormalizeURL/3ed1453c138071f6 @@ -0,0 +1,2 @@ +go test fuzz v1 +string("http://:80:80")