Skip to content

Commit

Permalink
Auto merge of #60145 - little-dude:ip2, r=alexcrichton
Browse files Browse the repository at this point in the history
std::net: Ipv4Addr and Ipv6Addr improvements

Picking this up again from my previous PR: #56050
Related to: #27709
Fixes: #57558

- add `add Ipv4Addr::is_reserved()`
  - [X] implementation
  - [X] tests
- add `Ipv6Addr::is_unicast_link_local_strict()` and update `Ipv6Addr::is_unicast_link_local()` documentation
  - [X] implementation
  - [X] test
- add `Ipv4Addr::is_benchmarking()`
  - [X] implementation
  - [X] test
- add `Ipv4Addr::is_ietf_protocol_assignment()`
  - [X] implementation
  - [X] test
- add `Ipv4Addr::is_shared()`
  - [X] implementation
  - [x] test
- fix `Ipv4Addr:is_global()`
  - [X] implementation
  - [x] test
- [X] refactor the tests for IP properties. This makes the tests more verbose, but using macros have two advantages:
    - it will now be easier to add tests for all the new methods
    - we get clear error messages when a test is failing. For instance:

```
---- net::ip::tests::ip_properties stdout ----
thread '<unnamed>' panicked at 'assertion failed: !ip!("fec0::").is_global()', src/libstd/net/ip.rs:2036:9

```

Whereas previously it was something like

```
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
   left: `true`,
  right: `false`', libstd/net/ip.rs:1948:13
```

-----------------------

# Ongoing discussions:

## Should `Ipv4Addr::is_global()` return `true` or `false` for reserved addresses?

Reserved addresses are addresses that are matched by `Ipv4Addr::is_reserved()`.
@the8472 [pointed out](#60145 (comment)) that [RFC 4291](https://tools.ietf.org/html/rfc4291#section-2.4) says IPv6 reserved addresses should be considered global:

```
Future specifications may redefine one or more sub-ranges of the
Global Unicast space for other purposes, but unless and until that
happens, implementations must treat all addresses that do not start
with any of the above-listed prefixes as Global Unicast addresses.
```

We could extrapolate that this should also be the case for IPv4. However, it seems that [IANA considers them non global](https://www.iana.org/assignments/ipv4-address-space/ipv4-address-space.xhtml) (see [my comment](#60145 (comment)))

### Final decision

There seems to be a consensus that reserved addresses have a different meaning for IPv4 and IPv6 ([comment1](#60145 (comment)) [comment2](#60145 (comment)), so we can consider that RFC4291 does not apply to IPv4, and that reserved IPv4 addresses are _not_ global.

## Should `Ipv6Addr::is_unicast_site_local()` exist?

@pusateri [noted](#60145 (comment)) that site-local addresses have been deprecated for a while by [RFC 3879](https://tools.ietf.org/html/rfc3879) and new implementations _must not_ support them. However, since this method is stable, removing does not seem possible. This kind of situation is covered by the RFC which stated that existing implementation _may_ continue supporting site-local addresses.

### Final decision

Let's keep this method. It is stable already, and the RFC explicitly states that existing implementation may remain.

---------

Note: I'll be AFK from April 27th to May 11th. Anyone should feel free to pick this up if the PR hasn't been merged by then. Sorry for dragging that for so long already.
  • Loading branch information
bors committed Jun 1, 2019
2 parents 84f729d + cddb838 commit 8b40a18
Showing 1 changed file with 781 additions and 163 deletions.
Loading

0 comments on commit 8b40a18

Please sign in to comment.