Skip to content

Commit

Permalink
Improve AddrParseError description
Browse files Browse the repository at this point in the history
The existing description was incorrect for socket addresses, and
misleading: users would see “invalid IP address syntax” and suppose they
were supposed to provide an IP address rather than a socket address.

I contemplated making it two variants (IP, socket), but realised we can
do still better for the IPv4 and IPv6 types, so here it is as six.

I contemplated more precise error descriptions (e.g. “invalid IPv6
socket address syntax: expected a decimal scope ID after %”), but that’s
a more invasive change, and probably not worthwhile anyway.
  • Loading branch information
chris-morgan committed Apr 19, 2022
1 parent 7b5408d commit 0255398
Showing 1 changed file with 28 additions and 11 deletions.
39 changes: 28 additions & 11 deletions library/std/src/net/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ impl<'a> Parser<'a> {

/// Run a parser, but fail if the entire input wasn't consumed.
/// Doesn't run atomically.
fn parse_with<T, F>(&mut self, inner: F) -> Result<T, AddrParseError>
fn parse_with<T, F>(&mut self, inner: F, kind: AddrKind) -> Result<T, AddrParseError>
where
F: FnOnce(&mut Parser<'_>) -> Option<T>,
{
let result = inner(self);
if self.state.is_empty() { result } else { None }.ok_or(AddrParseError(()))
if self.state.is_empty() { result } else { None }.ok_or(AddrParseError(kind))
}

/// Peek the next character from the input
Expand Down Expand Up @@ -278,7 +278,7 @@ impl<'a> Parser<'a> {
impl FromStr for IpAddr {
type Err = AddrParseError;
fn from_str(s: &str) -> Result<IpAddr, AddrParseError> {
Parser::new(s).parse_with(|p| p.read_ip_addr())
Parser::new(s).parse_with(|p| p.read_ip_addr(), AddrKind::Ip)
}
}

Expand All @@ -288,9 +288,9 @@ impl FromStr for Ipv4Addr {
fn from_str(s: &str) -> Result<Ipv4Addr, AddrParseError> {
// don't try to parse if too long
if s.len() > 15 {
Err(AddrParseError(()))
Err(AddrParseError(AddrKind::Ipv4))
} else {
Parser::new(s).parse_with(|p| p.read_ipv4_addr())
Parser::new(s).parse_with(|p| p.read_ipv4_addr(), AddrKind::Ipv4)
}
}
}
Expand All @@ -299,34 +299,44 @@ impl FromStr for Ipv4Addr {
impl FromStr for Ipv6Addr {
type Err = AddrParseError;
fn from_str(s: &str) -> Result<Ipv6Addr, AddrParseError> {
Parser::new(s).parse_with(|p| p.read_ipv6_addr())
Parser::new(s).parse_with(|p| p.read_ipv6_addr(), AddrKind::Ipv6)
}
}

#[stable(feature = "socket_addr_from_str", since = "1.5.0")]
impl FromStr for SocketAddrV4 {
type Err = AddrParseError;
fn from_str(s: &str) -> Result<SocketAddrV4, AddrParseError> {
Parser::new(s).parse_with(|p| p.read_socket_addr_v4())
Parser::new(s).parse_with(|p| p.read_socket_addr_v4(), AddrKind::SocketV4)
}
}

#[stable(feature = "socket_addr_from_str", since = "1.5.0")]
impl FromStr for SocketAddrV6 {
type Err = AddrParseError;
fn from_str(s: &str) -> Result<SocketAddrV6, AddrParseError> {
Parser::new(s).parse_with(|p| p.read_socket_addr_v6())
Parser::new(s).parse_with(|p| p.read_socket_addr_v6(), AddrKind::SocketV6)
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl FromStr for SocketAddr {
type Err = AddrParseError;
fn from_str(s: &str) -> Result<SocketAddr, AddrParseError> {
Parser::new(s).parse_with(|p| p.read_socket_addr())
Parser::new(s).parse_with(|p| p.read_socket_addr(), AddrKind::Socket)
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
enum AddrKind {
Ip,
Ipv4,
Ipv6,
Socket,
SocketV4,
SocketV6,
}

/// An error which can be returned when parsing an IP address or a socket address.
///
/// This error is used as the error type for the [`FromStr`] implementation for
Expand All @@ -353,7 +363,7 @@ impl FromStr for SocketAddr {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AddrParseError(());
pub struct AddrParseError(AddrKind);

#[stable(feature = "addr_parse_error_error", since = "1.4.0")]
impl fmt::Display for AddrParseError {
Expand All @@ -367,6 +377,13 @@ impl fmt::Display for AddrParseError {
impl Error for AddrParseError {
#[allow(deprecated)]
fn description(&self) -> &str {
"invalid IP address syntax"
match self.0 {
AddrKind::Ip => "invalid IP address syntax",
AddrKind::Ipv4 => "invalid IPv4 address syntax",
AddrKind::Ipv6 => "invalid IPv6 address syntax",
AddrKind::Socket => "invalid socket address syntax",
AddrKind::SocketV4 => "invalid IPv4 socket address syntax",
AddrKind::SocketV6 => "invalid IPv6 socket address syntax",
}
}
}

0 comments on commit 0255398

Please sign in to comment.