Skip to content

Commit d84edb8

Browse files
fredbiclaude
andcommitted
fix: remove repeated default ports from a URL host in linear time
removeDefaultPort stripped one ":80" (or ":443") at a time and re-parsed the whole remaining host after each strip. With lenient colon parsing (Go 1.25, or GODEBUG urlstrictcolons=0 since Go 1.26), url.Parse accepts a host such as ":80:80:...:80", so a single $ref of a few hundred KB kept jsonreference.New busy for tens of seconds: the cost was quadratic in the length of the host. The trailing repetitions are now counted in one scan and stripped at once. The shortened host is parsed at most twice: if it no longer parses, a single repetition stays, which is where the one-by-one removal stopped. Every shortened host but the last still ends with the default port, which url.Parse already accepted on the original host, so the result is the same as before and normalizing stays idempotent. With the default strict colon parsing of Go 1.26 such hosts are rejected by url.Parse before normalization, so v1.0.2 and later are only exposed when an application opts out with urlstrictcolons=0. v1.0.1 (go 1.25.0) is exposed by default when built with Go 1.25. FuzzRemoveDefaultPort checks the new implementation against the former one, and TestRemoveDefaultPortLinear pins the cost on 100 000 repetitions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
1 parent 2b801d1 commit d84edb8

2 files changed

Lines changed: 145 additions & 12 deletions

File tree

