From 69e59e3a99399775cd84cee4d10f96a3d8d88fb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Fri, 30 Dec 2022 14:39:46 +0100 Subject: [PATCH 1/5] Address code review comments - Mimic std names `IpAddr::V4`, `IpAddr::V6` for owned addresses, as well as `IpAddrRef::V4`, `IpAddrRef::V6` for referenced ones. Also mimic the exception name, now named `AddrParseError` instead of `InvalidIpAddressError`. - Update copyright header on `name.rs`. - Reduce the number of allocations on `ipv6_to_uncompressed_string` used to print IPv6 addresses in their uncompressed form. Other minor changes. --- src/lib.rs | 4 +- src/name.rs | 2 +- src/name/ip_address.rs | 242 ++++++++++++++++++----------------------- src/name/name.rs | 37 +++---- src/name/verify.rs | 68 +++++------- 5 files changed, 155 insertions(+), 198 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 17f01716..d44ee305 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,7 +50,7 @@ pub use { end_entity::EndEntityCert, error::Error, name::{ - ip_address::InvalidIpAddressError, ip_address::IpAddressRef, DnsNameOrIpRef, DnsNameRef, + ip_address::AddrParseError, ip_address::IpAddrRef, DnsNameOrIpRef, DnsNameRef, InvalidDnsNameError, InvalidDnsNameOrIpError, }, signed_data::{ @@ -63,7 +63,7 @@ pub use { #[cfg(feature = "alloc")] pub use { - name::{ip_address::IpAddress, DnsName}, + name::{ip_address::IpAddr, DnsName}, signed_data::{ RSA_PKCS1_2048_8192_SHA256, RSA_PKCS1_2048_8192_SHA384, RSA_PKCS1_2048_8192_SHA512, RSA_PKCS1_3072_8192_SHA384, RSA_PSS_2048_8192_SHA256_LEGACY_KEY, diff --git a/src/name.rs b/src/name.rs index c49babd6..5d7fdf6b 100644 --- a/src/name.rs +++ b/src/name.rs @@ -1,4 +1,4 @@ -// Copyright 2015-2020 Brian Smith. +// Copyright 2022 Rafael Fernández López. // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/name/ip_address.rs b/src/name/ip_address.rs index a0e3cd91..1dbce3f6 100644 --- a/src/name/ip_address.rs +++ b/src/name/ip_address.rs @@ -22,42 +22,40 @@ const VALID_IP_BY_CONSTRUCTION: &str = "IP address is a valid string by construc /// Either a IPv4 or IPv6 address, plus its owned string representation #[cfg(feature = "alloc")] #[derive(Clone, Debug, Eq, PartialEq, Hash)] -pub enum IpAddress { - /// An ipv4 address and its owned string representation - IpV4Address(String, [u8; 4]), - /// An ipv6 address and its owned string representation - IpV6Address(String, [u8; 16]), +pub enum IpAddr { + /// An IPv4 address and its owned string representation + V4(String, [u8; 4]), + /// An IPv6 address and its owned string representation + V6(String, [u8; 16]), } #[cfg(feature = "alloc")] -impl AsRef for IpAddress { +impl AsRef for IpAddr { fn as_ref(&self) -> &str { match self { - IpAddress::IpV4Address(ip_address, _) | IpAddress::IpV6Address(ip_address, _) => { - ip_address.as_str() - } + IpAddr::V4(ip_address, _) | IpAddr::V6(ip_address, _) => ip_address.as_str(), } } } /// Either a IPv4 or IPv6 address, plus its borrowed string representation #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum IpAddressRef<'a> { +pub enum IpAddrRef<'a> { /// An IPv4 address and its borrowed string representation - IpV4AddressRef(&'a [u8], [u8; 4]), + V4(&'a [u8], [u8; 4]), /// An IPv6 address and its borrowed string representation - IpV6AddressRef(&'a [u8], [u8; 16]), + V6(&'a [u8], [u8; 16]), } #[cfg(feature = "alloc")] -impl<'a> From> for IpAddress { - fn from(ip_address: IpAddressRef<'a>) -> IpAddress { +impl<'a> From> for IpAddr { + fn from(ip_address: IpAddrRef<'a>) -> IpAddr { match ip_address { - IpAddressRef::IpV4AddressRef(ip_address, ip_address_octets) => IpAddress::IpV4Address( + IpAddrRef::V4(ip_address, ip_address_octets) => IpAddr::V4( String::from_utf8(ip_address.to_vec()).expect(VALID_IP_BY_CONSTRUCTION), ip_address_octets, ), - IpAddressRef::IpV6AddressRef(ip_address, ip_address_octets) => IpAddress::IpV6Address( + IpAddrRef::V6(ip_address, ip_address_octets) => IpAddr::V6( String::from_utf8(ip_address.to_vec()).expect(VALID_IP_BY_CONSTRUCTION), ip_address_octets, ), @@ -69,16 +67,15 @@ impl<'a> From> for IpAddress { // // This function can only be called on IPv4 addresses that have // already been validated with `is_valid_ipv4_address`. -pub(crate) fn ipv4_octets(ip_address: &[u8]) -> Result<[u8; 4], InvalidIpAddressError> { +pub(crate) fn ipv4_octets(ip_address: &[u8]) -> Result<[u8; 4], AddrParseError> { let mut result: [u8; 4] = [0, 0, 0, 0]; for (i, textual_octet) in ip_address .split(|textual_octet| *textual_octet == b'.') .enumerate() { - result[i] = str::parse::( - core::str::from_utf8(textual_octet).map_err(|_| InvalidIpAddressError)?, - ) - .map_err(|_| InvalidIpAddressError)?; + result[i] = + str::parse::(core::str::from_utf8(textual_octet).map_err(|_| AddrParseError)?) + .map_err(|_| AddrParseError)?; } Ok(result) } @@ -87,17 +84,17 @@ pub(crate) fn ipv4_octets(ip_address: &[u8]) -> Result<[u8; 4], InvalidIpAddress // // This function can only be called on uncompressed IPv6 addresses // that have already been validated with `is_valid_ipv6_address`. -pub(crate) fn ipv6_octets(ip_address: &[u8]) -> Result<[u8; 16], InvalidIpAddressError> { +pub(crate) fn ipv6_octets(ip_address: &[u8]) -> Result<[u8; 16], AddrParseError> { let mut result: [u8; 16] = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; for (i, textual_block) in ip_address .split(|textual_block| *textual_block == b':') .enumerate() { let octets = u16::from_str_radix( - core::str::from_utf8(textual_block).map_err(|_| InvalidIpAddressError)?, + core::str::from_utf8(textual_block).map_err(|_| AddrParseError)?, 16, ) - .map_err(|_| InvalidIpAddressError)? + .map_err(|_| AddrParseError)? .to_be_bytes(); result[2 * i] = octets[0]; @@ -107,25 +104,25 @@ pub(crate) fn ipv6_octets(ip_address: &[u8]) -> Result<[u8; 16], InvalidIpAddres } #[cfg(feature = "alloc")] -impl<'a> From<&'a IpAddress> for IpAddressRef<'a> { - fn from(ip_address: &'a IpAddress) -> IpAddressRef<'a> { +impl<'a> From<&'a IpAddr> for IpAddrRef<'a> { + fn from(ip_address: &'a IpAddr) -> IpAddrRef<'a> { match ip_address { - IpAddress::IpV4Address(ip_address, ip_address_octets) => { - IpAddressRef::IpV4AddressRef(ip_address.as_bytes(), *ip_address_octets) + IpAddr::V4(ip_address, ip_address_octets) => { + IpAddrRef::V4(ip_address.as_bytes(), *ip_address_octets) } - IpAddress::IpV6Address(ip_address, ip_address_octets) => { - IpAddressRef::IpV6AddressRef(ip_address.as_bytes(), *ip_address_octets) + IpAddr::V6(ip_address, ip_address_octets) => { + IpAddrRef::V6(ip_address.as_bytes(), *ip_address_octets) } } } } -/// An error indicating that an `IpAddressRef` could not built because the input -/// is not a valid IP address. +/// An error indicating that an `IpAddrRef` could not built because +/// the input could not be parsed as an IP address. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct InvalidIpAddressError; +pub struct AddrParseError; -impl core::fmt::Display for InvalidIpAddressError { +impl core::fmt::Display for AddrParseError { fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { write!(f, "{:?}", self) } @@ -133,44 +130,38 @@ impl core::fmt::Display for InvalidIpAddressError { /// Requires the `std` feature. #[cfg(feature = "std")] -impl ::std::error::Error for InvalidIpAddressError {} +impl ::std::error::Error for AddrParseError {} -impl<'a> IpAddressRef<'a> { - /// Constructs an `IpAddressRef` from the given input if the input is a - /// valid IPv4 or IPv6 address. - pub fn try_from_ascii(ip_address: &'a [u8]) -> Result { +impl<'a> IpAddrRef<'a> { + /// Constructs an `IpAddrRef` from the given input if the input is + /// a valid IPv4 or IPv6 address. + pub fn try_from_ascii(ip_address: &'a [u8]) -> Result { if is_valid_ipv4_address(untrusted::Input::from(ip_address)) { - Ok(IpAddressRef::IpV4AddressRef( - ip_address, - ipv4_octets(ip_address)?, - )) + Ok(IpAddrRef::V4(ip_address, ipv4_octets(ip_address)?)) } else if is_valid_ipv6_address(untrusted::Input::from(ip_address)) { - Ok(IpAddressRef::IpV6AddressRef( - ip_address, - ipv6_octets(ip_address)?, - )) + Ok(IpAddrRef::V6(ip_address, ipv6_octets(ip_address)?)) } else { - Err(InvalidIpAddressError) + Err(AddrParseError) } } - /// Constructs an `IpAddressRef` from the given input if the input is a + /// Constructs an `IpAddrRef` from the given input if the input is a /// valid IP address. - pub fn try_from_ascii_str(ip_address: &'a str) -> Result { + pub fn try_from_ascii_str(ip_address: &'a str) -> Result { Self::try_from_ascii(ip_address.as_bytes()) } - /// Constructs an `IpAddress` from this `IpAddressRef` + /// Constructs an `IpAddr` from this `IpAddrRef` /// /// Requires the `alloc` feature. #[cfg(feature = "alloc")] - pub fn to_owned(&self) -> IpAddress { + pub fn to_owned(&self) -> IpAddr { match self { - IpAddressRef::IpV4AddressRef(ip_address, ip_address_octets) => IpAddress::IpV4Address( + IpAddrRef::V4(ip_address, ip_address_octets) => IpAddr::V4( String::from_utf8(ip_address.to_vec()).expect(VALID_IP_BY_CONSTRUCTION), *ip_address_octets, ), - IpAddressRef::IpV6AddressRef(ip_address, ip_address_octets) => IpAddress::IpV6Address( + IpAddrRef::V6(ip_address, ip_address_octets) => IpAddr::V6( String::from_utf8(ip_address.to_vec()).expect(VALID_IP_BY_CONSTRUCTION), *ip_address_octets, ), @@ -180,21 +171,27 @@ impl<'a> IpAddressRef<'a> { #[cfg(feature = "std")] fn ipv6_to_uncompressed_string(octets: [u8; 16]) -> String { - octets - .chunks(2) - .map(|octet| format!("{:02x?}{:02x?}", octet[0], octet[1])) - .collect::>() - .join(":") + format!( + "{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}", + octets[0], octets[1], + octets[2], octets[3], + octets[4], octets[5], + octets[6], octets[7], + octets[8], octets[9], + octets[10], octets[11], + octets[12], octets[13], + octets[14], octets[15], + ) } #[cfg(feature = "std")] -impl From for IpAddress { - fn from(ip_address: std::net::IpAddr) -> IpAddress { +impl From for IpAddr { + fn from(ip_address: std::net::IpAddr) -> IpAddr { match ip_address { std::net::IpAddr::V4(ip_address) => { - IpAddress::IpV4Address(ip_address.to_string(), ip_address.octets()) + IpAddr::V4(ip_address.to_string(), ip_address.octets()) } - std::net::IpAddr::V6(ip_address) => IpAddress::IpV6Address( + std::net::IpAddr::V6(ip_address) => IpAddr::V6( // We cannot rely on the Display implementation of // std::net::Ipv6Addr given that it might return // compressed IPv6 addresses if the address can be @@ -209,22 +206,20 @@ impl From for IpAddress { } } -impl<'a> From> for &'a str { - fn from(ip_address: IpAddressRef<'a>) -> &'a str { +impl<'a> From> for &'a str { + fn from(ip_address: IpAddrRef<'a>) -> &'a str { match ip_address { - IpAddressRef::IpV4AddressRef(ip_address, _) - | IpAddressRef::IpV6AddressRef(ip_address, _) => { + IpAddrRef::V4(ip_address, _) | IpAddrRef::V6(ip_address, _) => { core::str::from_utf8(ip_address).expect(VALID_IP_BY_CONSTRUCTION) } } } } -impl<'a> From> for &'a [u8] { - fn from(ip_address: IpAddressRef<'a>) -> &'a [u8] { +impl<'a> From> for &'a [u8] { + fn from(ip_address: IpAddrRef<'a>) -> &'a [u8] { match ip_address { - IpAddressRef::IpV4AddressRef(ip_address, _) - | IpAddressRef::IpV6AddressRef(ip_address, _) => ip_address, + IpAddrRef::V4(ip_address, _) | IpAddrRef::V6(ip_address, _) => ip_address, } } } @@ -501,9 +496,9 @@ mod tests { assert_eq!(ipv4_octets(b"0.0.0.0"), Ok([0, 0, 0, 0])); assert_eq!(ipv4_octets(b"54.155.246.232"), Ok([54, 155, 246, 232])); // Invalid UTF-8 encoding - assert_eq!(ipv4_octets(b"0.\xc3\x28.0.0"), Err(InvalidIpAddressError)); + assert_eq!(ipv4_octets(b"0.\xc3\x28.0.0"), Err(AddrParseError)); // Invalid number for a u8 - assert_eq!(ipv4_octets(b"0.0.0.256"), Err(InvalidIpAddressError)); + assert_eq!(ipv4_octets(b"0.0.0.256"), Err(AddrParseError)); } const IPV6_ADDRESSES_VALIDITY: &[(&[u8], bool)] = &[ @@ -584,33 +579,33 @@ mod tests { // Invalid UTF-8 encoding assert_eq!( ipv6_octets(b"\xc3\x28a05:d018:076c:b684:8e48:47c9:84aa:b34d"), - Err(InvalidIpAddressError), + Err(AddrParseError), ); // Invalid hexadecimal assert_eq!( ipv6_octets(b"ga05:d018:076c:b684:8e48:47c9:84aa:b34d"), - Err(InvalidIpAddressError), + Err(AddrParseError), ); } #[test] fn try_from_ascii_ip_address_test() { - const IP_ADDRESSES: &[(&[u8], Result)] = &[ + const IP_ADDRESSES: &[(&[u8], Result)] = &[ // Valid IPv4 addresses ( b"127.0.0.1", - Ok(IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1])), + Ok(IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1])), ), // Invalid IPv4 addresses ( // Ends with a dot; misses one octet b"127.0.0.", - Err(InvalidIpAddressError), + Err(AddrParseError), ), // Valid IPv6 addresses ( b"0000:0000:0000:0000:0000:0000:0000:0001", - Ok(IpAddressRef::IpV6AddressRef( + Ok(IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], )), @@ -619,38 +614,35 @@ mod tests { ( // IPv6 addresses in compressed form are not supported b"0:0:0:0:0:0:0:1", - Err(InvalidIpAddressError), + Err(AddrParseError), ), // Something else ( // A hostname b"example.com", - Err(InvalidIpAddressError), + Err(AddrParseError), ), ]; for &(ip_address, expected_result) in IP_ADDRESSES { - assert_eq!(IpAddressRef::try_from_ascii(ip_address), expected_result) + assert_eq!(IpAddrRef::try_from_ascii(ip_address), expected_result) } } #[test] fn try_from_ascii_str_ip_address_test() { - const IP_ADDRESSES: &[(&str, Result)] = &[ + const IP_ADDRESSES: &[(&str, Result)] = &[ // Valid IPv4 addresses - ( - "127.0.0.1", - Ok(IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1])), - ), + ("127.0.0.1", Ok(IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1]))), // Invalid IPv4 addresses ( // Ends with a dot; misses one octet "127.0.0.", - Err(InvalidIpAddressError), + Err(AddrParseError), ), // Valid IPv6 addresses ( "0000:0000:0000:0000:0000:0000:0000:0001", - Ok(IpAddressRef::IpV6AddressRef( + Ok(IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], )), @@ -659,20 +651,17 @@ mod tests { ( // IPv6 addresses in compressed form are not supported "0:0:0:0:0:0:0:1", - Err(InvalidIpAddressError), + Err(AddrParseError), ), // Something else ( // A hostname "example.com", - Err(InvalidIpAddressError), + Err(AddrParseError), ), ]; for &(ip_address, expected_result) in IP_ADDRESSES { - assert_eq!( - IpAddressRef::try_from_ascii_str(ip_address), - expected_result - ) + assert_eq!(IpAddrRef::try_from_ascii_str(ip_address), expected_result) } } @@ -680,13 +669,10 @@ mod tests { fn str_from_ip_address_ref_test() { let ip_addresses = vec![ // IPv4 addresses - ( - IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1]), - "127.0.0.1", - ), + (IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1]), "127.0.0.1"), // IPv6 addresses ( - IpAddressRef::IpV6AddressRef( + IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ), @@ -702,13 +688,10 @@ mod tests { fn u8_array_from_ip_address_ref_test() { let ip_addresses = vec![ // IPv4 addresses - ( - IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1]), - "127.0.0.1", - ), + (IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1]), "127.0.0.1"), // IPv6 addresses ( - IpAddressRef::IpV6AddressRef( + IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ), @@ -1045,11 +1028,11 @@ mod alloc_tests { #[test] fn as_ref_ip_address_test() { assert_eq!( - IpAddress::IpV4Address(String::from("127.0.0.1"), [127, 0, 0, 1]).as_ref(), + IpAddr::V4(String::from("127.0.0.1"), [127, 0, 0, 1]).as_ref(), "127.0.0.1", ); assert_eq!( - IpAddress::IpV6Address( + IpAddr::V6( String::from("0000:0000:0000:0000:0000:0000:0000:0001"), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] ) @@ -1063,11 +1046,8 @@ mod alloc_tests { { let (ip_address, ip_address_octets) = ("127.0.0.1", [127, 0, 0, 1]); assert_eq!( - IpAddress::from(IpAddressRef::IpV4AddressRef( - ip_address.as_bytes(), - ip_address_octets - )), - IpAddress::IpV4Address(String::from(ip_address), ip_address_octets), + IpAddr::from(IpAddrRef::V4(ip_address.as_bytes(), ip_address_octets)), + IpAddr::V4(String::from(ip_address), ip_address_octets), ) } { @@ -1076,11 +1056,8 @@ mod alloc_tests { [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ); assert_eq!( - IpAddress::from(IpAddressRef::IpV6AddressRef( - ip_address.as_bytes(), - ip_address_octets - )), - IpAddress::IpV6Address(String::from(ip_address), ip_address_octets), + IpAddr::from(IpAddrRef::V6(ip_address.as_bytes(), ip_address_octets)), + IpAddr::V6(String::from(ip_address), ip_address_octets), ) } } @@ -1088,20 +1065,20 @@ mod alloc_tests { #[test] fn from_ip_address_for_ip_address_ref_test() { { - let ip_address = IpAddress::IpV4Address(String::from("127.0.0.1"), [127, 0, 0, 1]); + let ip_address = IpAddr::V4(String::from("127.0.0.1"), [127, 0, 0, 1]); assert_eq!( - IpAddressRef::from(&ip_address), - IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1]), + IpAddrRef::from(&ip_address), + IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1]), ) } { - let ip_address = IpAddress::IpV6Address( + let ip_address = IpAddr::V6( String::from("0000:0000:0000:0000:0000:0000:0000:0001"), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ); assert_eq!( - IpAddressRef::from(&ip_address), - IpAddressRef::IpV6AddressRef( + IpAddrRef::from(&ip_address), + IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1] ), @@ -1111,28 +1088,25 @@ mod alloc_tests { #[test] fn display_invalid_ip_address_error_test() { - assert_eq!( - InvalidIpAddressError.to_string(), - String::from("InvalidIpAddressError"), - ) + assert_eq!(AddrParseError.to_string(), String::from("AddrParseError"),) } #[test] fn ip_address_ref_to_owned_test() { { assert_eq!( - IpAddressRef::IpV4AddressRef(b"127.0.0.1", [127, 0, 0, 1]).to_owned(), - IpAddress::IpV4Address(String::from("127.0.0.1"), [127, 0, 0, 1]), + IpAddrRef::V4(b"127.0.0.1", [127, 0, 0, 1]).to_owned(), + IpAddr::V4(String::from("127.0.0.1"), [127, 0, 0, 1]), ) } { assert_eq!( - IpAddressRef::IpV6AddressRef( + IpAddrRef::V6( b"0000:0000:0000:0000:0000:0000:0000:0001", [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ) .to_owned(), - IpAddress::IpV6Address( + IpAddr::V6( String::from("0000:0000:0000:0000:0000:0000:0000:0001"), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ), @@ -1145,18 +1119,18 @@ mod alloc_tests { let ip_addresses = vec![ ( std::net::IpAddr::V4(std::net::Ipv4Addr::new(127, 0, 0, 1)), - IpAddress::IpV4Address(String::from("127.0.0.1"), [127, 0, 0, 1]), + IpAddr::V4(String::from("127.0.0.1"), [127, 0, 0, 1]), ), ( std::net::IpAddr::V6(std::net::Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1)), - IpAddress::IpV6Address( + IpAddr::V6( String::from("0000:0000:0000:0000:0000:0000:0000:0001"), [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1], ), ), ]; for (ip_address, expected_ip_address) in ip_addresses { - assert_eq!(IpAddress::from(ip_address), expected_ip_address,) + assert_eq!(IpAddr::from(ip_address), expected_ip_address,) } } diff --git a/src/name/name.rs b/src/name/name.rs index 027dba55..acd82620 100644 --- a/src/name/name.rs +++ b/src/name/name.rs @@ -14,7 +14,7 @@ use crate::DnsNameRef; -use super::ip_address::{self, IpAddressRef}; +use super::ip_address::{self, IpAddrRef}; /// A DNS name or IP address, which borrows its text representation. #[derive(Debug, Clone, Copy)] @@ -23,7 +23,7 @@ pub enum DnsNameOrIpRef<'a> { DnsName(DnsNameRef<'a>), /// A valid IP address - IpAddress(IpAddressRef<'a>), + IpAddress(IpAddrRef<'a>), } /// An error indicating that a `DnsNameOrIpRef` could not built because the input @@ -32,26 +32,26 @@ pub enum DnsNameOrIpRef<'a> { pub struct InvalidDnsNameOrIpError; impl<'a> DnsNameOrIpRef<'a> { - /// Attempts to decode an encodingless string as either an ipv4 address, ipv6 address or + /// Attempts to decode an encodingless string as either an IPv4 address, IPv6 address or /// DNS name; in that order. In practice this space is non-overlapping because /// DNS name components are separated by periods but cannot be wholly numeric (so cannot - /// overlap with a valid ipv4 address), and ipv6 addresses are separated by colons but + /// overlap with a valid IPv4 address), and IPv6 addresses are separated by colons but /// cannot contain periods. /// - /// The ipv6 address encoding supported here is extremely simplified; it does not support + /// The IPv6 address encoding supported here is extremely simplified; it does not support /// compression, all leading zeroes must be present in each 16-bit word, etc. Generally /// this is not suitable as a parse for human-provided addresses for this reason. Instead: /// consider parsing these with `std::net::IpAddr` and then using - /// `IpAddress::from`. + /// `IpAddr::from`. pub fn try_from_ascii(dns_name_or_ip: &'a [u8]) -> Result { if ip_address::is_valid_ipv4_address(untrusted::Input::from(dns_name_or_ip)) { - return Ok(DnsNameOrIpRef::IpAddress(IpAddressRef::IpV4AddressRef( + return Ok(DnsNameOrIpRef::IpAddress(IpAddrRef::V4( dns_name_or_ip, ip_address::ipv4_octets(dns_name_or_ip).map_err(|_| InvalidDnsNameOrIpError)?, ))); } if ip_address::is_valid_ipv6_address(untrusted::Input::from(dns_name_or_ip)) { - return Ok(DnsNameOrIpRef::IpAddress(IpAddressRef::IpV6AddressRef( + return Ok(DnsNameOrIpRef::IpAddress(IpAddrRef::V6( dns_name_or_ip, ip_address::ipv6_octets(dns_name_or_ip).map_err(|_| InvalidDnsNameOrIpError)?, ))); @@ -74,20 +74,14 @@ impl<'a> From> for DnsNameOrIpRef<'a> { } } -impl<'a> From> for DnsNameOrIpRef<'a> { - fn from(dns_name: IpAddressRef<'a>) -> DnsNameOrIpRef { +impl<'a> From> for DnsNameOrIpRef<'a> { + fn from(dns_name: IpAddrRef<'a>) -> DnsNameOrIpRef { match dns_name { - IpAddressRef::IpV4AddressRef(ip_address, ip_address_octets) => { - DnsNameOrIpRef::IpAddress(IpAddressRef::IpV4AddressRef( - ip_address, - ip_address_octets, - )) + IpAddrRef::V4(ip_address, ip_address_octets) => { + DnsNameOrIpRef::IpAddress(IpAddrRef::V4(ip_address, ip_address_octets)) } - IpAddressRef::IpV6AddressRef(ip_address, ip_address_octets) => { - DnsNameOrIpRef::IpAddress(IpAddressRef::IpV6AddressRef( - ip_address, - ip_address_octets, - )) + IpAddrRef::V6(ip_address, ip_address_octets) => { + DnsNameOrIpRef::IpAddress(IpAddrRef::V6(ip_address, ip_address_octets)) } } } @@ -99,8 +93,7 @@ impl AsRef<[u8]> for DnsNameOrIpRef<'_> { match self { DnsNameOrIpRef::DnsName(dns_name) => dns_name.0, DnsNameOrIpRef::IpAddress(ip_address) => match ip_address { - IpAddressRef::IpV4AddressRef(ip_address, _) - | IpAddressRef::IpV6AddressRef(ip_address, _) => ip_address, + IpAddrRef::V4(ip_address, _) | IpAddrRef::V6(ip_address, _) => ip_address, }, } } diff --git a/src/name/verify.rs b/src/name/verify.rs index c1cc01a2..937bea00 100644 --- a/src/name/verify.rs +++ b/src/name/verify.rs @@ -14,7 +14,7 @@ use super::{ dns_name::{self, DnsNameRef}, - ip_address::{self, IpAddressRef}, + ip_address::{self, IpAddrRef}, name::DnsNameOrIpRef, }; use crate::{ @@ -49,45 +49,35 @@ pub fn verify_cert_dns_name_or_ip( cert: &crate::EndEntityCert, dns_name_or_ip: DnsNameOrIpRef, ) -> Result<(), Error> { - match dns_name_or_ip { - DnsNameOrIpRef::DnsName(dns_name) => verify_cert_dns_name(cert, dns_name), - DnsNameOrIpRef::IpAddress(ip_address) => { - let ip_address = match ip_address { - IpAddressRef::IpV4AddressRef(_, ref ip_address_octets) => { - untrusted::Input::from(ip_address_octets) - } - IpAddressRef::IpV6AddressRef(_, ref ip_address_octets) => { - untrusted::Input::from(ip_address_octets) - } - }; - iterate_names( - // IP addresses are not compared against the subject field; - // only against Subject Alternative Names. - None, - cert.inner().subject_alt_name, - Err(Error::CertNotValidForName), - &|name| { - #[allow(clippy::single_match)] - match name { - GeneralName::IpAddress(presented_id) => { - match ip_address::presented_id_matches_reference_id( - presented_id, - ip_address, - ) { - Ok(true) => return NameIteration::Stop(Ok(())), - Ok(false) => (), - Err(_) => { - return NameIteration::Stop(Err(Error::BadDER)); - } - } - } - _ => (), - } - NameIteration::KeepGoing - }, - ) + let ip_address = match dns_name_or_ip { + DnsNameOrIpRef::DnsName(dns_name) => return verify_cert_dns_name(cert, dns_name), + DnsNameOrIpRef::IpAddress(IpAddrRef::V4(_, ref ip_address_octets)) => { + untrusted::Input::from(ip_address_octets) } - } + DnsNameOrIpRef::IpAddress(IpAddrRef::V6(_, ref ip_address_octets)) => { + untrusted::Input::from(ip_address_octets) + } + }; + + iterate_names( + // IP addresses are not compared against the subject field; + // only against Subject Alternative Names. + None, + cert.inner().subject_alt_name, + Err(Error::CertNotValidForName), + &|name| { + if let GeneralName::IpAddress(presented_id) = name { + match ip_address::presented_id_matches_reference_id(presented_id, ip_address) { + Ok(true) => return NameIteration::Stop(Ok(())), + Ok(false) => (), + Err(_) => { + return NameIteration::Stop(Err(Error::BadDER)); + } + } + } + NameIteration::KeepGoing + }, + ) } // https://tools.ietf.org/html/rfc5280#section-4.2.1.10 From 961cfd785cc10c96fa64d91b2b88ab5da9a9e19f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Fri, 30 Dec 2022 17:02:16 +0100 Subject: [PATCH 2/5] Address code review comments - Rename `DnsNameOrIpRef` to `SubjectNameRef` - Rename `InvalidDnsNameOrIpError` to `InvalidSubjectNameError` --- src/end_entity.rs | 12 ++++----- src/lib.rs | 4 +-- src/name.rs | 4 +-- src/name/name.rs | 61 ++++++++++++++++++++++---------------------- src/name/verify.rs | 14 +++++----- tests/integration.rs | 8 +++--- 6 files changed, 52 insertions(+), 51 deletions(-) diff --git a/src/end_entity.rs b/src/end_entity.rs index 7c879016..3674c901 100644 --- a/src/end_entity.rs +++ b/src/end_entity.rs @@ -13,7 +13,7 @@ // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. use crate::{ - cert, name, signed_data, verify_cert, DnsNameOrIpRef, DnsNameRef, Error, SignatureAlgorithm, + cert, name, signed_data, verify_cert, DnsNameRef, Error, SignatureAlgorithm, SubjectNameRef, TLSClientTrustAnchors, TLSServerTrustAnchors, Time, }; use core::convert::TryFrom; @@ -27,7 +27,7 @@ use core::convert::TryFrom; /// certificate is currently valid *for use by a TLS server*. /// * `EndEntityCert.verify_is_valid_for_dns_name`: Verify that the server's /// certificate is valid for the host that is being connected to. -/// * `EndEntityCert.verify_is_valid_for_dns_name_or_ip`: Verify that the server's +/// * `EndEntityCert.verify_is_valid_for_subject_name`: Verify that the server's /// certificate is valid for the host or IP address that is being connected to. /// /// * `EndEntityCert.verify_signature`: Verify that the signature of server's @@ -151,12 +151,12 @@ impl<'a> EndEntityCert<'a> { name::verify_cert_dns_name(self, dns_name) } - /// Verifies that the certificate is valid for the given DNS host name or IP address. - pub fn verify_is_valid_for_dns_name_or_ip( + /// Verifies that the certificate is valid for the given Subject Name. + pub fn verify_is_valid_for_subject_name( &self, - dns_name_or_ip: DnsNameOrIpRef, + subject_name: SubjectNameRef, ) -> Result<(), Error> { - name::verify_cert_dns_name_or_ip(self, dns_name_or_ip) + name::verify_cert_subject_name(self, subject_name) } /// Verifies the signature `signature` of message `msg` using the diff --git a/src/lib.rs b/src/lib.rs index d44ee305..b9b4b363 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,8 +50,8 @@ pub use { end_entity::EndEntityCert, error::Error, name::{ - ip_address::AddrParseError, ip_address::IpAddrRef, DnsNameOrIpRef, DnsNameRef, - InvalidDnsNameError, InvalidDnsNameOrIpError, + ip_address::AddrParseError, ip_address::IpAddrRef, DnsNameRef, InvalidDnsNameError, + InvalidSubjectNameError, SubjectNameRef, }, signed_data::{ SignatureAlgorithm, ECDSA_P256_SHA256, ECDSA_P256_SHA384, ECDSA_P384_SHA256, diff --git a/src/name.rs b/src/name.rs index 5d7fdf6b..61c78651 100644 --- a/src/name.rs +++ b/src/name.rs @@ -21,9 +21,9 @@ pub use dns_name::DnsName; #[allow(clippy::module_inception)] mod name; -pub use name::{DnsNameOrIpRef, InvalidDnsNameOrIpError}; +pub use name::{InvalidSubjectNameError, SubjectNameRef}; pub mod ip_address; mod verify; -pub(super) use verify::{check_name_constraints, verify_cert_dns_name, verify_cert_dns_name_or_ip}; +pub(super) use verify::{check_name_constraints, verify_cert_dns_name, verify_cert_subject_name}; diff --git a/src/name/name.rs b/src/name/name.rs index acd82620..bb936e6a 100644 --- a/src/name/name.rs +++ b/src/name/name.rs @@ -18,7 +18,7 @@ use super::ip_address::{self, IpAddrRef}; /// A DNS name or IP address, which borrows its text representation. #[derive(Debug, Clone, Copy)] -pub enum DnsNameOrIpRef<'a> { +pub enum SubjectNameRef<'a> { /// A valid DNS name DnsName(DnsNameRef<'a>), @@ -26,12 +26,13 @@ pub enum DnsNameOrIpRef<'a> { IpAddress(IpAddrRef<'a>), } -/// An error indicating that a `DnsNameOrIpRef` could not built because the input -/// is not a syntactically-valid DNS Name or IP address. +/// An error indicating that a `SubjectNameRef` could not built +/// because the input is not a syntactically-valid DNS Name or IP +/// address. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct InvalidDnsNameOrIpError; +pub struct InvalidSubjectNameError; -impl<'a> DnsNameOrIpRef<'a> { +impl<'a> SubjectNameRef<'a> { /// Attempts to decode an encodingless string as either an IPv4 address, IPv6 address or /// DNS name; in that order. In practice this space is non-overlapping because /// DNS name components are separated by periods but cannot be wholly numeric (so cannot @@ -43,56 +44,56 @@ impl<'a> DnsNameOrIpRef<'a> { /// this is not suitable as a parse for human-provided addresses for this reason. Instead: /// consider parsing these with `std::net::IpAddr` and then using /// `IpAddr::from`. - pub fn try_from_ascii(dns_name_or_ip: &'a [u8]) -> Result { - if ip_address::is_valid_ipv4_address(untrusted::Input::from(dns_name_or_ip)) { - return Ok(DnsNameOrIpRef::IpAddress(IpAddrRef::V4( - dns_name_or_ip, - ip_address::ipv4_octets(dns_name_or_ip).map_err(|_| InvalidDnsNameOrIpError)?, + pub fn try_from_ascii(subject_name: &'a [u8]) -> Result { + if ip_address::is_valid_ipv4_address(untrusted::Input::from(subject_name)) { + return Ok(SubjectNameRef::IpAddress(IpAddrRef::V4( + subject_name, + ip_address::ipv4_octets(subject_name).map_err(|_| InvalidSubjectNameError)?, ))); } - if ip_address::is_valid_ipv6_address(untrusted::Input::from(dns_name_or_ip)) { - return Ok(DnsNameOrIpRef::IpAddress(IpAddrRef::V6( - dns_name_or_ip, - ip_address::ipv6_octets(dns_name_or_ip).map_err(|_| InvalidDnsNameOrIpError)?, + if ip_address::is_valid_ipv6_address(untrusted::Input::from(subject_name)) { + return Ok(SubjectNameRef::IpAddress(IpAddrRef::V6( + subject_name, + ip_address::ipv6_octets(subject_name).map_err(|_| InvalidSubjectNameError)?, ))); } - Ok(DnsNameOrIpRef::DnsName( - DnsNameRef::try_from_ascii(dns_name_or_ip).map_err(|_| InvalidDnsNameOrIpError)?, + Ok(SubjectNameRef::DnsName( + DnsNameRef::try_from_ascii(subject_name).map_err(|_| InvalidSubjectNameError)?, )) } - /// Constructs a `DnsNameOrIpRef` from the given input if the input is a - /// syntactically-valid DNS name or IP address. - pub fn try_from_ascii_str(dns_name_or_ip: &'a str) -> Result { - Self::try_from_ascii(dns_name_or_ip.as_bytes()) + /// Constructs a `SubjectNameRef` from the given input if the + /// input is a syntactically-valid DNS name or IP address. + pub fn try_from_ascii_str(subject_name: &'a str) -> Result { + Self::try_from_ascii(subject_name.as_bytes()) } } -impl<'a> From> for DnsNameOrIpRef<'a> { - fn from(dns_name: DnsNameRef<'a>) -> DnsNameOrIpRef { - DnsNameOrIpRef::DnsName(DnsNameRef(dns_name.0)) +impl<'a> From> for SubjectNameRef<'a> { + fn from(dns_name: DnsNameRef<'a>) -> SubjectNameRef { + SubjectNameRef::DnsName(DnsNameRef(dns_name.0)) } } -impl<'a> From> for DnsNameOrIpRef<'a> { - fn from(dns_name: IpAddrRef<'a>) -> DnsNameOrIpRef { +impl<'a> From> for SubjectNameRef<'a> { + fn from(dns_name: IpAddrRef<'a>) -> SubjectNameRef { match dns_name { IpAddrRef::V4(ip_address, ip_address_octets) => { - DnsNameOrIpRef::IpAddress(IpAddrRef::V4(ip_address, ip_address_octets)) + SubjectNameRef::IpAddress(IpAddrRef::V4(ip_address, ip_address_octets)) } IpAddrRef::V6(ip_address, ip_address_octets) => { - DnsNameOrIpRef::IpAddress(IpAddrRef::V6(ip_address, ip_address_octets)) + SubjectNameRef::IpAddress(IpAddrRef::V6(ip_address, ip_address_octets)) } } } } -impl AsRef<[u8]> for DnsNameOrIpRef<'_> { +impl AsRef<[u8]> for SubjectNameRef<'_> { #[inline] fn as_ref(&self) -> &[u8] { match self { - DnsNameOrIpRef::DnsName(dns_name) => dns_name.0, - DnsNameOrIpRef::IpAddress(ip_address) => match ip_address { + SubjectNameRef::DnsName(dns_name) => dns_name.0, + SubjectNameRef::IpAddress(ip_address) => match ip_address { IpAddrRef::V4(ip_address, _) | IpAddrRef::V6(ip_address, _) => ip_address, }, } diff --git a/src/name/verify.rs b/src/name/verify.rs index 937bea00..dbbb38b8 100644 --- a/src/name/verify.rs +++ b/src/name/verify.rs @@ -15,7 +15,7 @@ use super::{ dns_name::{self, DnsNameRef}, ip_address::{self, IpAddrRef}, - name::DnsNameOrIpRef, + name::SubjectNameRef, }; use crate::{ cert::{Cert, EndEntityOrCa}, @@ -45,16 +45,16 @@ pub fn verify_cert_dns_name( ) } -pub fn verify_cert_dns_name_or_ip( +pub fn verify_cert_subject_name( cert: &crate::EndEntityCert, - dns_name_or_ip: DnsNameOrIpRef, + subject_name: SubjectNameRef, ) -> Result<(), Error> { - let ip_address = match dns_name_or_ip { - DnsNameOrIpRef::DnsName(dns_name) => return verify_cert_dns_name(cert, dns_name), - DnsNameOrIpRef::IpAddress(IpAddrRef::V4(_, ref ip_address_octets)) => { + let ip_address = match subject_name { + SubjectNameRef::DnsName(dns_name) => return verify_cert_dns_name(cert, dns_name), + SubjectNameRef::IpAddress(IpAddrRef::V4(_, ref ip_address_octets)) => { untrusted::Input::from(ip_address_octets) } - DnsNameOrIpRef::IpAddress(IpAddrRef::V6(_, ref ip_address_octets)) => { + SubjectNameRef::IpAddress(IpAddrRef::V6(_, ref ip_address_octets)) => { untrusted::Input::from(ip_address_octets) } }; diff --git a/tests/integration.rs b/tests/integration.rs index 7c47c42a..0ae7bea8 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -76,19 +76,19 @@ pub fn cloudflare_dns() { let check_name = |name: &str| { let dns_name_ref = webpki::DnsNameRef::try_from_ascii_str(name).unwrap(); assert_eq!(Ok(()), cert.verify_is_valid_for_dns_name(dns_name_ref)); - let dns_name_or_ip_ref = webpki::DnsNameOrIpRef::from(dns_name_ref); + let subject_name_ref = webpki::SubjectNameRef::from(dns_name_ref); assert_eq!( Ok(()), - cert.verify_is_valid_for_dns_name_or_ip(dns_name_or_ip_ref) + cert.verify_is_valid_for_subject_name(subject_name_ref) ); println!("{:?} ok as name", name); }; let check_addr = |addr: &str| { - let dns_name_or_ip_ref = webpki::DnsNameOrIpRef::try_from_ascii(addr.as_bytes()).unwrap(); + let subject_name_ref = webpki::SubjectNameRef::try_from_ascii(addr.as_bytes()).unwrap(); assert_eq!( Ok(()), - cert.verify_is_valid_for_dns_name_or_ip(dns_name_or_ip_ref) + cert.verify_is_valid_for_subject_name(subject_name_ref) ); println!("{:?} ok as address", addr); }; From 416976fc8ba67324491c9be789c17e48f6fb6405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Fri, 30 Dec 2022 18:09:15 +0100 Subject: [PATCH 3/5] Address code review comments Parse, don't validate --- src/name/ip_address.rs | 480 +++++++++++++++++++++++++---------------- src/name/name.rs | 22 +- 2 files changed, 306 insertions(+), 196 deletions(-) diff --git a/src/name/ip_address.rs b/src/name/ip_address.rs index 1dbce3f6..a7c475ed 100644 --- a/src/name/ip_address.rs +++ b/src/name/ip_address.rs @@ -63,46 +63,6 @@ impl<'a> From> for IpAddr { } } -// Returns the octets that correspond to the provided IPv4 address. -// -// This function can only be called on IPv4 addresses that have -// already been validated with `is_valid_ipv4_address`. -pub(crate) fn ipv4_octets(ip_address: &[u8]) -> Result<[u8; 4], AddrParseError> { - let mut result: [u8; 4] = [0, 0, 0, 0]; - for (i, textual_octet) in ip_address - .split(|textual_octet| *textual_octet == b'.') - .enumerate() - { - result[i] = - str::parse::(core::str::from_utf8(textual_octet).map_err(|_| AddrParseError)?) - .map_err(|_| AddrParseError)?; - } - Ok(result) -} - -// Returns the octets that correspond to the provided IPv6 address. -// -// This function can only be called on uncompressed IPv6 addresses -// that have already been validated with `is_valid_ipv6_address`. -pub(crate) fn ipv6_octets(ip_address: &[u8]) -> Result<[u8; 16], AddrParseError> { - let mut result: [u8; 16] = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; - for (i, textual_block) in ip_address - .split(|textual_block| *textual_block == b':') - .enumerate() - { - let octets = u16::from_str_radix( - core::str::from_utf8(textual_block).map_err(|_| AddrParseError)?, - 16, - ) - .map_err(|_| AddrParseError)? - .to_be_bytes(); - - result[2 * i] = octets[0]; - result[(2 * i) + 1] = octets[1]; - } - Ok(result) -} - #[cfg(feature = "alloc")] impl<'a> From<&'a IpAddr> for IpAddrRef<'a> { fn from(ip_address: &'a IpAddr) -> IpAddrRef<'a> { @@ -136,10 +96,10 @@ impl<'a> IpAddrRef<'a> { /// Constructs an `IpAddrRef` from the given input if the input is /// a valid IPv4 or IPv6 address. pub fn try_from_ascii(ip_address: &'a [u8]) -> Result { - if is_valid_ipv4_address(untrusted::Input::from(ip_address)) { - Ok(IpAddrRef::V4(ip_address, ipv4_octets(ip_address)?)) - } else if is_valid_ipv6_address(untrusted::Input::from(ip_address)) { - Ok(IpAddrRef::V6(ip_address, ipv6_octets(ip_address)?)) + if let Ok(ip_address) = parse_ipv4_address(ip_address) { + Ok(ip_address) + } else if let Ok(ip_address) = parse_ipv6_address(ip_address) { + Ok(ip_address) } else { Err(AddrParseError) } @@ -306,13 +266,16 @@ pub(super) fn presented_id_matches_constraint( Ok(true) } -pub(crate) fn is_valid_ipv4_address(ip_address: untrusted::Input) -> bool { - let mut ip_address = untrusted::Reader::new(ip_address); +pub(crate) fn parse_ipv4_address(ip_address_: &[u8]) -> Result { + let mut ip_address = untrusted::Reader::new(untrusted::Input::from(ip_address_)); let mut is_first_byte = true; - let mut current: [u8; 3] = [0, 0, 0]; + let mut current_octet: [u8; 3] = [0, 0, 0]; let mut current_size = 0; let mut dot_count = 0; + let mut octet = 0; + let mut octets: [u8; 4] = [0, 0, 0, 0]; + // Returns a u32 so it's possible to identify (and error) when // provided textual octets > 255, not representable by u8. fn radix10_to_octet(textual_octets: &[u8]) -> u32 { @@ -329,27 +292,29 @@ pub(crate) fn is_valid_ipv4_address(ip_address: untrusted::Input) -> bool { Ok(b'.') => { if is_first_byte { // IPv4 address cannot start with a dot. - return false; + return Err(AddrParseError); } if ip_address.at_end() { // IPv4 address cannot end with a dot. - return false; + return Err(AddrParseError); } if dot_count == 3 { // IPv4 address cannot have more than three dots. - return false; + return Err(AddrParseError); } dot_count += 1; if current_size == 0 { // IPv4 address cannot contain two dots in a row. - return false; + return Err(AddrParseError); } - if radix10_to_octet(¤t[..current_size]) > 255 { + if radix10_to_octet(¤t_octet[..current_size]) > 255 { // No octet can be greater than 255. - return false; + return Err(AddrParseError); } + octets[octet] = radix10_to_octet(¤t_octet[..current_size]) as u8; + octet += 1; // We move on to the next textual octet. - current = [0, 0, 0]; + current_octet = [0, 0, 0]; current_size = 0; } Ok(number @ b'0'..=b'9') => { @@ -359,80 +324,108 @@ pub(crate) fn is_valid_ipv4_address(ip_address: untrusted::Input) -> bool { && !ip_address.at_end() { // No octet can start with 0 if a dot does not follow and if we are not at the end. - return false; + return Err(AddrParseError); } - if current_size >= current.len() { + if current_size >= current_octet.len() { // More than 3 octets in a triple - return false; + return Err(AddrParseError); } - current[current_size] = number - b'0'; + current_octet[current_size] = number - b'0'; current_size += 1; } _ => { - return false; + return Err(AddrParseError); } } is_first_byte = false; if ip_address.at_end() { - if current_size > 0 && radix10_to_octet(¤t[..current_size]) > 255 { + let last_octet = radix10_to_octet(¤t_octet[..current_size]); + if current_size > 0 && last_octet > 255 { // No octet can be greater than 255. - return false; + return Err(AddrParseError); } + octets[octet] = last_octet as u8; break; } } - dot_count == 3 + if dot_count != 3 { + return Err(AddrParseError); + } + Ok(IpAddrRef::V4(ip_address_, octets)) } -pub(crate) fn is_valid_ipv6_address(ip_address: untrusted::Input) -> bool { +pub(crate) fn parse_ipv6_address(ip_address_: &[u8]) -> Result { // Compressed addresses are not supported. Also, IPv4-mapped IPv6 // addresses are not supported. This makes 8 groups of 4 // hexadecimal characters + 7 colons. - if ip_address.len() != 39 { - return false; + if ip_address_.len() != 39 { + return Err(AddrParseError); } - let mut ip_address = untrusted::Reader::new(ip_address); + let mut ip_address = untrusted::Reader::new(untrusted::Input::from(ip_address_)); let mut is_first_byte = true; let mut current_textual_block_size = 0; let mut colon_count = 0; + + let mut octet = 0; + let mut previous_character = None; + let mut octets: [u8; 16] = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; + loop { match ip_address.read_byte() { Ok(b':') => { if is_first_byte { // Uncompressed IPv6 address cannot start with a colon. - return false; + return Err(AddrParseError); } if ip_address.at_end() { // Uncompressed IPv6 address cannot end with a colon. - return false; + return Err(AddrParseError); } if colon_count == 7 { // IPv6 address cannot have more than seven colons. - return false; + return Err(AddrParseError); } colon_count += 1; if current_textual_block_size == 0 { // Uncompressed IPv6 address cannot contain two colons in a row. - return false; + return Err(AddrParseError); } if current_textual_block_size != 4 { // Compressed IPv6 addresses are not supported. - return false; + return Err(AddrParseError); } // We move on to the next textual block. current_textual_block_size = 0; + previous_character = None; } - Ok(b'0'..=b'9') | Ok(b'a'..=b'f') | Ok(b'A'..=b'F') => { + Ok(character @ b'0'..=b'9') + | Ok(character @ b'a'..=b'f') + | Ok(character @ b'A'..=b'F') => { if current_textual_block_size == 4 { // Blocks cannot contain more than 4 hexadecimal characters. - return false; + return Err(AddrParseError); + } + if let Some(previous_character_) = previous_character { + octets[octet] = (((previous_character_ as char) + .to_digit(16) + // Safe to unwrap because we know character is within hexadecimal bounds ([0-9a-f]) + .unwrap() as u8) + << 4) + | ((character as char) + .to_digit(16) + // Safe to unwrap because we know character is within hexadecimal bounds ([0-9a-f]) + .unwrap() as u8); + previous_character = None; + octet += 1; + } else { + previous_character = Some(character); } current_textual_block_size += 1; } _ => { - return false; + return Err(AddrParseError); } } is_first_byte = false; @@ -441,153 +434,276 @@ pub(crate) fn is_valid_ipv6_address(ip_address: untrusted::Input) -> bool { break; } } - colon_count == 7 + if colon_count != 7 { + return Err(AddrParseError); + } + Ok(IpAddrRef::V6(ip_address_, octets)) } #[cfg(test)] mod tests { use super::*; - const IPV4_ADDRESSES_VALIDITY: &[(&[u8], bool)] = &[ + const fn ipv4_address( + ip_address: &[u8], + octets: [u8; 4], + ) -> (&[u8], Result) { + (ip_address, Ok(IpAddrRef::V4(ip_address, octets))) + } + + const IPV4_ADDRESSES: &[(&[u8], Result)] = &[ // Valid IPv4 addresses - (b"0.0.0.0", true), - (b"127.0.0.1", true), - (b"1.1.1.1", true), - (b"255.255.255.255", true), - (b"205.0.0.0", true), - (b"0.205.0.0", true), - (b"0.0.205.0", true), - (b"0.0.0.205", true), - (b"0.0.0.20", true), + ipv4_address(b"0.0.0.0", [0, 0, 0, 0]), + ipv4_address(b"1.1.1.1", [1, 1, 1, 1]), + ipv4_address(b"205.0.0.0", [205, 0, 0, 0]), + ipv4_address(b"0.205.0.0", [0, 205, 0, 0]), + ipv4_address(b"0.0.205.0", [0, 0, 205, 0]), + ipv4_address(b"0.0.0.205", [0, 0, 0, 205]), + ipv4_address(b"0.0.0.20", [0, 0, 0, 20]), // Invalid IPv4 addresses - (b"", false), - (b"...", false), - (b".0.0.0.0", false), - (b"0.0.0.0.", false), - (b"256.0.0.0", false), - (b"0.256.0.0", false), - (b"0.0.256.0", false), - (b"0.0.0.256", false), - (b"1..1.1.1", false), - (b"1.1..1.1", false), - (b"1.1.1..1", false), - (b"025.0.0.0", false), - (b"0.025.0.0", false), - (b"0.0.025.0", false), - (b"0.0.0.025", false), - (b"1234.0.0.0", false), - (b"0.1234.0.0", false), - (b"0.0.1234.0", false), - (b"0.0.0.1234", false), + (b"", Err(AddrParseError)), + (b"...", Err(AddrParseError)), + (b".0.0.0.0", Err(AddrParseError)), + (b"0.0.0.0.", Err(AddrParseError)), + (b"256.0.0.0", Err(AddrParseError)), + (b"0.256.0.0", Err(AddrParseError)), + (b"0.0.256.0", Err(AddrParseError)), + (b"0.0.0.256", Err(AddrParseError)), + (b"1..1.1.1", Err(AddrParseError)), + (b"1.1..1.1", Err(AddrParseError)), + (b"1.1.1..1", Err(AddrParseError)), + (b"025.0.0.0", Err(AddrParseError)), + (b"0.025.0.0", Err(AddrParseError)), + (b"0.0.025.0", Err(AddrParseError)), + (b"0.0.0.025", Err(AddrParseError)), + (b"1234.0.0.0", Err(AddrParseError)), + (b"0.1234.0.0", Err(AddrParseError)), + (b"0.0.1234.0", Err(AddrParseError)), + (b"0.0.0.1234", Err(AddrParseError)), ]; #[test] - fn is_valid_ipv4_address_test() { - for &(ip_address, expected_result) in IPV4_ADDRESSES_VALIDITY { - assert_eq!( - is_valid_ipv4_address(untrusted::Input::from(ip_address)), - expected_result - ); + fn parse_ipv4_address_test() { + for &(ip_address, expected_result) in IPV4_ADDRESSES { + assert_eq!(parse_ipv4_address(ip_address), expected_result,); } } - #[test] - fn ipv4_octets_test() { - assert_eq!(ipv4_octets(b"0.0.0.0"), Ok([0, 0, 0, 0])); - assert_eq!(ipv4_octets(b"54.155.246.232"), Ok([54, 155, 246, 232])); - // Invalid UTF-8 encoding - assert_eq!(ipv4_octets(b"0.\xc3\x28.0.0"), Err(AddrParseError)); - // Invalid number for a u8 - assert_eq!(ipv4_octets(b"0.0.0.256"), Err(AddrParseError)); + const fn ipv6_address( + ip_address: &[u8], + octets: [u8; 16], + ) -> (&[u8], Result) { + (ip_address, Ok(IpAddrRef::V6(ip_address, octets))) } - const IPV6_ADDRESSES_VALIDITY: &[(&[u8], bool)] = &[ + const IPV6_ADDRESSES: &[(&[u8], Result)] = &[ // Valid IPv6 addresses - (b"2a05:d018:076c:b685:e8ab:afd3:af51:3aed", true), - (b"2A05:D018:076C:B685:E8AB:AFD3:AF51:3AED", true), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", true), - (b"FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF", true), - (b"FFFF:ffff:ffff:ffff:ffff:ffff:ffff:ffff", true), // both case hex allowed + ipv6_address( + b"2a05:d018:076c:b685:e8ab:afd3:af51:3aed", + [ + 0x2a, 0x05, 0xd0, 0x18, 0x07, 0x6c, 0xb6, 0x85, 0xe8, 0xab, 0xaf, 0xd3, 0xaf, 0x51, + 0x3a, 0xed, + ], + ), + ipv6_address( + b"2A05:D018:076C:B685:E8AB:AFD3:AF51:3AED", + [ + 0x2a, 0x05, 0xd0, 0x18, 0x07, 0x6c, 0xb6, 0x85, 0xe8, 0xab, 0xaf, 0xd3, 0xaf, 0x51, + 0x3a, 0xed, + ], + ), + ipv6_address( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, + ], + ), + ipv6_address( + b"FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF:FFFF", + [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, + ], + ), + ipv6_address( + b"FFFF:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + [ + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, + ], + ), // Invalid IPv6 addresses // Missing octets on uncompressed addresses. The unmatching letter has the violation - (b"aaa:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:aaa:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:aaa:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:aaa:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:aaa:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:aaa:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:aaa:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:aaa", false), + ( + b"aaa:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:aaa:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:aaa:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:aaa:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:aaa:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:aaa:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:aaa:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:aaa", + Err(AddrParseError), + ), // Wrong hexadecimal characters on different positions - (b"ffgf:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:gfff:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:fffg:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffgf:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:gfff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:fgff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffgf:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffgf:fffg", false), + ( + b"ffgf:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:gfff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:fffg:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffgf:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:gfff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:fgff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffgf:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffgf:fffg", + Err(AddrParseError), + ), // Wrong colons on uncompressed addresses - (b":ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff::ffff:ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff::ffff:ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff::ffff:ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff::ffff:ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff::ffff:ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff::ffff:ffff", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffff::ffff", false), + ( + b":ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff::ffff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff::ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff::ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff::ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff::ffff:ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff::ffff:ffff", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffff::ffff", + Err(AddrParseError), + ), // More colons than allowed - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:", false), - (b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:", + Err(AddrParseError), + ), + ( + b"ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", + Err(AddrParseError), + ), // v Invalid UTF-8 encoding - (b"\xc3\x28a05:d018:076c:b685:e8ab:afd3:af51:3aed", false), + ( + b"\xc3\x28a05:d018:076c:b685:e8ab:afd3:af51:3aed", + Err(AddrParseError), + ), // v Invalid hexadecimal - (b"ga05:d018:076c:b685:e8ab:afd3:af51:3aed", false), + ( + b"ga05:d018:076c:b685:e8ab:afd3:af51:3aed", + Err(AddrParseError), + ), // Cannot start with colon - (b":a05:d018:076c:b685:e8ab:afd3:af51:3aed", false), + ( + b":a05:d018:076c:b685:e8ab:afd3:af51:3aed", + Err(AddrParseError), + ), // Cannot end with colon - (b"2a05:d018:076c:b685:e8ab:afd3:af51:3ae:", false), + ( + b"2a05:d018:076c:b685:e8ab:afd3:af51:3ae:", + Err(AddrParseError), + ), // Cannot have more than seven colons - (b"2a05:d018:076c:b685:e8ab:afd3:af51:3a::", false), + ( + b"2a05:d018:076c:b685:e8ab:afd3:af51:3a::", + Err(AddrParseError), + ), // Cannot contain two colons in a row - (b"2a05::018:076c:b685:e8ab:afd3:af51:3aed", false), + ( + b"2a05::018:076c:b685:e8ab:afd3:af51:3aed", + Err(AddrParseError), + ), // v Textual block size is longer - (b"2a056:d018:076c:b685:e8ab:afd3:af51:3ae", false), + ( + b"2a056:d018:076c:b685:e8ab:afd3:af51:3ae", + Err(AddrParseError), + ), // v Textual block size is shorter - (b"2a0:d018:076c:b685:e8ab:afd3:af51:3aed ", false), + ( + b"2a0:d018:076c:b685:e8ab:afd3:af51:3aed ", + Err(AddrParseError), + ), // Shorter IPv6 address - (b"d018:076c:b685:e8ab:afd3:af51:3aed", false), + (b"d018:076c:b685:e8ab:afd3:af51:3aed", Err(AddrParseError)), // Longer IPv6 address - (b"2a05:d018:076c:b685:e8ab:afd3:af51:3aed3aed", false), + ( + b"2a05:d018:076c:b685:e8ab:afd3:af51:3aed3aed", + Err(AddrParseError), + ), // These are valid IPv6 addresses, but we don't support compressed addresses - (b"0:0:0:0:0:0:0:1", false), - (b"2a05:d018:76c:b685:e8ab:afd3:af51:3aed", false), + (b"0:0:0:0:0:0:0:1", Err(AddrParseError)), + ( + b"2a05:d018:76c:b685:e8ab:afd3:af51:3aed", + Err(AddrParseError), + ), ]; #[test] - fn is_valid_ipv6_address_test() { - for &(ip_address, expected_result) in IPV6_ADDRESSES_VALIDITY { - assert_eq!( - is_valid_ipv6_address(untrusted::Input::from(ip_address)), - expected_result - ); + fn parse_ipv6_address_test() { + for &(ip_address, expected_result) in IPV6_ADDRESSES { + assert_eq!(parse_ipv6_address(ip_address), expected_result,); } } - #[test] - fn ipv6_octets_test() { - // Invalid UTF-8 encoding - assert_eq!( - ipv6_octets(b"\xc3\x28a05:d018:076c:b684:8e48:47c9:84aa:b34d"), - Err(AddrParseError), - ); - // Invalid hexadecimal - assert_eq!( - ipv6_octets(b"ga05:d018:076c:b684:8e48:47c9:84aa:b34d"), - Err(AddrParseError), - ); - } - #[test] fn try_from_ascii_ip_address_test() { const IP_ADDRESSES: &[(&[u8], Result)] = &[ diff --git a/src/name/name.rs b/src/name/name.rs index bb936e6a..3f34edfa 100644 --- a/src/name/name.rs +++ b/src/name/name.rs @@ -45,21 +45,15 @@ impl<'a> SubjectNameRef<'a> { /// consider parsing these with `std::net::IpAddr` and then using /// `IpAddr::from`. pub fn try_from_ascii(subject_name: &'a [u8]) -> Result { - if ip_address::is_valid_ipv4_address(untrusted::Input::from(subject_name)) { - return Ok(SubjectNameRef::IpAddress(IpAddrRef::V4( - subject_name, - ip_address::ipv4_octets(subject_name).map_err(|_| InvalidSubjectNameError)?, - ))); + if let Ok(ip_address) = ip_address::parse_ipv4_address(subject_name) { + return Ok(SubjectNameRef::IpAddress(ip_address)); + } else if let Ok(ip_address) = ip_address::parse_ipv6_address(subject_name) { + return Ok(SubjectNameRef::IpAddress(ip_address)); + } else { + Ok(SubjectNameRef::DnsName( + DnsNameRef::try_from_ascii(subject_name).map_err(|_| InvalidSubjectNameError)?, + )) } - if ip_address::is_valid_ipv6_address(untrusted::Input::from(subject_name)) { - return Ok(SubjectNameRef::IpAddress(IpAddrRef::V6( - subject_name, - ip_address::ipv6_octets(subject_name).map_err(|_| InvalidSubjectNameError)?, - ))); - } - Ok(SubjectNameRef::DnsName( - DnsNameRef::try_from_ascii(subject_name).map_err(|_| InvalidSubjectNameError)?, - )) } /// Constructs a `SubjectNameRef` from the given input if the From 40b84b6efe185d9d62ac4b632a7f10a8c7f64d2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Sat, 31 Dec 2022 13:01:48 +0100 Subject: [PATCH 4/5] Address code review comments Use `write_fmt` in order to reduce repetition of arguments when formatting an IPv6 address. --- src/name/ip_address.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/name/ip_address.rs b/src/name/ip_address.rs index a7c475ed..1fc162ad 100644 --- a/src/name/ip_address.rs +++ b/src/name/ip_address.rs @@ -12,6 +12,8 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +use core::fmt::Write; + use crate::Error; #[cfg(feature = "alloc")] @@ -131,17 +133,21 @@ impl<'a> IpAddrRef<'a> { #[cfg(feature = "std")] fn ipv6_to_uncompressed_string(octets: [u8; 16]) -> String { - format!( - "{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}:{:02x?}{:02x?}", - octets[0], octets[1], - octets[2], octets[3], - octets[4], octets[5], - octets[6], octets[7], - octets[8], octets[9], - octets[10], octets[11], - octets[12], octets[13], - octets[14], octets[15], - ) + let mut result = String::with_capacity(39); + for i in 0..7 { + result + .write_fmt(format_args!( + "{:02x?}{:02x?}:", + octets[i * 2], + octets[(i * 2) + 1] + )) + .expect("unexpected error while formatting IPv6 address"); + } + result + .write_fmt(format_args!("{:02x?}{:02x?}", octets[14], octets[15])) + .expect("unexpected error while formatting IPv6 address"); + + result } #[cfg(feature = "std")] From 327e47f09f24a938b9912c33e092ccfe177a0345 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Fern=C3=A1ndez=20L=C3=B3pez?= Date: Sat, 31 Dec 2022 13:43:21 +0100 Subject: [PATCH 5/5] clippy: remove as-conversions --- src/name/ip_address.rs | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/name/ip_address.rs b/src/name/ip_address.rs index 1fc162ad..f5ac5ced 100644 --- a/src/name/ip_address.rs +++ b/src/name/ip_address.rs @@ -12,7 +12,7 @@ // ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF // OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -use core::fmt::Write; +use core::{convert::TryInto, fmt::Write}; use crate::Error; @@ -313,11 +313,13 @@ pub(crate) fn parse_ipv4_address(ip_address_: &[u8]) -> Result 255 { + let current_raw_octet = radix10_to_octet(¤t_octet[..current_size]); + if current_raw_octet > 255 { // No octet can be greater than 255. return Err(AddrParseError); } - octets[octet] = radix10_to_octet(¤t_octet[..current_size]) as u8; + octets[octet] = + TryInto::::try_into(current_raw_octet).expect("invalid character"); octet += 1; // We move on to the next textual octet. current_octet = [0, 0, 0]; @@ -351,7 +353,7 @@ pub(crate) fn parse_ipv4_address(ip_address_: &[u8]) -> Result::try_into(last_octet).expect("invalid character"); break; } } @@ -414,15 +416,26 @@ pub(crate) fn parse_ipv6_address(ip_address_: &[u8]) -> Result::try_into( + TryInto::::try_into( + (TryInto::::try_into(previous_character_) + .expect("invalid character")) .to_digit(16) // Safe to unwrap because we know character is within hexadecimal bounds ([0-9a-f]) - .unwrap() as u8); + .unwrap(), + ) + .expect("invalid character"), + ) + .expect("invalid character") + << 4) + | (TryInto::::try_into( + TryInto::::try_into(character) + .expect("invalid character") + .to_digit(16) + // Safe to unwrap because we know character is within hexadecimal bounds ([0-9a-f]) + .unwrap(), + ) + .expect("invalid character")); previous_character = None; octet += 1; } else { @@ -471,6 +484,8 @@ mod tests { (b"...", Err(AddrParseError)), (b".0.0.0.0", Err(AddrParseError)), (b"0.0.0.0.", Err(AddrParseError)), + (b"0.0.0", Err(AddrParseError)), + (b"0.0.0.", Err(AddrParseError)), (b"256.0.0.0", Err(AddrParseError)), (b"0.256.0.0", Err(AddrParseError)), (b"0.0.256.0", Err(AddrParseError)),