You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
At our workplace we ran into an issue where we were accidentally converting an Ipv6Addr to an Ipv6Network. The conversion seems a bit risky because an address and a network represent different things.
In contrast, From<Ipv4Network> for IpNetwork and From<Ipv6Network> for IpNetwork are reasonable to have.
Would you consider removing these impls and maybe introducing single constructors to represent the single IP use case? That would have made this bug much easier to spot.
Thanks!
The text was updated successfully, but these errors were encountered:
sunshowers
changed the title
Consider removing From<IpXXAddr> for From<IpXXNetwork>
Consider removing From<IpXXAddr> for IpXXNetwork
Apr 30, 2024
Hi 👋 sounds like you want a conversion from Ipv6Addr to Ipv6Network to error out, but that does not happen due to this impl https://github.com/achanda/ipnetwork/blob/master/src/ipv6.rs#L315 so you want to remove those impls. Is that correct? What would the type signature of the single constructor be?
Thanks for the quick response! Yes, I'm suggesting removing those impls. It'll be a breaking change, but I think one that's good from an API misuse perspective.
How about just an Ipv6Network::single(addr: Ipv6Addr) constructor? I'm not saying don't provide this, I'm just saying that it would be nice for that to be more explicit.
Hi there, thanks for maintaining this crate!
At our workplace we ran into an issue where we were accidentally converting an Ipv6Addr to an Ipv6Network. The conversion seems a bit risky because an address and a network represent different things.
In contrast,
From<Ipv4Network> for IpNetwork
andFrom<Ipv6Network> for IpNetwork
are reasonable to have.Would you consider removing these impls and maybe introducing
single
constructors to represent the single IP use case? That would have made this bug much easier to spot.Thanks!
The text was updated successfully, but these errors were encountered: