rust-lang / libs-team

The home of the library team
Apache License 2.0
116 stars 18 forks source link

ACP: Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses #239

Closed clarfonthey closed 1 year ago

clarfonthey commented 1 year ago

Proposal

(The main reason for filing an ACP here is because it does change the behaviour of Display slightly in an observable way, and not for indisputable reasons.)

Problem statement

In IPv6, devices are generally given access to a majority of the latter 64 bits of the address space and can freely select an address that does not conflict with others on the same network. When representing addresses, it's common to provide a "token" that sets the latter 64 bits while leaving the former 64 bits empty, so that the network prefix and the token can be ORed together to create a valid address. In these cases, if the "token" address only occupies the latter 32 bits of the address, the Display implementation for Ipv6Addr will show the address as a (deprecated) IPv4-compatible address in decimal notation rather than an IPv6 address in hexadecimal notation.

Note that ::1 is an exception to this rule and always displayed in hexadecimal notation.

Motivating examples or use cases

While the IPv4-mapped address scheme (::ffff:a.b.c.d) is still valid, the IPv4-compatible address scheme (::a.b.c.d) is deprecated. This makes it much more likely that a user will want to see the hexadecimal notation for the latter case instead of the decimal notation. The standard only suggests using the decimal notation when it's unambiguous, and this implies only the former is unambiguous: https://datatracker.ietf.org/doc/html/rfc5952#section-5

Certain networking configurations(*) accept an IPv6 token, and it's desirable to be able to treat this as a simple Ipv6Addr in Rust code. Additionally, applications may wish to represent addresses using tokens in their own code; see #235 for a potential API that could make this more desirable.

(*) Example from systemd-networkd (see Token option): https://www.man7.org/linux/man-pages/man5/systemd.network.5.html#[DHCPPREFIXDELEGATION]_SECTION_OPTIONS

Solution sketch

In the Display impl for Ipv6Addr, we have this branch:

// ...
} else if let Some(ipv4) = self.to_ipv4() {
    match segments[5] {
        // IPv4 Compatible address
        0 => write!(f, "::{}", ipv4),
        // IPv4 Mapped address
        0xffff => write!(f, "::ffff:{}", ipv4),
        _ => unreachable!(),
    }
} else {
// ...

The proposed solution would change this to:

} else if let Some(ipv4) = self.to_ipv4_mapped() {
    write!(f, "::ffff:{}", ipv4)
} else {

Additionally, the code could be extended so that the alternate (#) flag uses the existing formatting with IPv4-compatible addresses. This would be the first case where the # flag is used for a Display implementation in the standard library (to my knowledge), but it would still be valid. The Debug implementation just directly calls the Display implementation.

Alternatives

The main alternative is for users to write their own formatting, which is nontrivial due to the fact that IPv6 formatting is rather complicated. This seems undesired due to the relative ease of modifying the libstd implementation.

The other alternative is to invert the meaning of the # flag as suggested, keeping existing behaviour by default but allowing the use of # to use the proposed behaviour. This might even be desired due to the fact that pretty-printing a struct containing IPv6 values will automatically apply this flag regardless.

Links and related work

N/A

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution:

joshtriplett commented 1 year ago

We talked about this in a @rust-lang/libs-api meeting. We were in favor of making this change: go ahead and submit a PR, please.