Split InvalidHostPart into separate error enum (fixes #8)

* Separate enum for Cidr parsing * Add inet address to InvalidHostPart variant to allow easier recovery from invalid host part errors in case you want to ignore them.
stbuehler · Dec 3, 2023 · db749ad · db749ad
1 parent 5fdc64e
commit db749ad
Show file tree

Hide file tree

Showing 8 changed files with 299 additions and 52 deletions.
diff --git a/src/cidr/any.rs b/src/cidr/any.rs
@@ -7,7 +7,9 @@ use std::net::IpAddr;
 use super::from_str::cidr_from_str;
 use crate::{
 	errors::*,
+	internal_traits::PrivCidr,
 	Family,
+	GenericCidr,
 	IpCidr,
 	IpInet,
 	Ipv4Cidr,
@@ -69,15 +71,15 @@ impl AnyIpCidr {
 	/// network length exceeds the address length or the address is not
 	/// the first address in the network ("host part not zero") an error
 	/// is returned.
-	pub const fn new(addr: IpAddr, len: u8) -> Result<Self, NetworkParseError> {
+	pub const fn new(addr: IpAddr, len: u8) -> Result<Self, CidrParseError<Self>> {
 		match addr {
 			IpAddr::V4(a) => match Ipv4Cidr::new(a, len) {
 				Ok(cidr) => Ok(Self::V4(cidr)),
-				Err(e) => Err(e),
+				Err(e) => Err(e.const_v4_into_any()),
 			},
 			IpAddr::V6(a) => match Ipv6Cidr::new(a, len) {
 				Ok(cidr) => Ok(Self::V6(cidr)),
-				Err(e) => Err(e),
+				Err(e) => Err(e.const_v6_into_any()),
 			},
 		}
 	}
@@ -220,6 +222,12 @@ impl fmt::Display for AnyIpCidr {
 	}
 }
 
+impl PrivCidr for AnyIpCidr {}
+
+impl GenericCidr for AnyIpCidr {
+	type Address = IpAddr;
+}
+
 impl From<AnyIpCidr> for Option<IpCidr> {
 	fn from(value: AnyIpCidr) -> Option<IpCidr> {
 		match value {
@@ -241,13 +249,15 @@ impl From<Option<IpCidr>> for AnyIpCidr {
 }
 
 impl FromStr for AnyIpCidr {
-	type Err = NetworkParseError;
+	type Err = CidrParseError<AnyIpCidr>;
 
-	fn from_str(s: &str) -> Result<Self, NetworkParseError> {
+	fn from_str(s: &str) -> Result<Self, CidrParseError<AnyIpCidr>> {
 		if s == "any" {
 			Ok(Self::Any)
 		} else {
-			cidr_from_str::<IpCidr>(s).map(Self::from)
+			cidr_from_str::<IpCidr>(s)
+				.map(Self::from)
+				.map_err(|e| e.into())
 		}
 	}
 }

diff --git a/src/cidr/combined.rs b/src/cidr/combined.rs
@@ -10,6 +10,7 @@ use crate::{
 	internal_traits::PrivCidr,
 	Cidr,
 	Family,
+	GenericCidr,
 	InetIterator,
 	IpCidr,
 	IpInet,
@@ -41,15 +42,15 @@ impl IpCidr {
 	/// network length exceeds the address length or the address is not
 	/// the first address in the network ("host part not zero") an
 	/// error is returned.
-	pub const fn new(addr: IpAddr, len: u8) -> Result<Self, NetworkParseError> {
+	pub const fn new(addr: IpAddr, len: u8) -> Result<Self, CidrParseError<Self>> {
 		match addr {
 			IpAddr::V4(a) => match Ipv4Cidr::new(a, len) {
 				Ok(cidr) => Ok(Self::V4(cidr)),
-				Err(e) => Err(e),
+				Err(e) => Err(e.const_v4_into()),
 			},
 			IpAddr::V6(a) => match Ipv6Cidr::new(a, len) {
 				Ok(cidr) => Ok(Self::V6(cidr)),
-				Err(e) => Err(e),
+				Err(e) => Err(e.const_v6_into()),
 			},
 		}
 	}
@@ -162,10 +163,12 @@ impl IpCidr {
 
 impl PrivCidr for IpCidr {}
 
-impl Cidr for IpCidr {
+impl GenericCidr for IpCidr {
 	type Address = IpAddr;
+}
 
-	fn new(addr: IpAddr, len: u8) -> Result<Self, NetworkParseError> {
+impl Cidr for IpCidr {
+	fn new(addr: IpAddr, len: u8) -> Result<Self, CidrParseError<Self>> {
 		Self::new(addr, len)
 	}
 
@@ -224,9 +227,9 @@ impl fmt::Display for IpCidr {
 }
 
 impl FromStr for IpCidr {
-	type Err = NetworkParseError;
+	type Err = CidrParseError<Self>;
 
-	fn from_str(s: &str) -> Result<Self, NetworkParseError> {
+	fn from_str(s: &str) -> Result<Self, CidrParseError<Self>> {
 		cidr_from_str(s)
 	}
 }

diff --git a/src/cidr/direct.rs b/src/cidr/direct.rs
@@ -20,6 +20,7 @@ use crate::{
 	},
 	Cidr,
 	Family,
+	GenericCidr,
 	InetIterator,
 	Ipv4Cidr,
 	Ipv4Inet,
@@ -83,13 +84,17 @@ macro_rules! impl_cidr_for {
 			/// network length exceeds the address length or the address is not
 			/// the first address in the network ("host part not zero") an
 			/// error is returned.
-			pub const fn new(addr: $addr, len: u8) -> Result<Self, NetworkParseError> {
-				if len > $family.len() {
-					Err(NetworkParseError::NetworkLengthTooLongError(
-						NetworkLengthTooLongError::new(len as usize, $family),
-					))
-				} else if !<$addr as PrivUnspecAddress>::_Tools::_has_zero_host_part(addr, len) {
-					Err(NetworkParseError::InvalidHostPart)
+			pub const fn new(addr: $addr, len: u8) -> Result<Self, CidrParseError<Self>> {
+				let address = match $inet::new(addr, len) {
+					Ok(address) => address,
+					Err(e) => {
+						return Err(CidrParseError::NetworkParseError(
+							NetworkParseError::NetworkLengthTooLongError(e),
+						))
+					},
+				};
+				if !<$addr as PrivUnspecAddress>::_Tools::_has_zero_host_part(addr, len) {
+					Err(CidrParseError::InvalidHostPart { address })
 				} else {
 					Ok(Self {
 						address: addr,
@@ -187,10 +192,12 @@ macro_rules! impl_cidr_for {
 
 		impl PrivCidr for $n {}
 
-		impl Cidr for $n {
+		impl GenericCidr for $n {
 			type Address = $addr;
+		}
 
-			fn new(addr: $addr, len: u8) -> Result<Self, NetworkParseError> {
+		impl Cidr for $n {
+			fn new(addr: $addr, len: u8) -> Result<Self, CidrParseError<Self>> {
 				Self::new(addr, len)
 			}
 
@@ -271,9 +278,9 @@ macro_rules! impl_cidr_for {
 		}
 
 		impl FromStr for $n {
-			type Err = NetworkParseError;
+			type Err = CidrParseError<Self>;
 
-			fn from_str(s: &str) -> Result<$n, NetworkParseError> {
+			fn from_str(s: &str) -> Result<$n, CidrParseError<Self>> {
 				cidr_from_str(s)
 			}
 		}

diff --git a/src/cidr/from_str.rs b/src/cidr/from_str.rs
@@ -6,7 +6,7 @@ use crate::{
 	Cidr,
 };
 
-pub fn cidr_from_str<C>(s: &str) -> Result<C, NetworkParseError>
+pub fn cidr_from_str<C>(s: &str) -> Result<C, CidrParseError<C>>
 where
 	C: Cidr,
 	C::Address: ParseableAddress,

diff --git a/src/cidr/tests.rs b/src/cidr/tests.rs
@@ -424,7 +424,9 @@ fn parse_v4_23bit() {
 }
 
 #[test]
-#[should_panic(expected = "host part of address was not zero")]
+#[should_panic(
+	expected = "host part of address 192.0.3.0/23 was not zero; did you mean 192.0.2.0/23?"
+)]
 fn parse_v4_23bit_non_zero_host_bits() {
 	"192.0.3.0/23".parse::<Ipv4Cidr>().unwrap();
 }
@@ -441,7 +443,9 @@ fn parse_v4_17bit() {
 }
 
 #[test]
-#[should_panic(expected = "host part of address was not zero")]
+#[should_panic(
+	expected = "host part of address 192.0.192.0/17 was not zero; did you mean 192.0.128.0/17?"
+)]
 fn parse_v4_17bit_non_zero_host_bits() {
 	"192.0.192.0/17".parse::<Ipv4Cidr>().unwrap();
 }
@@ -491,7 +495,9 @@ fn parse_v4_0bit_short() {
 }
 
 #[test]
-#[should_panic(expected = "host part of address was not zero")]
+#[should_panic(
+	expected = "host part of address 10.1.1.1/24 was not zero; did you mean 10.1.1.0/24?"
+)]
 fn parse_v4_non_zero_host_bits() {
 	"10.1.1.1/24".parse::<Ipv4Cidr>().unwrap();
 }
@@ -641,7 +647,9 @@ fn parse_v6_64bit() {
 }
 
 #[test]
-#[should_panic(expected = "host part of address was not zero")]
+#[should_panic(
+	expected = "host part of address 2001:db8:1234:5678:1::/64 was not zero; did you mean 2001:db8:1234:5678::/64?"
+)]
 fn parse_v6_non_zero_host_bits() {
 	"2001:DB8:1234:5678:1::/64".parse::<Ipv6Cidr>().unwrap();
 }