net/netip: export Prefix.Compare, fix ordering

Fixes #61642

Co-authored-by: David Anderson <dave@natulte.net>
Change-Id: I54795763bdc5f62da469c2ae20618c36b64396f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/700355
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/api/next/61642.txt b/api/next/61642.txt
new file mode 100644 (file)
index 0000000..dd67874
--- /dev/null
+++ b/api/next/61642.txt
@@ -0,0 +1 @@
+pkg net/netip, method (Prefix) Compare(Prefix) int #61642

diff --git a/doc/next/6-stdlib/99-minor/net/netip/61642.md b/doc/next/6-stdlib/99-minor/net/netip/61642.md
new file mode 100644 (file)
index 0000000..3d79f2e
--- /dev/null
+++ b/doc/next/6-stdlib/99-minor/net/netip/61642.md
@@ -0,0 +1 @@
+The new [Prefix.Compare] method compares two prefixes.

diff --git a/src/net/netip/export_test.go b/src/net/netip/export_test.go
index b2fae1aa47eedc2c9fcdfe38af20b2bf2495b42e..777a76a6b264014bd24db0b55452c8bda7c6835b 100644 (file)
--- a/src/net/netip/export_test.go
+++ b/src/net/netip/export_test.go
@@ -34,5 +34,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 35abfd3241bc13aa8beebcf94556531eefd8225a..b1b15b47287de28c90cc93281452627fa2401c23 100644 (file)
--- a/src/net/netip/netip.go
+++ b/src/net/netip/netip.go
@@ -1330,21 +1330,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 ea03f9a9e72473f4b7dadfd2d1989fc3cef4fac2..71e39021ca89696f5d5c145e0c720e93e926f0e4 100644 (file)
--- a/src/net/netip/netip_test.go
+++ b/src/net/netip/netip_test.go
@@ -1123,6 +1123,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)
@@ -1148,10 +1151,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, Prefix.Compare)
        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)
        }