netaddr icon indicating copy to clipboard operation
netaddr copied to clipboard

proposal: Add an IsPublic() function

Open mellowdrifter opened this issue 4 years ago • 8 comments
trafficstars

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

mellowdrifter avatar Mar 17 '21 01:03 mellowdrifter

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 avatar Mar 17 '21 23:03 danderson

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?

danderson avatar Mar 17 '21 23:03 danderson

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.

mdlayher avatar Mar 18 '21 00:03 mdlayher

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.

bradfitz avatar Mar 18 '21 19:03 bradfitz

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.

danderson avatar Mar 18 '21 19:03 danderson

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 .

mellowdrifter avatar Mar 19 '21 00:03 mellowdrifter

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

moreati avatar May 22 '21 11:05 moreati

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

mdlayher avatar Jul 07 '21 22:07 mdlayher