Add IPv4 variant to Host and remove custom Ipv6 address type #135
Conversation
|
https://github.com/pyfisch/heapsize/tree/4bdd4e2524e506d9dd23115dff6a9a2249fe6cc4 implements |
|
By the way this solves #116 |
|
@pyfisch I think you should file a PR against heapsize regarding that commit. |
Add Ipv4Addr and Ipv6Addr for use in rust-url Needed for servo/rust-url#135. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/heapsize/16) <!-- Reviewable:end -->
Add Ipv4Addr and Ipv6Addr for use in rust-url Needed for servo/rust-url#135. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/heapsize/16) <!-- Reviewable:end -->
|
Blocked on servo/heapsize#17 getting r+ since crates and the repo weren't in sync. |
|
Please bump the heapsize dep to |
|
done |
|
And squash the commits, please |
The Whatwg URL Standard was changed to describe Ipv4 address parsing. Before this change there was no difference made between domains and Ipv4 addresses. This change also removes the custom Ipv6 type and uses the types provided by std for ip addresses. This is a breaking change. Version bumped to 0.3.0
|
done |
|
@bors-servo r+ |
|
📌 Commit ecc7bb7 has been approved by |
Add IPv4 variant to Host and remove custom Ipv6 address type The Whatwg URL Standard was changed to describe Ipv4 address parsing. Before this change there was no difference made between domains and Ipv4 addresses. This change also removes the custom Ipv6 type and uses the types provided by std for ip addresses. https://url.spec.whatwg.org/#syntax-host-ipv4 Note `heap_size` feature is broken because the HeapSizeOf trait is not yet implemented for the ip addresses. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/135) <!-- Reviewable:end -->
|
☀️ Test successful - travis |
|
Thanks! |
| '\0', '\t', '\n', '\r', ' ', '#', '%', '/', ':', '?', '@', '[', '\\', ']' | ||
| ][..]).is_some() { | ||
| Err(ParseError::InvalidDomainCharacter) | ||
| if let Ok(addr) = input.parse() { |
There was a problem hiding this comment.
@pyfisch @Ms2ger have you verified that Ipv4Addr::parse and Ipv6::parse match https://url.spec.whatwg.org/#host-parsing in every edge case? For example: leading zeros, hex/octal components in v4, …
|
I’ve reverted this change. It looks like there was a misunderstanding in the review, sorry. At least the IPv4 parsing of the standard library differs from https://url.spec.whatwg.org/#concept-ipv4-parser . (Note that the "parsing" and "serialization" sections of the spec are more relevant to implementation than the "syntax" sections, which only give a high level overview as opposed to exact algorithms.) I don’t know if the current parsing or serialization behavior of std for IPv6 happens to match the current spec, but I don’t think we should rely on it staying that way. (I don’t know if it should. Web compatibility with all its weirdness is an overriding concern for browsers, but not as much for a programming language standard library.) @pyfisch, could you please open a new PR? But keep the existing |
|
(Edge cases to look for in IPv4 include: leading zeroes, hexadecimal or octal components, less than 4 components.) |
|
I will create a new pull request which will use custom parse methods as they are described in the urlspec. The canonical formatting is unambiguous so I will try to use the methods provided by std but add many tests for edge cases. |
|
Sounds great, thanks! And yes, serialisation has fewer edge cases than parsing. |
|
Please also add "Fixes #116" in the new PR or commit message. Thanks! |
… to work around rust-lang/crates.io#76 0.3.0 was published and then yanked: #135 (comment)
The Whatwg URL Standard was changed to describe Ipv4 address
parsing. Before this change there was no difference made
between domains and Ipv4 addresses. This change also removes
the custom Ipv6 type and uses the types provided by std for
ip addresses.
https://url.spec.whatwg.org/#syntax-host-ipv4
Note
heap_sizefeature is broken because the HeapSizeOf trait is not yet implemented for the ip addresses.