dnscrypt-proxy icon indicating copy to clipboard operation
dnscrypt-proxy copied to clipboard

Usage of netip leading to bugs

Open kfken opened this issue 3 years ago • 1 comments

There's a couple errors related to use of netip in baee50f1dce05f54796db6dd5092d8673914a434 that I think need to be addressed:

  1. The new xtransport.ParseIP() doesn't match the intended usage. In the original code, it retuned nil if the host wasn't an IP. netip.ParseAddr() returns err == nil when host is an IP. So by just swapping ip with err, in the comparisons, the logic is swapped and the wrong code path is taken.

  2. Using netip.IPv4Unspecified() doesn't match the original intent of the code that was replaced. IPv4Unspecified() returns 0.0.0.0, but the original code was checking if (for example) cachedIP == nil. net.IP is a []byte, and the zero value is nil. So the new zero value that should be used from netip is netip.Addr{}.

  3. Also, using IPv4Unspecified() completely misses IPv6 resolvers (yes they really are used). There's also usage of netip.Is4() which similarly breaks support for v6 addresses.

kfken avatar Aug 10 '22 20:08 kfken

Thanks!

These changes have been reverted. The semantics are indeed quite different, so we need to be extremely careful with these changes.

jedisct1 avatar Aug 10 '22 20:08 jedisct1