‎internal/normalize_url.go‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -54,21 +54,43 @@ func lowercaseHost(u *url.URL) {
5454
// shortened host is parsed before being kept. url.Parse reads "https://:a:443" as the host ":a"
5555
// on port 443, and ":a" on its own is an invalid port, so "https://:a" no longer parses.
5656
//
57-
// A degenerate authority can spell a default port twice - url.Parse reads "http://:80:80" as the
58-
// host ":80" on port 80 - so removal repeats until nothing more comes off. Each pass shortens the
59-
// host, so the loop ends, and normalizing the result again changes nothing.
57+
// A degenerate authority can spell a default port more than once - with GODEBUG urlstrictcolons=0,
58+
// url.Parse reads "http://:80:80" as the host ":80" on port 80 - so every trailing repetition comes
59+
// off, and normalizing the result again changes nothing.
60+
//
61+
// The repetitions are counted in one scan and the shortened host is parsed at most twice, so the
62+
// cost stays linear in the length of the host: stripping and re-parsing one repetition at a time
63+
// was quadratic, and a $ref spelling ":80" a few ten thousand times took seconds to normalize.
64+
//
65+
// Every shortened host but the last still ends with the default port, which url.Parse accepted
66+
// on the original host already, so only the last one may fail to parse. When it does, a single
67+
// repetition stays: that is where removing them one by one would have stopped.
6068
func removeDefaultPort(u *url.URL) {
61-
for {
62-
port := u.Port()
63-
if port == "" || port != defaultPortForScheme(strings.ToLower(u.Scheme)) {
64-
return
65-
}
69+
port := u.Port()
70+
if port == "" || port != defaultPortForScheme(strings.ToLower(u.Scheme)) {
71+
return
72+
}
6673

67-
host := strings.TrimSuffix(u.Host, ":"+port)
68-
if _, err := url.Parse("//" + host); err != nil {
69-
return
70-
}
74+
suffix := ":" + port
75+
host := u.Host
76+
repeats := 0
77+
for strings.HasSuffix(host, suffix) {
78+
host = host[:len(host)-len(suffix)]
79+
repeats++
80+
}
81+
82+
if _, err := url.Parse("//" + host); err == nil {
83+
u.Host = host
84+
85+
return
86+
}
87+
88+
if repeats == 1 { // the host as it came is the only one to keep
89+
return
90+
}
7191

92+
host += suffix
93+
if _, err := url.Parse("//" + host); err == nil {
7294
u.Host = host
7395
}
7496
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2015-2025 go-swagger maintainers
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package internal
5+
6+
import (
7+
"net/url"
8+
"strings"
9+
"testing"
10+
"time"
11+
12+
"github.com/go-openapi/testify/v2/assert"
13+
"github.com/go-openapi/testify/v2/require"
14+
)
15+
16+
// TestRemoveDefaultPortLinear pins the cost of removeDefaultPort on a host spelling the default
17+
// port over and over. Removing one repetition at a time and re-parsing what remained was
18+
// quadratic: 40 000 repetitions took seconds.
19+
//
20+
// The host is set directly on the url.URL: since go1.26, url.Parse rejects such a host unless
21+
// GODEBUG urlstrictcolons=0, but callers opting out of the strict parsing still get there.
22+
func TestRemoveDefaultPortLinear(t *testing.T) {
23+
const repeats = 100_000
24+
25+
for _, tc := range []struct{ scheme, port string }{
26+
{"http", defaultHTTPPort},
27+
{"https", defaultHTTPSPort},
28+
} {
29+
t.Run(tc.scheme, func(t *testing.T) {
30+
u := &url.URL{Scheme: tc.scheme, Host: strings.Repeat(":"+tc.port, repeats), Path: "/a.json"}
31+
32+
start := time.Now()
33+
removeDefaultPort(u)
34+
elapsed := time.Since(start)
35+
36+
assert.Empty(t, u.Host)
37+
assert.Lessf(t, elapsed, time.Second, "removing %d default ports took %v", repeats, elapsed)
38+
})
39+
}
40+
}
41+
42+
func TestRemoveDefaultPort(t *testing.T) {
43+
for _, tc := range []struct{ scheme, host, expected string }{
44+
{"http", "xyz.com:80", "xyz.com"},
45+
{"https", "xyz.com:443", "xyz.com"},
46+
{"http", "xyz.com:443", "xyz.com:443"},
47+
{"https", "xyz.com:80", "xyz.com:80"},
48+
{"ftp", "xyz.com:80", "xyz.com:80"},
49+
{"http", "xyz.com:8080", "xyz.com:8080"},
50+
{"http", "xyz.com", "xyz.com"},
51+
{"https", "[2001:db8::1]:443", "[2001:db8::1]"},
52+
{"http", ":80", ""},
53+
{"http", ":80:80:80", ""},
54+
// the host would become ":a", which is an invalid port: the last repetition stays
55+
{"http", ":a:80", ":a:80"},
56+
{"http", ":a:80:80:80", ":a:80"},
57+
} {
58+
t.Run(tc.scheme+"://"+tc.host, func(t *testing.T) {
59+
u := &url.URL{Scheme: tc.scheme, Host: tc.host}
60+
removeDefaultPort(u)
61+
assert.EqualT(t, tc.expected, u.Host)
62+
})
63+
}
64+
}
65+
66+
// FuzzRemoveDefaultPort checks removeDefaultPort against the former implementation, which
67+
// removed one repetition of the default port at a time and re-parsed the host after each.
68+
func FuzzRemoveDefaultPort(f *testing.F) {
69+
for _, seed := range []string{
70+
"xyz.com:80", "xyz.com:443", ":80", ":80:80", ":443:443", ":a:80", ":a:80:80",
71+
":a:443:443:443", "[::1]:80", "[::1]:80:80", ":]:443", "a%20b:80", "80:80:80",
72+
} {
73+
f.Add(true, seed)
74+
f.Add(false, seed)
75+
}
76+
77+
f.Fuzz(func(t *testing.T, https bool, host string) {
78+
if len(host) > 1024 {
79+
return // the reference implementation is quadratic
80+
}
81+
82+
scheme := "http"
83+
if https {
84+
scheme = "https"
85+
}
86+
87+
got := &url.URL{Scheme: scheme, Host: host}
88+
removeDefaultPort(got)
89+
90+
want := &url.URL{Scheme: scheme, Host: host}
91+
removeDefaultPortOneByOne(want)
92+
93+
require.EqualTf(t, want.Host, got.Host, "removing the default port from %s://%s", scheme, host)
94+
})
95+
}
96+
97+
func removeDefaultPortOneByOne(u *url.URL) {
98+
for {
99+
port := u.Port()
100+
if port == "" || port != defaultPortForScheme(strings.ToLower(u.Scheme)) {
101+
return
102+
}
103+
104+
host := strings.TrimSuffix(u.Host, ":"+port)
105+
if _, err := url.Parse("//" + host); err != nil {
106+
return
107+
}
108+
109+
u.Host = host
110+
}
111+
}

0 commit comments

Comments
 (0)