Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate Ipv4Addr/Ipv6Addr types, new methods for inspecting addresses #20785

Closed
wants to merge 1 commit into from

Conversation

ktossell
Copy link
Contributor

@ktossell ktossell commented Jan 9, 2015

This adds several address inspection methods, such as is_loopback, is_multicast, and
is_private, as well as improving formatting (It adds support for ::-shortening addresses and
fixes :: and ::1) and adding methods to convert between different IP versions.

The variants IpAddr::Ipv4Addr and Ipv6Addr were limiting: There are is_... methods that
make sense for either v4 or v6 addresses but not for both versions, and there's no way to define
methods for only one variant of an enum, so the variants are promoted to independent types.

The IpAddr enum now wraps the Ipv4Addr and Ipv6Addr types:

enum IpAddr { V4(Ipv4Addr), V6(Ipv6Addr) }

Addresses are created as:

Ipv4Addr::new(1, 2, 3, 4) -> Ipv4Addr
IpAddr::new_v4(1, 2, 3, 4) -> IpvAddr::V4(Ipv4Addr)

or using FromStr for any of the three address types.

IP addresses can be passed as their core types (IPvXAddr), as IpAddr, or using the trait
ToIpAddr, which is implemented for Ipv4Addr, Ipv6Addr and IpAddr.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@ktossell ktossell force-pushed the ipaddr-expansion branch 4 times, most recently from 7a3251b to dccb27b Compare January 9, 2015 07:08
@ktossell
Copy link
Contributor Author

ktossell commented Jan 9, 2015

added tests, rebased, fixed typos

Ipv6Addr::is_unicast_global(&self) probably needs a better implementation, and currently Ipv6Addr's property tests don't check whether an underlying IPv4 address has the IPv4 version of the specified property (e.g., ::ffff:127.0.0.1 is not considered a loopback address).

@alexcrichton
Copy link
Member

This looks pretty awesome, thanks @ktossell! There's a few other issues you may be interested in as well:

It may be worth dealing with #15688 while rewriting the IpAddr types. Also, out of curiosity, is this following the design of another library you're familiar with? We'd probably want most of these functions to start out unstable/experimental, but overall the design seems pretty reasonable to me!

@ktossell
Copy link
Contributor Author

I was looking at #15688 and having a hard time trying to come to a conclusion about where the scope identifier should go. I'm leaning toward saying that it should be part of SockAddr, not Ipv6Addr. I'll see if I can make some progress and leave a comment over there.

I'm most familiar with Python's IP address module: https://docs.python.org/3/library/ipaddress.html. I mostly copied its features. Some similar modules are https://golang.org/src/net/ip.go and http://www.rubydoc.info/gems/ipaddress/0.8.0/.

I think it would also be good to have an IpNetwork (and Ipv4Network/Ipv6Network) and to extend IpAddr further, adding masking (x:IpAddr matches y:IpAddr up to the nth bit, z:IpAddr's 24-bit network address is w:IpAddr). I'm not sure IpNetwork would be totally appropriate to add to libstd by itself, since it's probably not directly required for other core features (like socket support requires IpAddr). But I think it's best to include network utilities (is-address-in-network, what-hosts-are-in-network, is-network-subnetwork-of...) when we're providing IpAddr. Without IpNetwork or comparison functions, IpAddr is much less useful. I guess I better bring all of this up in rust-lang/rfcs#576...

@steveklabnik
Copy link
Member

This needs a rebase.

…adding version-specific methods

This adds several address inspection methods, such as `is_loopback`, `is_multicast`, and
`is_private`, as well as improving formatting (It adds support for `::`-shortening addresses and
fixes :: and ::1) and adding methods to convert between different IP versions.

The variants `IpAddr::Ipv4Addr` and `Ipv6Addr` were limiting: There are `is_...` methods that
make sense for either v4 or v6 addresses but not for both versions, and there's no way to define
methods for only one variant of an enum, so the variants are promoted to independent types.

The IpAddr enum now wraps the Ipv4Addr and Ipv6Addr types:

  enum IpAddr { V4(Ipv4Addr), V6(Ipv6Addr) }

Addresses are created as:

  Ipv4Addr::new(1, 2, 3, 4) -> Ipv4Addr
  IpAddr::new_v4(1, 2, 3, 4) -> IpvAddr::V4(Ipv4Addr)

or using FromStr for any of the three address types.

IP addresses can be passed as their core types (IPvXAddr), as IpAddr, or using the trait
ToIpAddr, which is implemented for Ipv4Addr, Ipv6Addr and IpAddr.
@ktossell
Copy link
Contributor Author

Rebased

@tbu-
Copy link
Contributor

tbu- commented Feb 1, 2015

@steveklabnik Can you or someone else review this, please? It has been in the queue for quite a long time.

@aturon
Copy link
Member

aturon commented Feb 2, 2015

Ping @alexcrichton (I realize I was initially assigned, but it looks like you already gave a first pass review).

While this touches on IO reform, this was an area explicitly left unresolved in the RFCs, and I'm happy to land improvements here that we can carry over to the new modules.

@alexcrichton
Copy link
Member

Sorry about being so slow on this @ktossell! At this point I've now opened an RFC for the upcoming std::net. It may be nice to summarize these changes and put them in the Addresses section of the RFC which currently largely leaves this functionality as an open question.

Would you be willing to help out in writing up a bit summarizing the direction this PR takes? If not I can also try drafting up something and send it over to you for proofing!

@alexcrichton
Copy link
Member

Thanks again for this @ktossell! This all seemed basically good as-is, so I've rolled it all into the upcoming std::net module here: #22015.

I'm gonna close this in favor of that now as we'll probably keep old_io as-is to prevent breakage.

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 11, 2015
This commit is an implementation of [RFC 807][rfc] which adds a `std::net`
module for basic neworking based on top of `std::io`. This module serves as a
replacement for the `std::old_io::net` module and networking primitives in
`old_io`.

[rfc]: fillmein

The major focus of this redesign is to cut back on the level of abstraction to
the point that each of the networking types is just a bare socket. To this end
functionality such as timeouts and cloning has been removed (although cloning
can be done through `duplicate`, it may just yield an error).

With this `net` module comes a new implementation of `SocketAddr` and `IpAddr`.
This work is entirely based on rust-lang#20785 and the only changes were to alter the
in-memory representation to match the `libc`-expected variants and to move from
public fields to accessors.
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Feb 11, 2015
This commit is an implementation of [RFC 807][rfc] which adds a `std::net`
module for basic neworking based on top of `std::io`. This module serves as a
replacement for the `std::old_io::net` module and networking primitives in
`old_io`.

[rfc]: fillmein

The major focus of this redesign is to cut back on the level of abstraction to
the point that each of the networking types is just a bare socket. To this end
functionality such as timeouts and cloning has been removed (although cloning
can be done through `duplicate`, it may just yield an error).

With this `net` module comes a new implementation of `SocketAddr` and `IpAddr`.
This work is entirely based on rust-lang#20785 and the only changes were to alter the
in-memory representation to match the `libc`-expected variants and to move from
public fields to accessors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants