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

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

Closed
clarfonthey opened this issue Jun 11, 2023 · 1 comment
Closed

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

clarfonthey opened this issue Jun 11, 2023 · 1 comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@clarfonthey
Copy link

clarfonthey commented Jun 11, 2023

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):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@clarfonthey clarfonthey added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jun 11, 2023
@joshtriplett
Copy link
Member

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.

@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Jun 13, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 18, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 18, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
jyn514 added a commit to jyn514/rust that referenced this issue Jun 19, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 20, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
bors pushed a commit to rust-lang/miri that referenced this issue Jun 22, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Aug 24, 2023
Alter `Display` for `Ipv6Addr` for IPv4-compatible addresses

ACP: rust-lang/libs-team#239
overvenus added a commit to overvenus/tikv that referenced this issue Jan 3, 2024
Rust std library changed its behaviour of Display `Ipv6Addr`
See rust-lang/libs-team#239

Signed-off-by: Neil Shen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

2 participants