Change host to use ip address types provided by std.

[pyfisch]

Removes the custom IPv6Addr type and replaces it with the std one.
Parses IPv4Addrs to the std type using the parser described in the
url.spec.whatwg.org handling all edge cases. Add tests.

fixes #116

This is a breaking change. Version bumped to v0.5.0.

[SimonSapin]

Review status: 0 of 6 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

---

src/host.rs, line 88 [r1] (raw file):
Nit: The input.len() check here is not necessary, starts_with already does it.

---

src/host.rs, line 89 [r1] (raw file):
Nit: rather than keep integers indices around, you could have input = &input[2..] here. (If you also change input: &str to mut input: &str in the function definition line)

---

src/host.rs, line 124 [r1] (raw file):
expect panics if the option is None. Panic should only happen in situations that should not happen unless there is a bug. Are you confident that numbers here is never empty? What about Host::parse("")? (Same comment for the other .expect() call below.)

---

src/host.rs, line 126 [r1] (raw file):
I don’t think this should return https://url.spec.whatwg.org/#syntax-violation says that a syntax violation is not fatal and does not terminate the parser. In this was a compiler, it would be called a warnings. Since we don’t have a way of logging warnings at the moment, please ignore when the spec says syntax violation, or simply add a comment.

---

src/host.rs, line 128 [r1] (raw file):
I don’t know if u32::pow is guaranteed to compile down to a bit shift here. Please use << instead. (Multiplying by 256 is the same as shifting by 8 bits.) Same for the other .pow() below.

---

Comments from the review on Reviewable.io

[pyfisch]

src/host.rs, line 89 [r1] (raw file):
Wow, I didn't knew the borrow checker would be so smart and accept this. :)

---

Comments from the review on Reviewable.io

[pyfisch]

Review status: 0 of 6 files reviewed at latest revision, 3 unresolved discussions.

---

src/host.rs, line 124 [r1] (raw file):
It is a bug in the code if the list is empty. Host::parse("") does not work because empty hosts are rejected by the host parser. The spec assumes that the list will not be empty because it says Let ipv4 be the last item in numbers. there is no handling for the empty list case. I also tried to come up with an example to produce an empty list but failed.

---

src/host.rs, line 126 [r1] (raw file):
https://url.spec.whatwg.org/#concept-ipv4-parser step 9 says "If any but the last item in numbers is greater than 255, return failure." So this is no syntax violation but a failure.

---

src/host.rs, line 128 [r1] (raw file):
Done.

---

Comments from the review on Reviewable.io

[SimonSapin]

Reviewed 4 of 6 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/host.rs, line 126 [r1] (raw file):
Ah, sorry, I was looking as step 8 and did not read carefully enough.

---

src/host.rs, line 123 [r2] (raw file):
Shouldn’t this be a strict >?

---

Comments from the review on Reviewable.io

[pyfisch]

Review status: 5 of 6 files reviewed at latest revision, 1 unresolved discussion.

---

src/host.rs, line 123 [r2] (raw file):
Done.

---

Comments from the review on Reviewable.io

[SimonSapin]

@bors-servo r+

Thanks!

[bors-servo]

📌 Commit ce1efd3 has been approved by SimonSapin

[bors-servo]

⌛ Testing commit ce1efd3 with merge ff8e8c9...

[bors-servo]

☀️ Test successful - travis

[frewsxcv]

Worth mentioning that IpAddr in std is now deprecated

rust-lang/rust#30187

[SimonSapin]

This uses Ipv4Addr and Ipv6Addr (which are stable and not deprecated) separately in a custom enum (with a third Domain variant), not the deprecated IpvAddr enum.

[frewsxcv]

Ah, thought the enum was used in here, carry on