Skip to content

Consider changing Ipv*Addr serialization to a tuple of integers from a string #181

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.

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

Closed
erickt opened this issue Nov 10, 2015 · 7 comments
Closed
Milestone

Comments

@erickt
Copy link
Member

erickt commented Nov 10, 2015

Serde is missing support for Ipv4Addr and Ipv6Addr. It would be nice if we supported it and other std::net types. One open question though is how should we actually serialize IP addresses? It would be the most compact by serializing to a u32 or (u64, u64), or in pieces with (u8, u8, u8, u8) and 8 u16s, but I'm sure human consumable formats like markdown or JSON would prefer a textual format.

@oli-obk
Copy link
Member

oli-obk commented Nov 11, 2015

[127, 0, 0, 1] is very human readable imo. As long as it's done with serialize_tuple and serialize_u8 bincode will do the optimal thing (4 bytes) and json will do the readable thing. The same goes for ipv6.

The only issue I see is that some might prefer to output a String in the canonical "127.0.0.1" form. For deserialization that's a no-brainer, simply offer a deserialization for Strings. But there's no way to serialize them this way.

@erickt erickt added this to the v0.7.0 milestone Jan 18, 2016
@JohnHeitmann
Copy link
Contributor

I like the octet array version.

Versus text: Once some version of #198 (mapping proxies) lands then serde can add pre-built text mapper proxies that folks can opt in to with annotations.

Versus u32: Octet arrays look better in JSON. Also, arrays have the same endianness universally.

The type I'm really interested in is Timespec, which has the same basic requirements as Ipv*Addr. Serde could by default offer a mapping that very directly models the native shape (i64, i32), and vend #198 mapping proxies with customizable formats for strings.

@erickt
Copy link
Member Author

erickt commented Feb 22, 2016

@JohnHeitmann / @oli-obk: Now that 0.7 has the ability to customize the output, in #242 I decided it'd be easier to just serialize addresses to a string, and leave a more optimized form up to the user.

@oli-obk
Copy link
Member

oli-obk commented Feb 22, 2016

just serialize addresses to a string, and leave a more optimized form up to the user.

We should leave this issue open to make sure we re-evaluate this decision after we see some usage of serde with these types.

@erickt erickt changed the title Add impls for Ipv*Addr and other std::net types Consider changing Ipv*Addr serialization to a tuple of integers from a string Feb 22, 2016
@erickt erickt modified the milestones: V0.8.0, v0.7.0 Feb 22, 2016
@erickt
Copy link
Member Author

erickt commented Feb 22, 2016

@oli-obk: Sure. I've changed the title, and bumped the milestone to v0.8.

@dtolnay
Copy link
Member

dtolnay commented Jun 11, 2016

Any updates here? Have people mostly been happy with the string format? If so, we can close this.

@dtolnay
Copy link
Member

dtolnay commented Jun 15, 2016

If anyone requests this in the future, we could provide a crate of alternative de/serialization representations so that you could do something like:

#[serde(serialize_with="serde_alt::ipv6_to_tuple",
        deserialize_with="serde_alt::ipv6_from_tuple")]
ip: Ipv6Addr

@dtolnay dtolnay closed this as completed Jun 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants