Skip to content
This repository has been archived by the owner on May 25, 2023. It is now read-only.

proposal: make IP zero value mean unspecified address? #7

Open
bradfitz opened this issue Apr 1, 2020 · 9 comments
Open

proposal: make IP zero value mean unspecified address? #7

bradfitz opened this issue Apr 1, 2020 · 9 comments

Comments

@bradfitz
Copy link
Contributor

bradfitz commented Apr 1, 2020

Perhaps the zero value of IP can be the (or, "an") unspecified address.

Then our version of https://golang.org/pkg/net/#IP.IsUnspecified can also respect a nil ipImpl as being unspecified.

We'd need to decide how to stringify it. Maybe the empty string, as in Listen(":80") where it's the empty string?

/cc @mdlayher @danderson

@mdlayher
Copy link
Member

mdlayher commented Apr 1, 2020

I think my concern with a zero-value unspecified address is that we support both IPv4 and IPv6, and it would be ambigious as to which (or both?) would be represented by that address.

I do think it would be reasonable to provide package-level vars like netaddr.IPv4Unspecified, netaddr.IPv6Unspecified, etc as stdlib does however.

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 1, 2020

I think it'd be nice to have three unspecified, though. Because once we add netaddr.ParseIPPort (for the newly added IPPort type), then we'd want ParseIPPort(":80") to have an IP that doesn't mean IPv4 or IPv6.

@mdlayher
Copy link
Member

mdlayher commented Apr 1, 2020

That's a good point. Would var ip netaddr.IP then imply "dual-stack unspecified IP address"? Would it be coerced to IPv4 or IPv6 unspecified in some other place in the API?

I think I am +1 on the idea at this point but am curious if it will lead to possible confusion.

@danderson
Copy link
Member

danderson commented Apr 1, 2020

I worry about this leading to a nil or NaN type of situation, where multiple opaque values that feel equal from the accessor methods are still != when used as map keys and whatnot. How do we reduce the potential for bugs there?

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 1, 2020

@danderson, the general solution to that is canonicalization methods for those who need it. (e.g. https://golang.org/pkg/time/#Time.UTC and https://godoc.org/inet.af/netaddr#IP.Unmap)

@mdlayher,

Would var ip netaddr.IP then imply "dual-stack unspecified IP address"?

Yes.

Would it be coerced to IPv4 or IPv6 unspecified in some other place in the API?

Yes. I at least plan to add As6() IP that would pack a 4-in-6, and it could also turn the zero value IP into the IPv6 unspecified address.

The bigger question is what Is4 and Is6 report for the zero value. Both true or both false? I can see either way but an leaning towards both false.

@mdlayher
Copy link
Member

mdlayher commented Apr 1, 2020

Yes. I at least plan to add As6() IP that would pack a 4-in-6, and it could also turn the zero value IP into the IPv6 unspecified address.

This would seem to imply a sort of symmetry with the existing Unmap, so perhaps Unmap should be renamed to As4? Then the zero value can be interpreted as IPv4 unspecified.

The bigger question is what Is4 and Is6 report for the zero value. Both true or both false? I can see either way but an leaning towards both false.

Both false IMO, until an explicit As4/6.

@bradfitz
Copy link
Contributor Author

bradfitz commented Apr 1, 2020

I don't want something named As4 because my experience with Go's To4 is that all code looks like:

    if ip4 := ip.To4(); ip4 != nil {
           ip = ip4
    }

I want to get that common pattern down to an expression.

Also, As4/To4 (whatever it's called) may fail for real IPv6 addresses, so you either have to return a zero value (and have all callers check it or be buggy) or return two things.

So I'd prefer to never have an As4/To4, and only have ones that try to shrink it internally (Unmap) and ones that expand it (Map? To6?)

@mdlayher
Copy link
Member

mdlayher commented Apr 1, 2020

Map sounds good to me. I agree that it is probably best to try to avoid another To4/To16 situation.

@mdlayher
Copy link
Member

It's been 8ish months and I personally haven't run into a need for this. Any further thoughts on the matter?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants