From 56c1b4d8fb563c794a50ac55e454b7c7c227cc58 Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Mon, 4 Sep 2023 18:04:38 +0000 Subject: [PATCH 1/6] TIL net.SplitHostPort requires ipv6 brackets Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 30 ++++++++++++------------------ conformance/utils/http/http.go | 31 +++++++++++++------------------ 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index 8e4dd884ad..a4a78fffce 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -58,7 +58,7 @@ func (m *MeshPod) MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, t.Helper() http.AwaitConvergence(t, timeoutConfig.RequiredConsecutiveSuccesses, timeoutConfig.MaxTimeToConsistency, func(elapsed time.Duration) bool { - req := makeRequest(exp.Request) + req := makeRequest(t, exp.Request) resp, err := m.request(req) if err != nil { @@ -76,12 +76,12 @@ func (m *MeshPod) MakeRequestAndExpectEventuallyConsistentResponse(t *testing.T, t.Logf("Request passed") } -func makeRequest(r http.Request) []string { +func makeRequest(t *testing.T, r http.Request) []string { protocol := strings.ToLower(r.Protocol) if protocol == "" { protocol = "http" } - host := calculateHost(r.Host, protocol) + host := calculateHost(t, r.Host, protocol) args := []string{"client", fmt.Sprintf("%s://%s%s", protocol, host, r.Path)} if r.Method != "" { args = append(args, "--method="+r.Method) @@ -113,32 +113,26 @@ func compareRequest(exp http.ExpectedResponse, resp Response) error { return nil } -// copied from conformance/http to get the ipv6 matching -func calculateHost(reqHost, scheme string) string { +func calculateHost(t *testing.T, reqHost, scheme string) string { host, port, err := net.SplitHostPort(reqHost) + if strings.Contains(err.Error(), "too many colons in address") { + // This is an IPv6 address; assume it's valid ipv6 + // Assume caller won't add a port without brackets + host, port, err = net.SplitHostPort("[" + reqHost + "]") + } if err != nil { + t.Logf("Failed to parse host %q: %v", reqHost, err) return reqHost } if strings.ToLower(scheme) == "http" && port == "80" { - return ipv6SafeHost(host) + return host } if strings.ToLower(scheme) == "https" && port == "443" { - return ipv6SafeHost(host) + return host } return reqHost } -func ipv6SafeHost(host string) string { - // We assume that host is a literal IPv6 address if host has - // colons. - // Per https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2. - // This is like net.JoinHostPort, but we don't need a port. - if strings.Contains(host, ":") { - return "[" + host + "]" - } - return host -} - func (m *MeshPod) request(args []string) (Response, error) { container := "echo" diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index a86ea48e09..c0dd724d7d 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -112,7 +112,7 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch } path, query, _ := strings.Cut(expected.Request.Path, "?") - reqURL := url.URL{Scheme: scheme, Host: calculateHost(gwAddr, scheme), Path: path, RawQuery: query} + reqURL := url.URL{Scheme: scheme, Host: calculateHost(t, gwAddr, scheme), Path: path, RawQuery: query} t.Logf("Making %s request to %s", expected.Request.Method, reqURL.String()) @@ -145,29 +145,24 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch // case of any error, the input gwAddr will be returned as the default. // // [HTTP spec]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 -func calculateHost(gwAddr, scheme string) string { - host, port, err := net.SplitHostPort(gwAddr) +func calculateHost(t *testing.T, reqHost, scheme string) string { + host, port, err := net.SplitHostPort(reqHost) + if strings.Contains(err.Error(), "too many colons in address") { + // This is an IPv6 address; assume it's valid ipv6 + // Assume caller won't add a port without brackets + host, port, err = net.SplitHostPort("[" + reqHost + "]") + } if err != nil { - return gwAddr + t.Logf("Failed to parse host %q: %v", reqHost, err) + return reqHost } if strings.ToLower(scheme) == "http" && port == "80" { - return ipv6SafeHost(host) + return host } if strings.ToLower(scheme) == "https" && port == "443" { - return ipv6SafeHost(host) - } - return gwAddr -} - -func ipv6SafeHost(host string) string { - // We assume that host is a literal IPv6 address if host has - // colons. - // Per https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2. - // This is like net.JoinHostPort, but we don't need a port. - if strings.Contains(host, ":") { - return "[" + host + "]" + return host } - return host + return reqHost } // AwaitConvergence runs the given function until it returns 'true' `threshold` times in a row. From eec07b0ff30913b583107f05c595514acafe1c81 Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Mon, 4 Sep 2023 22:19:46 +0000 Subject: [PATCH 2/6] Fix small bug Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 3 ++- conformance/utils/http/http.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index a4a78fffce..5f3571f90f 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -118,7 +118,8 @@ func calculateHost(t *testing.T, reqHost, scheme string) string { if strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets - host, port, err = net.SplitHostPort("[" + reqHost + "]") + reqHost = "[" + reqHost + "]" + host, port, err = net.SplitHostPort(reqHost) } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index c0dd724d7d..f247ec67a5 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -150,7 +150,8 @@ func calculateHost(t *testing.T, reqHost, scheme string) string { if strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets - host, port, err = net.SplitHostPort("[" + reqHost + "]") + reqHost = "[" + reqHost + "]" + host, port, err = net.SplitHostPort(reqHost) } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) From e4659e6a6ab5b0cdbe199456ac9d19700e7dbaf8 Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Mon, 4 Sep 2023 23:14:39 +0000 Subject: [PATCH 3/6] Turns out port is required Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 4 ++++ conformance/utils/http/http.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index 5f3571f90f..665fa588b2 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -121,6 +121,10 @@ func calculateHost(t *testing.T, reqHost, scheme string) string { reqHost = "[" + reqHost + "]" host, port, err = net.SplitHostPort(reqHost) } + // If there's no port, we're ok with that + if strings.Contains(err.Error(), "missing port in address") { + return reqHost + } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) return reqHost diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index f247ec67a5..ac1323dbb3 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -153,6 +153,10 @@ func calculateHost(t *testing.T, reqHost, scheme string) string { reqHost = "[" + reqHost + "]" host, port, err = net.SplitHostPort(reqHost) } + // If there's no port, we're ok with that + if strings.Contains(err.Error(), "missing port in address") { + return reqHost + } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) return reqHost From d8c6dd8182529ce91c202acb284120b1815c9499 Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Tue, 5 Sep 2023 00:58:11 +0000 Subject: [PATCH 4/6] Better err handling Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 6 +----- conformance/utils/http/http.go | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index 665fa588b2..8889f6ea65 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -115,16 +115,12 @@ func compareRequest(exp http.ExpectedResponse, resp Response) error { func calculateHost(t *testing.T, reqHost, scheme string) string { host, port, err := net.SplitHostPort(reqHost) - if strings.Contains(err.Error(), "too many colons in address") { + if err != nil && strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets reqHost = "[" + reqHost + "]" host, port, err = net.SplitHostPort(reqHost) } - // If there's no port, we're ok with that - if strings.Contains(err.Error(), "missing port in address") { - return reqHost - } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) return reqHost diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index ac1323dbb3..d3064ee606 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -147,16 +147,12 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch // [HTTP spec]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 func calculateHost(t *testing.T, reqHost, scheme string) string { host, port, err := net.SplitHostPort(reqHost) - if strings.Contains(err.Error(), "too many colons in address") { + if err != nil && strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets reqHost = "[" + reqHost + "]" host, port, err = net.SplitHostPort(reqHost) } - // If there's no port, we're ok with that - if strings.Contains(err.Error(), "missing port in address") { - return reqHost - } if err != nil { t.Logf("Failed to parse host %q: %v", reqHost, err) return reqHost From 52b6e260f4a9ae58a7567f290271828e48ad283e Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Tue, 5 Sep 2023 03:56:17 +0000 Subject: [PATCH 5/6] Should be the last fix Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 17 ++++++++++++++--- conformance/utils/http/http.go | 29 ++++++++++++++++++++--------- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index 8889f6ea65..b3f218bee6 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -114,7 +114,7 @@ func compareRequest(exp http.ExpectedResponse, resp Response) error { } func calculateHost(t *testing.T, reqHost, scheme string) string { - host, port, err := net.SplitHostPort(reqHost) + host, port, err := net.SplitHostPort(reqHost) // note: this will strip brackets of an IPv6 address if err != nil && strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets @@ -126,14 +126,25 @@ func calculateHost(t *testing.T, reqHost, scheme string) string { return reqHost } if strings.ToLower(scheme) == "http" && port == "80" { - return host + return ipv6SafeHost(host) } if strings.ToLower(scheme) == "https" && port == "443" { - return host + return ipv6SafeHost(host) } return reqHost } +func ipv6SafeHost(host string) string { + // We assume that host is a literal IPv6 address if host has + // colons. + // Per https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2. + // This is like net.JoinHostPort, but we don't need a port. + if strings.Contains(host, ":") { + return "[" + host + "]" + } + return host +} + func (m *MeshPod) request(args []string) (Response, error) { container := "echo" diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index d3064ee606..30c0b72f95 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -145,25 +145,36 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch // case of any error, the input gwAddr will be returned as the default. // // [HTTP spec]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 -func calculateHost(t *testing.T, reqHost, scheme string) string { - host, port, err := net.SplitHostPort(reqHost) +func calculateHost(t *testing.T, gwAddr, scheme string) string { + host, port, err := net.SplitHostPort(gwAddr) // note: this will strip brackets of an IPv6 address if err != nil && strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6 // Assume caller won't add a port without brackets - reqHost = "[" + reqHost + "]" - host, port, err = net.SplitHostPort(reqHost) + gwAddr = "[" + gwAddr + "]" + host, port, err = net.SplitHostPort(gwAddr) } if err != nil { - t.Logf("Failed to parse host %q: %v", reqHost, err) - return reqHost + t.Logf("Failed to parse host %q: %v", gwAddr, err) + return gwAddr } if strings.ToLower(scheme) == "http" && port == "80" { - return host + return ipv6SafeHost(host) } if strings.ToLower(scheme) == "https" && port == "443" { - return host + return ipv6SafeHost(host) } - return reqHost + return gwAddr +} + +func ipv6SafeHost(host string) string { + // We assume that host is a literal IPv6 address if host has + // colons. + // Per https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2. + // This is like net.JoinHostPort, but we don't need a port. + if strings.Contains(host, ":") { + return "[" + host + "]" + } + return host } // AwaitConvergence runs the given function until it returns 'true' `threshold` times in a row. From 88c8efad2473854448d71cf72b14f636a630c1c5 Mon Sep 17 00:00:00 2001 From: Keith Mattix II Date: Thu, 7 Sep 2023 15:03:42 +0000 Subject: [PATCH 6/6] de-dupe calculateHost Signed-off-by: Keith Mattix II --- conformance/utils/echo/pod.go | 35 +--------------------------------- conformance/utils/http/http.go | 6 +++--- 2 files changed, 4 insertions(+), 37 deletions(-) diff --git a/conformance/utils/echo/pod.go b/conformance/utils/echo/pod.go index b3f218bee6..fcd04ae29e 100644 --- a/conformance/utils/echo/pod.go +++ b/conformance/utils/echo/pod.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "fmt" - "net" "strings" "testing" "time" @@ -81,7 +80,7 @@ func makeRequest(t *testing.T, r http.Request) []string { if protocol == "" { protocol = "http" } - host := calculateHost(t, r.Host, protocol) + host := http.CalculateHost(t, r.Host, protocol) args := []string{"client", fmt.Sprintf("%s://%s%s", protocol, host, r.Path)} if r.Method != "" { args = append(args, "--method="+r.Method) @@ -113,38 +112,6 @@ func compareRequest(exp http.ExpectedResponse, resp Response) error { return nil } -func calculateHost(t *testing.T, reqHost, scheme string) string { - host, port, err := net.SplitHostPort(reqHost) // note: this will strip brackets of an IPv6 address - if err != nil && strings.Contains(err.Error(), "too many colons in address") { - // This is an IPv6 address; assume it's valid ipv6 - // Assume caller won't add a port without brackets - reqHost = "[" + reqHost + "]" - host, port, err = net.SplitHostPort(reqHost) - } - if err != nil { - t.Logf("Failed to parse host %q: %v", reqHost, err) - return reqHost - } - if strings.ToLower(scheme) == "http" && port == "80" { - return ipv6SafeHost(host) - } - if strings.ToLower(scheme) == "https" && port == "443" { - return ipv6SafeHost(host) - } - return reqHost -} - -func ipv6SafeHost(host string) string { - // We assume that host is a literal IPv6 address if host has - // colons. - // Per https://datatracker.ietf.org/doc/html/rfc3986#section-3.2.2. - // This is like net.JoinHostPort, but we don't need a port. - if strings.Contains(host, ":") { - return "[" + host + "]" - } - return host -} - func (m *MeshPod) request(args []string) (Response, error) { container := "echo" diff --git a/conformance/utils/http/http.go b/conformance/utils/http/http.go index 30c0b72f95..61fbf4eccb 100644 --- a/conformance/utils/http/http.go +++ b/conformance/utils/http/http.go @@ -112,7 +112,7 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch } path, query, _ := strings.Cut(expected.Request.Path, "?") - reqURL := url.URL{Scheme: scheme, Host: calculateHost(t, gwAddr, scheme), Path: path, RawQuery: query} + reqURL := url.URL{Scheme: scheme, Host: CalculateHost(t, gwAddr, scheme), Path: path, RawQuery: query} t.Logf("Making %s request to %s", expected.Request.Method, reqURL.String()) @@ -140,12 +140,12 @@ func MakeRequest(t *testing.T, expected *ExpectedResponse, gwAddr, protocol, sch return req } -// calculateHost will calculate the Host header as per [HTTP spec]. To +// CalculateHost will calculate the Host header as per [HTTP spec]. To // summarize, host will not include any port if it is implied from the scheme. In // case of any error, the input gwAddr will be returned as the default. // // [HTTP spec]: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23 -func calculateHost(t *testing.T, gwAddr, scheme string) string { +func CalculateHost(t *testing.T, gwAddr, scheme string) string { host, port, err := net.SplitHostPort(gwAddr) // note: this will strip brackets of an IPv6 address if err != nil && strings.Contains(err.Error(), "too many colons in address") { // This is an IPv6 address; assume it's valid ipv6