From 9c502b50721e6bea61fd7363ea7277b9b4bdd8e5 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 14 Dec 2023 11:27:04 -0800 Subject: [PATCH] net/netip: export Prefix.Compare, fix ordering Fixes #61642 --- api/go1.22.txt | 1 + src/net/netip/export_test.go | 2 -- src/net/netip/netip.go | 18 +++++----- src/net/netip/netip_test.go | 65 +++++++++++++++++++++++++++++++++++- 4 files changed, 75 insertions(+), 11 deletions(-) diff --git a/api/go1.22.txt b/api/go1.22.txt index 86eb80deafe4a..d2a1ae7a88748 100644 --- a/api/go1.22.txt +++ b/api/go1.22.txt @@ -131,6 +131,7 @@ pkg net/http, func ServeFileFS(ResponseWriter, *Request, fs.FS, string) #51971 pkg net/http, method (*Request) PathValue(string) string #61410 pkg net/http, method (*Request) SetPathValue(string, string) #61410 pkg net/netip, method (AddrPort) Compare(AddrPort) int #61642 +pkg net/netip, method (Prefix) Compare(Prefix) int #61642 pkg os, method (*File) WriteTo(io.Writer) (int64, error) #58808 pkg reflect, func PtrTo //deprecated #59599 pkg reflect, func TypeFor[$0 interface{}]() Type #60088 diff --git a/src/net/netip/export_test.go b/src/net/netip/export_test.go index 72347ee01bd1d..59971fa2e4e51 100644 --- a/src/net/netip/export_test.go +++ b/src/net/netip/export_test.go @@ -28,5 +28,3 @@ var TestAppendToMarshal = testAppendToMarshal func (a Addr) IsZero() bool { return a.isZero() } func (p Prefix) IsZero() bool { return p.isZero() } - -func (p Prefix) Compare(p2 Prefix) int { return p.compare(p2) } diff --git a/src/net/netip/netip.go b/src/net/netip/netip.go index 9acc48a6e02b8..4be1f5a841fd8 100644 --- a/src/net/netip/netip.go +++ b/src/net/netip/netip.go @@ -1261,21 +1261,23 @@ func (p Prefix) isZero() bool { return p == Prefix{} } // IsSingleIP reports whether p contains exactly one IP. func (p Prefix) IsSingleIP() bool { return p.IsValid() && p.Bits() == p.ip.BitLen() } -// compare returns an integer comparing two prefixes. +// Compare returns an integer comparing two prefixes. // The result will be 0 if p == p2, -1 if p < p2, and +1 if p > p2. // Prefixes sort first by validity (invalid before valid), then -// address family (IPv4 before IPv6), then prefix length, then -// address. -// -// Unexported for Go 1.22 because we may want to compare by p.Addr first. -// See post-acceptance discussion on go.dev/issue/61642. -func (p Prefix) compare(p2 Prefix) int { - if c := cmp.Compare(p.Addr().BitLen(), p2.Addr().BitLen()); c != 0 { +// address family (IPv4 before IPv6), then masked prefix address, then +// prefix length, then unmasked address. +func (p Prefix) Compare(p2 Prefix) int { + // Aside from sorting based on the masked address, this use of + // Addr.Compare also enforces the valid vs. invalid and address + // family ordering for the prefix. + if c := p.Masked().Addr().Compare(p2.Masked().Addr()); c != 0 { return c } + if c := cmp.Compare(p.Bits(), p2.Bits()); c != 0 { return c } + return p.Addr().Compare(p2.Addr()) } diff --git a/src/net/netip/netip_test.go b/src/net/netip/netip_test.go index a748ac34f13cc..a06b25070df21 100644 --- a/src/net/netip/netip_test.go +++ b/src/net/netip/netip_test.go @@ -953,6 +953,9 @@ func TestPrefixCompare(t *testing.T) { {mustPrefix("fe80::/48"), mustPrefix("fe80::/64"), -1}, {mustPrefix("1.2.3.0/24"), mustPrefix("fe80::/8"), -1}, + + {mustPrefix("1.2.3.0/24"), mustPrefix("1.2.3.4/24"), -1}, + {mustPrefix("1.2.3.0/24"), mustPrefix("1.2.3.0/28"), -1}, } for _, tt := range tests { got := tt.a.Compare(tt.b) @@ -978,10 +981,70 @@ func TestPrefixCompare(t *testing.T) { Prefix{}, mustPrefix("fe80::/48"), mustPrefix("1.2.0.0/24"), + mustPrefix("1.2.3.4/24"), + mustPrefix("1.2.3.0/28"), } slices.SortFunc(values, func(a, b Prefix) int { return a.Compare(b) }) got := fmt.Sprintf("%s", values) - want := `[invalid Prefix 1.2.0.0/16 1.2.0.0/24 1.2.3.0/24 fe80::/48 fe80::/64 fe90::/64]` + want := `[invalid Prefix 1.2.0.0/16 1.2.0.0/24 1.2.3.0/24 1.2.3.4/24 1.2.3.0/28 fe80::/48 fe80::/64 fe90::/64]` + if got != want { + t.Errorf("unexpected sort\n got: %s\nwant: %s\n", got, want) + } + + // Lists from + // https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml and + // https://www.iana.org/assignments/ipv6-address-space/ipv6-address-space.xhtml, + // to verify that the sort order matches IANA's conventional + // ordering. + values = []Prefix{ + mustPrefix("0.0.0.0/8"), + mustPrefix("127.0.0.0/8"), + mustPrefix("10.0.0.0/8"), + mustPrefix("203.0.113.0/24"), + mustPrefix("169.254.0.0/16"), + mustPrefix("192.0.0.0/24"), + mustPrefix("240.0.0.0/4"), + mustPrefix("192.0.2.0/24"), + mustPrefix("192.0.0.170/32"), + mustPrefix("198.18.0.0/15"), + mustPrefix("192.0.0.8/32"), + mustPrefix("0.0.0.0/32"), + mustPrefix("192.0.0.9/32"), + mustPrefix("198.51.100.0/24"), + mustPrefix("192.168.0.0/16"), + mustPrefix("192.0.0.10/32"), + mustPrefix("192.175.48.0/24"), + mustPrefix("192.52.193.0/24"), + mustPrefix("100.64.0.0/10"), + mustPrefix("255.255.255.255/32"), + mustPrefix("192.31.196.0/24"), + mustPrefix("172.16.0.0/12"), + mustPrefix("192.0.0.0/29"), + mustPrefix("192.88.99.0/24"), + mustPrefix("fec0::/10"), + mustPrefix("6000::/3"), + mustPrefix("fe00::/9"), + mustPrefix("8000::/3"), + mustPrefix("0000::/8"), + mustPrefix("0400::/6"), + mustPrefix("f800::/6"), + mustPrefix("e000::/4"), + mustPrefix("ff00::/8"), + mustPrefix("a000::/3"), + mustPrefix("fc00::/7"), + mustPrefix("1000::/4"), + mustPrefix("0800::/5"), + mustPrefix("4000::/3"), + mustPrefix("0100::/8"), + mustPrefix("c000::/3"), + mustPrefix("fe80::/10"), + mustPrefix("0200::/7"), + mustPrefix("f000::/5"), + mustPrefix("2000::/3"), + } + slices.SortFunc(values, func(a, b Prefix) int { return a.Compare(b) }) + got = fmt.Sprintf("%s", values) + want = `[0.0.0.0/8 0.0.0.0/32 10.0.0.0/8 100.64.0.0/10 127.0.0.0/8 169.254.0.0/16 172.16.0.0/12 192.0.0.0/24 192.0.0.0/29 192.0.0.8/32 192.0.0.9/32 192.0.0.10/32 192.0.0.170/32 192.0.2.0/24 192.31.196.0/24 192.52.193.0/24 192.88.99.0/24 192.168.0.0/16 192.175.48.0/24 198.18.0.0/15 198.51.100.0/24 203.0.113.0/24 240.0.0.0/4 255.255.255.255/32 ::/8 100::/8 200::/7 400::/6 800::/5 1000::/4 2000::/3 4000::/3 6000::/3 8000::/3 a000::/3 c000::/3 e000::/4 f000::/5 f800::/6 fc00::/7 fe00::/9 fe80::/10 fec0::/10 ff00::/8]` if got != want { t.Errorf("unexpected sort\n got: %s\nwant: %s\n", got, want) }