Fix Ipv6Addr::to_ipv4

[nanpuyue]

The IPv4 address used in the IPv4-compatible IPv6 address must be a globally-unique IPv4 unicast address.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @KodrAus (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[nanpuyue]

https://tools.ietf.org/html/rfc4291#section-2.5.5.1

[tesuji]

You might be interested in the discussion here: #27709

[KodrAus]

Thanks for the PR @nanpuyue!

Unfortunately because these methods are already stable I think we’ll need to be very careful about changing their behaviour, inconsistencies and all.

Would the correct behaviour look something like this?

ipv6.is_unicast_global().then_some(ipv6.to_ipv4());

If so, maybe we should list that in the docs for this method.

[nanpuyue]

@KodrAus
Here are some reference links:
https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml
https://www.iana.org/assignments/ipv4-address-space/ipv4-address-space.xhtml
https://doc.rust-lang.org/std/net/struct.Ipv4Addr.html#method.is_global

It seems that 192.0.0.9/32 and 192.0.0.10/32 should also be excluded, maybe we should add the Ipv4Addr::is_unicast_global like this:

impl Ipv4Addr {
    pub fn is_unicast_global(&self) -> bool {
        match self.octets() {
            [0, ..] => false,                               // 0.0.0.0/8
            [10, ..] => false,                              // 10.0.0.0/8
            [100, b, ..] if b & 0b1100_0000 == 64 => false, // 100.64.0.0/10
            [127, ..] => false,                             // 127.0.0.1/8
            [172, b, ..] if b & 0b1111_0000 == 16 => false, // 172.16.0.0/12
            [192, 0, 0, _] => false,                        // 192.0.0.0/24
            [192, 0, 2, _] => false,                        // 192.0.2.0/24
            [192, 31, 196, _] => false,                     // 192.31.196.0/24
            [192, 52, 193, _] => false,                     // 192.52.193.0/24
            [192, 88, 99, _] => false,                      // 192.88.99.0/24
            [192, 168, ..] => false,                        // 192.168.0.0/16
            [192, 175, 48, _] => false,                     // 192.175.48.0/24
            [198, b, ..] if b & 0b1111_1110 == 18 => false, // 198.18.0.0/15
            [198, 51, 100, _] => false,                     // 198.51.100.0/24
            [a, ..] if a & 0b1111_0000 == 240 => false,     // 240.0.0.0/4, 255.255.255.255/32
            _ => true,
        }
    }
}

[nanpuyue]

And you might be interested in the discussion here: #75019

[the8472]

It seems that 192.0.0.9/32 and 192.0.0.10/32 should also be excluded, maybe we should add the Ipv4Addr::is_unicast_global like this:

@nanpuyue it might make more sense to have a is_unicast() in addition to the existing is_global() since those are somewhat orthogonal concerns and then just have to_ipv4() check both.

[bors]

☔ The latest upstream changes (presumably #75421) made this pull request unmergeable. Please resolve the merge conflicts.

[KodrAus]

It looks like is_unicast_global is based on Section 2.5.7 of RFC 4291, but is now deprecated.

So if a combination of a new is_unicast and the existing is_global would cover it then that sounds like a good way to go.

We do have a lot of unstable helper methods on Ipv6Addr that are all similarly named, but behave slightly differently. It would be good to go through them at some point and figure out which ones (it might be all of them) are useful and whether their current organization is the most useful way of doing things.

[nanpuyue]

In fact, I did not find the exact definition or scope of the "globally-unique IPv4 unicast address", so it's difficult to fix this method to make it conform to the definition in IETF RFC 4291 section-2.5.5.1.

Considering that the "IPv4-compatible IPv6 address" has been deprecated, maybe this pr should be closed.

[JohnCSimon]

@nanpuyue - can you please address the merge conflicts? thank you.

[KodrAus]

Based on #75046 (comment) I'll go ahead and close this one for now. Since we're already tied to the current behavior of this method, and a dig through the RFCs themselves hasn't given us a clear definition to base decisions from. I think our best bet is to consider additional helper methods that let users codify their assumptions of what it means to convert v4 into v6.

Thanks so much for working on this @nanpuyue!