Ipv4Net FromStr allows host bits to be set in CIDR

[nu11ptr]

This should return an error since a /24 would require a zero 4th octet, but it accepts it. Worse, it doesn't seem to transform it, but keeps all the bits set.

use std::str::FromStr;
use ipnet::Ipv4Net;

fn main() {
    let net = Ipv4Net::from_str("1.1.1.1/24");
    assert!(net.is_ok());
    println!("{}", net.unwrap());
}

NOTE: I just found the trunc function so now I'm starting to think this is per design. For the record, I think this is broken behavior as these are in no way valid subnets. My preference, and I believe correct behavior, would be to reject these outright. However, if this is kept, it would be nice if trunc could at least run by default so Serde, etc. can properly transform the data. Since there is no way to call trunc in the deserialization path that I'm aware of, it has to be a per call site type thing which is error prone.

Thanks for looking at this.

[nu11ptr]

Just hit another downside of not rejecting bad subnets or at least auto running trunc: there will be duplicates when putting them in a set as 1.1.1.0/24 and 1.1.1.1/24 will not be considered equal.

[krisprice]

Thanks @nu11ptr. Yep it was designed that way. Rejecting it would be preferable to silently transforming it. Possibly that's what I should've done, but the structure can show up in all kinds of places that aren't purely IP routes/prefixes, so I wanted to leave it flexible. Wonder if it's worth surveying users to see what they prefer and if changing this behaviour in a future release would affect them? Could leave this to a v3 update

[nu11ptr]

Thanks for the reply and consideration.

I look at this way - your crate is either the only one, or certainly the most popular, to provide this. Thus, anyone who wants to implement network/subnet-like functionality is either using your crate or reinventing. I would expect almost all those who need to do this are going to want the behavior I describe (and I agree rejecting is better). It would be easy to leave a backdoor API if someone really needs the old behavior (fn from_host_bits_str()), but it should be easy to do the right thing (FromStr) and out of the way for the exception case.