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.

Sign up for GitHub

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

Jump to bottom

Ipv4Addr::is_global() returns true for non global address #57558

Closed
JakubOnderka opened this issue Jan 12, 2019 · 2 comments · Fixed by #60145
Closed

Ipv4Addr::is_global() returns true for non global address #57558

JakubOnderka opened this issue Jan 12, 2019 · 2 comments · Fixed by #60145
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@JakubOnderka
Copy link
Contributor

JakubOnderka commented Jan 12, 2019 •
edited
Loading

This code should be correct, because according to IANA IPv4 Special-Purpose Address Registry document, all address in 0.0.0.0/8 are not global reachable.

#![feature(ip)]
use std::net::Ipv4Addr;

fn main() {
    let ip = Ipv4Addr::new(0, 0, 0, 1);
    assert!(!ip.is_global());
}

But it panic on assertion failed in latest nightly (2019-01-11) (see Rust Playground)
@Centril Centril changed the title Ipv4Addr::is_globa() returns true for non global address Ipv4Addr::is_global() returns true for non global address Jan 13, 2019
@varkor varkor added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 13, 2019
@frewsxcv
Copy link
Member

frewsxcv commented Jan 13, 2019 •
edited
Loading

Relevant thread: #27709

@frewsxcv frewsxcv mentioned this issue Jan 13, 2019
Open
16 tasks
@JakubOnderka
Copy link
Contributor Author

@JakubOnderka please post a proper link to the playground by clicking on "share" in the upper right corner and copying that link it gives you.

Sorry, fixed

@little-dude little-dude mentioned this issue Apr 21, 2019
Merged
13 tasks
bors added a commit that referenced this issue Jun 1, 2019
@bors
Auto merge of #60145 - little-dude:ip2, r=alexcrichton
8b40a18 
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

std::net: Ipv4Addr and Ipv6Addr improvements little-dude/rust
3 participants
@JakubOnderka @frewsxcv @varkor