From c62b5f122bdf4e07a581cefd811caa7db1debf88 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Fri, 13 May 2022 18:37:35 +0200 Subject: [PATCH] fix(netxlite): consolidate IPv4/IPv6 checking code Originally https://github.com/bassosimone/websteps-illustrated/commit/966e7f7cdde534dca8beaf54dda08746660cc324 See https://github.com/ooni/probe/issues/2096 --- internal/netxlite/quirks.go | 20 ++++++++------------ internal/netxlite/quirks_test.go | 12 ++++++++++++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/netxlite/quirks.go b/internal/netxlite/quirks.go index e705c32b72..72d5fa8314 100644 --- a/internal/netxlite/quirks.go +++ b/internal/netxlite/quirks.go @@ -2,6 +2,7 @@ package netxlite import ( "errors" + "net" "strings" ) @@ -52,26 +53,21 @@ func quirkReduceErrors(errorslist []error) error { // before IPv6. Dialers SHOULD call this code. // // It saddens me to have this quirk, but it is here to pair -// with quirkReduceErrors, which assumes that . +// with quirkReduceErrors, which assumes that IPv4 addrs +// appear before IPv6 addrs . +// +// Note: this function will skip any input that is not not +// a valid IPv4 or IPv6 address. // // See TODO(https://github.com/ooni/probe/issues/1779). func quirkSortIPAddrs(addrs []string) (out []string) { - isIPv6 := func(x string) bool { - // This check for identifying IPv6 is discussed - // at https://stackoverflow.com/questions/22751035 - // and seems good-enough for our purposes. - return strings.Contains(x, ":") - } - isIPv4 := func(x string) bool { - return !isIPv6(x) - } for _, addr := range addrs { - if isIPv4(addr) { + if net.ParseIP(addr) != nil && !isIPv6(addr) { out = append(out, addr) } } for _, addr := range addrs { - if isIPv6(addr) { + if net.ParseIP(addr) != nil && isIPv6(addr) { out = append(out, addr) } } diff --git a/internal/netxlite/quirks_test.go b/internal/netxlite/quirks_test.go index 62bdf63b23..de622607e0 100644 --- a/internal/netxlite/quirks_test.go +++ b/internal/netxlite/quirks_test.go @@ -57,7 +57,9 @@ func TestQuirkSortIPAddrs(t *testing.T) { addrs := []string{ "::1", "192.168.1.2", + "x.org", // ensure we skip non IP addrs "2a00:1450:4002:404::2004", + "example.com", // ensure we skip non IP addrs "142.250.184.36", "2604:8800:5000:82:466:38ff:fecb:d46e", "198.145.29.83", @@ -83,4 +85,14 @@ func TestQuirkSortIPAddrs(t *testing.T) { t.Fatal("expected nil output") } }) + + t.Run("with non-IP addrs", func(t *testing.T) { + addrs := []string{ + "example.com", + "x.org", + } + if quirkSortIPAddrs(addrs) != nil { + t.Fatal("expected nil output") + } + }) }