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

proposal: Add an IsPublic() function #151

Open
mellowdrifter opened this issue Mar 17, 2021 · 8 comments
Open

proposal: Add an IsPublic() function #151

mellowdrifter opened this issue Mar 17, 2021 · 8 comments

Comments

@mellowdrifter
Copy link

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

danderson commented Mar 18, 2021

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
Copy link
Author

mellowdrifter commented Mar 19, 2021 via email

@moreati
Copy link
Contributor

moreati commented May 22, 2021

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. golang/go#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 added a commit to moreati/netaddr that referenced this issue May 23, 2021
These helped me to cross reference inet.af/netaddr, net.IP, and
github.com/mellowdrifter/bogons for inetaf#151.

Signed-off-by: Alex Willmer <[email protected]>
josharian pushed a commit that referenced this issue May 23, 2021
These helped me to cross reference inet.af/netaddr, net.IP, and
github.com/mellowdrifter/bogons for #151.

Signed-off-by: Alex Willmer <[email protected]>
@mdlayher
Copy link
Member

mdlayher commented Jul 7, 2021

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

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

5 participants