Tracking issue for IpAddr

[alexcrichton]

This is a tracking issue for the unstable ip_addr feature of the standard library. Some thoughts:

• This isn't actually ever used in the system APIs as there's no notion of "an IP address", just an IPv4 or IPv6 address.
• We have no stable APIs producing this value.
• Some old UDP APIs took this, but they didn't actually want to
• The DNS APIs may want to produce or consume this.

[sfackler]

Nominating for discussion for 1.6.

If it isn't consumed or produced by any libstd APIs, I'd vote for its removal.

[dimbleby]

It's certainly a useful thing to have:

• mio has its own implementation
• as does my c-ares wrapper (for DNS lookups)
• murarth/resolve is another DNS library: it has chosen to use the unstable feature so would presumably be disappointed if this was removed

I expect there are others. Perhaps it would be feasible to search crates.io for: either use of the ip_addr feature, or re-implementations of it?

It would surely be better if there were a single canonical implementation of IpAddr, though I guess it doesn't have to be in std::net - some well-known crate would be pretty much as good.

[murarth]

I think IpAddr should be stabilized. I would note that it's not strictly true that no std::net interface accepts an IpAddr. ToSocketAddrs is implemented by (IpAddr, u16), which is accepted by TcpStream::connect, UdpSocket::bind, and UdpSocket::send_to. I have found this to be useful for a program which performs name resolution ahead of time, often in an asynchronous manner. It is especially useful in the UDP case as one would typically not wish to resolve a hostname each time one sends a message. Therefore, I think that having a value with the notion of "an IP address" is a useful thing, even if it's not consumed by any low-level system APIs.

It is common, in my experience, for a userspace program to accept a hostname, resolve the hostname to either an IPv4 or IPv6 address and pass that address to one of the aforementioned TcpStream or UdpSocket methods. Such a program often does not care whether the address is IPv4 or IPv6 and forcing a program to deal with each of these variants only muddies the intent of the code.

I also think that it doesn't particularly make sense to have DNS facilities in std::net return a SocketAddr. It's entirely possible that one is resolving a hostname for reasons other than to immediately open a connection on one of its ports. Returning an IpAddr from DNS methods makes far more sense, in my opinion. If one wishes to resolve a "hostname:port" string, the ToSocketAddrs trait exists for this reason.

[alexcrichton]

🔔 This issue is now entering its cycle-long final comment period 🔔

The libs team is a little up in the air about whether to stabilize or deprecate this type, so some thoughts would be welcome!

---

On a personal opinion side of things, I'm a little wary to stabilize this as there's no corresponding type in C (as opposed to a sockaddr). This means that this is purely a Rust abstraction, in which case it may not be appropriate for the standard library. If it's widely used, however, then it's pretty harmless and seems fine to me!

[eminence]

I think I agree with @murarth that IpAddr should be stabilized. Personally, the argument that "libstd doesn't produce or consume these, so it should be removed" doesn't hold much sway with me. Since the concept of an "IP Address" is a stable and consistent across platforms, and the fact that there isn't a corresponding type in C are both arguments that suggest a path to stabilization.

Also, I wonder if including IpAddr in stdlib would help with compatibility among third-party modules (they could all deal with IpAddrs instead of building their own enum types)

[mrmonday]

Figured I'd chime in and mention that libpnet uses IpAddr to support Layer 3 and 4 networking, where the port isn't relevant.

[aphistic]

I'd like to mirror @mrmonday 's sentiment about keeping IpAddr around. There are times when you need to represent an ip address without the port.

[daschl]

+1 to what @aphistic and @mrmonday said. In a few projects (in other languages) I needed this type. It is much nicer than just carrying around an opaque String.

[abaumhauer]

+1 on stabilizing this type. Many times we (as network programmers) don't care if it's an IPv4 or v6 address. In Postgres, there's an Inet type that holds either format. This would be helpful for rust-postgres crate, too.

[sfackler]

rust-postgres would need a bit more than just IpAddr since the Postgres INET and CIDR types store a netmask as well.

[abaumhauer]

@sfackler, are you implying that IpvXAddr types should add netmask bits to better represent IP addresses? It's a good point that an IP address is more valuable when you know the netmask.

[sfackler]

Uh, no I am not saying we should add a netmask field to this type.

[aturon]

I feel like this enum should either be beefed up and used elsewhere in std, or deprecated. I don't care much which route we take.

[alexcrichton]

The libs team discussed this during triage today and the conclusion was to deprecate. This type can be added back to the standard library if need be and otherwise without many concrete users in-tree itself this sort of type seems suitable for an external crate or definition locally.

[SimonSapin]

FWIW: I considered using this in rust-url but went with a custom enum instead for hosts since I need another variant anyway, for DNS domain names.

[johnzachary]

Even though this is closed and done, it's a shame you guys removed IpAddr when many developers found it useful. Is there an ABI compatible alternative? We really depend on IpAddr in our code.

[mrmonday]

I reimplemented it myself in libpnet[1] - based on all the references to this issue, so has everyone else!

It's a shame none of these networking libraries are going to be able to work together now, without deconstructing/reconstructing the IpAddr.

[1] https://github.com/libpnet/libpnet/blob/735ee2bcc39d6037feb1b2e8a64be72f0999fc92/src/util.rs#L129-L168

[johnzachary]

Thanks @mrmonday. That was the answer I expected but hoped for something better. And I agree with your second comment. If people use a feature in a code base, why isn't that "pulling it's own weight"?

[sfackler]

@johnzachary To rephrase that, do you believe that the standard library should include every feature that someone at some point has used in a code base?

[dimbleby]

I released my re-implementation as its own tiny crate. Feel free to use.

[aphistic]

@sfackler No, but I do think it's a bit weird that the IpAddr enum was removed but both std::net::Ipv4Addr and std::net::Ipv6Addr were kept. IpAddr was a nice way to represent those in a single binding. Why not keep what (I think) is a logical relationship between those two structs?

[sfackler]

Ipv6Addr and Ipv4Addr were used in SocketAddrV6 and SocketAddrV4 respectively, which were in turn parts of SocketAddr, which is a fundamental component of the TcpStream and UdpSocket APIs. IpAddr was neither produced nor consumed by anything in the standard library.

[murarth]

Well, IpAddr arguably should be returned and consumed by the DNS lookup functions (which themselves are not yet stable), but DNS lookup instead returns SocketAddr values with a port value of 0. It does not accept input of the form "host:port". It unconditionally returns values with a port value of 0.

[sfackler]

If DNS lookup functions are overhauled to return IpAddrs, then we will add back IpAddrs.

[johnzachary]

We implemented our own version of IpAddr (thanks @dimbleby for suggesting you crate).

do you believe that the standard library should include every feature that someone at some point has used in a code base

@sfackler No and this isn't that case. This is a case of many people finding a feature in the standard library useful and voicing their disapproval when it was removed. I understand it was marked unstable, we were warned, &c. But, we are your user community and are providing you with feedback.

[aturon]

The libs team took up this issue again at our meeting this week, and have agreed that deprecation was the wrong call.

There's a clear community consensus to ship this, and the rationale for deprecation essentially boiled down to a concern about minimalism in std. There's no real maintenance burden here, the design is fine, and the community finds it valuable; let's ship it.

I've opened a PR to revert, which actually stabilizes the enum as of 1.7 (currently in beta). That will require a backport.

[johnzachary]

Awesome. Thank you!