Fix/normalize url parseable host

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()
 }

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)
+		}
+	}
+}

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",
+	}
+}

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)
 	}
 }

internal/testdata/fuzz/FuzzNormalizeURL/1c6127448518abde

@@ -0,0 +1,2 @@
+go test fuzz v1
+string("https://:]:443")

internal/testdata/fuzz/FuzzNormalizeURL/3ed1453c138071f6

@@ -0,0 +1,2 @@
+go test fuzz v1
+string("http://:80:80")