inetaf / netaddr

Network address types
BSD 3-Clause "New" or "Revised" License
714 stars 40 forks source link

proposal: Add an IsPublic() function #151

Open mellowdrifter opened 3 years ago

mellowdrifter commented 3 years ago

I currently have a bogon checker based on the net.IP package (https://github.com/mellowdrifter/bogons/blob/main/bogons.go#L46)

Is this something I could add into netaddr, or would it be considered a moving target considering new IPs are sometimes added in newer RFCs, though rarely

danderson commented 3 years ago

In theory, that would be useful. However, I'm wondering if it should be the inverse, i.e. IsBogon that returns true if an IP is in the bogon list. I'm thinking that just because "public" is a bit of a fluid term in IP space, and means different things to different people. I worry about fixing an API with one of those definitions.

Perhaps I'm overthinking it and "anything not defined as private or unroutable by RFCs is public" is a definition that makes sense to have. Thoughts @mdlayher @bradfitz?

danderson commented 3 years ago

To clarify, glancing at your code, it looks like you're implementing IsPublic as "not any of https://ipgeolocation.io/resources/bogon.html". Does that sound right?

mdlayher commented 3 years ago

I agree with Dave's assessment. In IPv6 there is already disagreement about whether ULA are "public" or not and I'm sure there are other ranges with similar questions.

bradfitz commented 3 years ago

I don't have anything super unique to add here. I'm fine adding a method named IsPublic as long as it's super well specified about what it means. Though even then people will probably use it based on its name alone without reading the docs.

danderson commented 3 years ago

So, I think our overall feeling is: adding predicates for "what kind of IP is this" we're generally okay with, even for things where the definition evolves a little over time. We're now just arguing about the precise semantics of "what does public mean?".

If you send a PR for IsPublic, with a docstring that defines precisely what "public" means (which I think can be something like "unicast IPs not listed in [list of RFCs in your current bogons.go]", I think I'm okay with adding IsPublic. There may be minor disagreements on the meaning of public, but something anchored in RFCs is something I think we can defend as a good definition.

mellowdrifter commented 3 years ago

Great! Let me work on this

On Thu, 18 Mar 2021 at 15:49, Dave Anderson @.***> wrote:

So, I think our overall feeling is: adding predicates for "what kind of IP is this" we're generally okay with, even for things where the definition evolves a little over time. We're now just arguing about the precise semantics of "what does public mean?".

If you send a PR for IsPublic, with a docstring that defines precisely what "public" means (which I think can be something like "unicast IPs not listed in ", I think I'm okay with adding IsPublic. There may be minor disagreements on the meaning of public, but something anchored in RFCs is something I think we can defend as a good definition.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/inetaf/netaddr/issues/151#issuecomment-802241421, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQFHWBJV4NUCXMFFK2INOLTEJKLZANCNFSM4ZJTKT3A .

moreati commented 3 years ago

Some adhoc research while I attempt this ticket

net.IP netaddr.IP Notes
Is4() y
Is6() y
Is4in6() y
IsGlobalUnicast() y Go 1.0 (or earlier)
IsInterfaceLocalMulticast() y y
IsLinkLocalMulticast() y y
IsLinkLocalUnicast() y y
IsLoopback() y y
IsMulticast() y y
IsPrivate() (1.17) ETA Aug 2021. https://github.com/golang/go/issues/29146, Code Review
IsUnspecified() y return true iif == 0.0.0.0, or == ::
IsZero() y return true iif == IP{}? Unsure of exact Go semantics
mdlayher commented 3 years ago

We now have IsGlobalUnicast, IsPrivate, and IsUnspecified. In theory callers could just !ip.IsPrivate() at this point to exclude RFC1918 and ULAs.