False positive when comparing parts of IP addresses

[marten-seemann]

staticcheck doesn't like to following function:

func sameV6Net(a, b net.IP) bool {
	return len(a) == net.IPv6len && len(b) == net.IPv6len && bytes.Equal(a[0:8], b[0:8])
}

It complains: use net.IP.Equal to compare net.IPs, not bytes.Equal (SA1021).

This would be reasonable thing to do when using bytes.Equal(a, b), but not when using sub-slices of a and b.

[ainar-g]

There are actually a couple of less-low-level ways of doing that:

var mask64 = net.CIDRMask(64, 128)

func sameIPv6NetIPNetContains(a, b net.IP) (ok bool) {
        return (&net.IPNet{IP: a, Mask: mask64}).Contains(b)
}

var mask64 = net.CIDRMask(64, 128)

func sameIPv6NetIPMask(a, b net.IP) (ok bool) {
        return a.Mask(mask64).Equal(b.Mask(mask64))
}

Unfortunately, both of those lose to bytes.Equal on my machine. Probably because the Go compiler optimises the hell out of it.

goos: linux
goarch: amd64
cpu: AMD Ryzen 7 PRO 4750U with Radeon Graphics
BenchmarkSameIPv6Net/ipnet_contains-16          70799658         16.80 ns/op
BenchmarkSameIPv6Net/bytes_equal-16             398179159         2.931 ns/op
BenchmarkSameIPv6Net/ip_mask-16                 10471532        124.1 ns/op
PASS
ok      command-line-arguments  4.916s

https://play.golang.org/p/ZMbDDcHrRk5

[dominikh]

We should fix this. I haven't decided on the best solution yet. The current implementation is based on types only. And while we certainly could detect some problematic values, we'll never be fully free of false positives. Maybe you slice in one function, then pass the slices to another function that does the check…

This is where it'd be useful to have statistics, namely how often the check matches correctly. Is it even worth having?