Rollup merge of #146541 - joboet:simplify-lookup-host, r=tgross35

std: simplify host lookup

The logic for splitting up a string into a hostname and port is currently duplicated across (nearly) all of the networking implementations in `sys`. Since it does not actually rely on any system internals, this PR moves it to the `ToSocketAddr` implementation for `&str`, making it easier to discover and maintain.

On the other hand, the `ToSocketAddr` implementation (or rather the `resolve_socket_addr` function) contained logic to overwrite the port on the socket addresses returned by `LookupHost`, even though `LookupHost` is already aware of the port and sets the port already on Xous. This PR thus removes this logic by moving the responsibility of setting the port to the system-specific `LookupHost` implementation.

As a consequence of these changes, there remains only one way of creating `LookupHost`, hence I've removed the `TryFrom` implementations in favour of a `lookup_host` function, mirroring other, public iterator-based features.

And finally, I've simplified the parsing logic responsible for recognising IP addresses passed to `<(&str, u16)>::to_socket_addrs()` by using the `FromStr` impl of `IpAddr` rather than duplicating the parsing for both IP versions.

9 files changed, 50 insertions, 193 deletions

diff --git a/library/std/src/net/socket_addr.rs b/library/std/src/net/socket_addr.rs
index 41e623e79ce..5b56dd3f744 100644
--- a/library/std/src/net/socket_addr.rs
+++ b/library/std/src/net/socket_addr.rs
@@ -6,7 +6,6 @@ mod tests;
 pub use core::net::{SocketAddr, SocketAddrV4, SocketAddrV6};
 
 use crate::net::{IpAddr, Ipv4Addr, Ipv6Addr};
-use crate::sys::net::LookupHost;
 use crate::{io, iter, option, slice, vec};
 
 /// A trait for objects which can be converted or resolved to one or more
@@ -188,15 +187,9 @@ impl ToSocketAddrs for (Ipv6Addr, u16) {
     }
 }
 
-fn resolve_socket_addr(lh: LookupHost) -> io::Result<vec::IntoIter<SocketAddr>> {
-    let p = lh.port();
-    let v: Vec<_> = lh
-        .map(|mut a| {
-            a.set_port(p);
-            a
-        })
-        .collect();
-    Ok(v.into_iter())
+fn lookup_host(host: &str, port: u16) -> io::Result<vec::IntoIter<SocketAddr>> {
+    let addrs = crate::sys::net::lookup_host(host, port)?;
+    Ok(Vec::from_iter(addrs).into_iter())
 }
 
 #[stable(feature = "rust1", since = "1.0.0")]
@@ -205,17 +198,14 @@ impl ToSocketAddrs for (&str, u16) {
     fn to_socket_addrs(&self) -> io::Result<vec::IntoIter<SocketAddr>> {
         let (host, port) = *self;
 
-        // try to parse the host as a regular IP address first
-        if let Ok(addr) = host.parse::<Ipv4Addr>() {
-            let addr = SocketAddrV4::new(addr, port);
-            return Ok(vec![SocketAddr::V4(addr)].into_iter());
-        }
-        if let Ok(addr) = host.parse::<Ipv6Addr>() {
-            let addr = SocketAddrV6::new(addr, port, 0, 0);
-            return Ok(vec![SocketAddr::V6(addr)].into_iter());
+        // Try to parse the host as a regular IP address first
+        if let Ok(addr) = host.parse::<IpAddr>() {
+            let addr = SocketAddr::new(addr, port);
+            return Ok(vec![addr].into_iter());
         }
 
-        resolve_socket_addr((host, port).try_into()?)
+        // Otherwise, make the system look it up.
+        lookup_host(host, port)
     }
 }
 
@@ -232,12 +222,21 @@ impl ToSocketAddrs for (String, u16) {
 impl ToSocketAddrs for str {
     type Iter = vec::IntoIter<SocketAddr>;
     fn to_socket_addrs(&self) -> io::Result<vec::IntoIter<SocketAddr>> {
-        // try to parse as a regular SocketAddr first
+        // Try to parse as a regular SocketAddr first
         if let Ok(addr) = self.parse() {
             return Ok(vec![addr].into_iter());
         }
 
-        resolve_socket_addr(self.try_into()?)
+        // Otherwise, split the string by ':' and convert the second part to u16...
+        let Some((host, port_str)) = self.rsplit_once(':') else {
+            return Err(io::const_error!(io::ErrorKind::InvalidInput, "invalid socket address"));
+        };
+        let Ok(port) = port_str.parse::<u16>() else {
+            return Err(io::const_error!(io::ErrorKind::InvalidInput, "invalid port value"));
+        };
+
+        // ... and make the system look up the host.
+        lookup_host(host, port)
     }
 }
 
diff --git a/library/std/src/sys/net/connection/sgx.rs b/library/std/src/sys/net/connection/sgx.rs
index 9b54571997d..8c9c17d3f17 100644
--- a/library/std/src/sys/net/connection/sgx.rs
+++ b/library/std/src/sys/net/connection/sgx.rs
@@ -499,16 +499,6 @@ impl fmt::Display for NonIpSockAddr {
 
 pub struct LookupHost(!);
 
-impl LookupHost {
-    fn new(host: String) -> io::Result<LookupHost> {
-        Err(io::Error::new(io::ErrorKind::Uncategorized, NonIpSockAddr { host }))
-    }
-
-    pub fn port(&self) -> u16 {
-        self.0
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -516,18 +506,9 @@ impl Iterator for LookupHost {
     }
 }
 
-impl TryFrom<&str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(v: &str) -> io::Result<LookupHost> {
-        LookupHost::new(v.to_owned())
-    }
-}
-
-impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from((host, port): (&'a str, u16)) -> io::Result<LookupHost> {
-        LookupHost::new(format!("{host}:{port}"))
-    }
+pub fn lookup_host(host: &str, port: u16) -> io::Result<LookupHost> {
+    Err(io::Error::new(
+        io::ErrorKind::Uncategorized,
+        NonIpSockAddr { host: format!("{host}:{port}") },
+    ))
 }
diff --git a/library/std/src/sys/net/connection/socket/mod.rs b/library/std/src/sys/net/connection/socket/mod.rs
index 564f2e3a01f..1dd06e97bba 100644
--- a/library/std/src/sys/net/connection/socket/mod.rs
+++ b/library/std/src/sys/net/connection/socket/mod.rs
@@ -258,7 +258,7 @@ fn to_ipv6mr_interface(value: u32) -> crate::ffi::c_uint {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// get_host_addresses
+// lookup_host
 ////////////////////////////////////////////////////////////////////////////////
 
 pub struct LookupHost {
@@ -267,12 +267,6 @@ pub struct LookupHost {
     port: u16,
 }
 
-impl LookupHost {
-    pub fn port(&self) -> u16 {
-        self.port
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -281,7 +275,10 @@ impl Iterator for LookupHost {
                 let cur = self.cur.as_ref()?;
                 self.cur = cur.ai_next;
                 match socket_addr_from_c(cur.ai_addr.cast(), cur.ai_addrlen as usize) {
-                    Ok(addr) => return Some(addr),
+                    Ok(mut addr) => {
+                        addr.set_port(self.port);
+                        return Some(addr);
+                    }
                     Err(_) => continue,
                 }
             }
@@ -298,42 +295,17 @@ impl Drop for LookupHost {
     }
 }
 
-impl TryFrom<&str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(s: &str) -> io::Result<LookupHost> {
-        macro_rules! try_opt {
-            ($e:expr, $msg:expr) => {
-                match $e {
-                    Some(r) => r,
-                    None => return Err(io::const_error!(io::ErrorKind::InvalidInput, $msg)),
-                }
-            };
+pub fn lookup_host(host: &str, port: u16) -> io::Result<LookupHost> {
+    init();
+    run_with_cstr(host.as_bytes(), &|c_host| {
+        let mut hints: c::addrinfo = unsafe { mem::zeroed() };
+        hints.ai_socktype = c::SOCK_STREAM;
+        let mut res = ptr::null_mut();
+        unsafe {
+            cvt_gai(c::getaddrinfo(c_host.as_ptr(), ptr::null(), &hints, &mut res))
+                .map(|_| LookupHost { original: res, cur: res, port })
         }
-
-        // split the string by ':' and convert the second part to u16
-        let (host, port_str) = try_opt!(s.rsplit_once(':'), "invalid socket address");
-        let port: u16 = try_opt!(port_str.parse().ok(), "invalid port value");
-        (host, port).try_into()
-    }
-}
-
-impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from((host, port): (&'a str, u16)) -> io::Result<LookupHost> {
-        init();
-
-        run_with_cstr(host.as_bytes(), &|c_host| {
-            let mut hints: c::addrinfo = unsafe { mem::zeroed() };
-            hints.ai_socktype = c::SOCK_STREAM;
-            let mut res = ptr::null_mut();
-            unsafe {
-                cvt_gai(c::getaddrinfo(c_host.as_ptr(), ptr::null(), &hints, &mut res))
-                    .map(|_| LookupHost { original: res, cur: res, port })
-            }
-        })
-    }
+    })
 }
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/library/std/src/sys/net/connection/socket/tests.rs b/library/std/src/sys/net/connection/socket/tests.rs
index fc236b8027b..049355afca7 100644
--- a/library/std/src/sys/net/connection/socket/tests.rs
+++ b/library/std/src/sys/net/connection/socket/tests.rs
@@ -4,7 +4,7 @@ use crate::collections::HashMap;
 #[test]
 fn no_lookup_host_duplicates() {
     let mut addrs = HashMap::new();
-    let lh = match LookupHost::try_from(("localhost", 0)) {
+    let lh = match lookup_host("localhost", 0) {
         Ok(lh) => lh,
         Err(e) => panic!("couldn't resolve `localhost`: {e}"),
     };
diff --git a/library/std/src/sys/net/connection/uefi/mod.rs b/library/std/src/sys/net/connection/uefi/mod.rs
index 00368042873..004f6d413a1 100644
--- a/library/std/src/sys/net/connection/uefi/mod.rs
+++ b/library/std/src/sys/net/connection/uefi/mod.rs
@@ -333,12 +333,6 @@ impl fmt::Debug for UdpSocket {
 
 pub struct LookupHost(!);
 
-impl LookupHost {
-    pub fn port(&self) -> u16 {
-        self.0
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -346,18 +340,6 @@ impl Iterator for LookupHost {
     }
 }
 
-impl TryFrom<&str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: &str) -> io::Result<LookupHost> {
-        unsupported()
-    }
-}
-
-impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: (&'a str, u16)) -> io::Result<LookupHost> {
-        unsupported()
-    }
+pub fn lookup_host(_host: &str, _port: u16) -> io::Result<LookupHost> {
+    unsupported()
 }
diff --git a/library/std/src/sys/net/connection/unsupported.rs b/library/std/src/sys/net/connection/unsupported.rs
index fbc86343272..fb18e8dec55 100644
--- a/library/std/src/sys/net/connection/unsupported.rs
+++ b/library/std/src/sys/net/connection/unsupported.rs
@@ -304,12 +304,6 @@ impl fmt::Debug for UdpSocket {
 
 pub struct LookupHost(!);
 
-impl LookupHost {
-    pub fn port(&self) -> u16 {
-        self.0
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -317,18 +311,6 @@ impl Iterator for LookupHost {
     }
 }
 
-impl TryFrom<&str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: &str) -> io::Result<LookupHost> {
-        unsupported()
-    }
-}
-
-impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: (&'a str, u16)) -> io::Result<LookupHost> {
-        unsupported()
-    }
+pub fn lookup_host(_host: &str, _port: u16) -> io::Result<LookupHost> {
+    unsupported()
 }
diff --git a/library/std/src/sys/net/connection/wasip1.rs b/library/std/src/sys/net/connection/wasip1.rs
index cdfa25c8a44..048dafdcd7f 100644
--- a/library/std/src/sys/net/connection/wasip1.rs
+++ b/library/std/src/sys/net/connection/wasip1.rs
@@ -477,12 +477,6 @@ impl fmt::Debug for UdpSocket {
 
 pub struct LookupHost(!);
 
-impl LookupHost {
-    pub fn port(&self) -> u16 {
-        self.0
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -490,18 +484,6 @@ impl Iterator for LookupHost {
     }
 }
 
-impl<'a> TryFrom<&'a str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: &'a str) -> io::Result<LookupHost> {
-        unsupported()
-    }
-}
-
-impl<'a> TryFrom<(&'a str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(_v: (&'a str, u16)) -> io::Result<LookupHost> {
-        unsupported()
-    }
+pub fn lookup_host(_host: &str, _port: u16) -> io::Result<LookupHost> {
+    unsupported()
 }
diff --git a/library/std/src/sys/net/connection/xous/dns.rs b/library/std/src/sys/net/connection/xous/dns.rs
index bb29d211fad..b139376f597 100644
--- a/library/std/src/sys/net/connection/xous/dns.rs
+++ b/library/std/src/sys/net/connection/xous/dns.rs
@@ -1,15 +1,8 @@
-use core::convert::{TryFrom, TryInto};
-
 use crate::io;
 use crate::net::{Ipv4Addr, SocketAddr, SocketAddrV4, SocketAddrV6};
 use crate::os::xous::ffi::lend_mut;
 use crate::os::xous::services::{DnsLendMut, dns_server};
 
-pub struct DnsError {
-    #[allow(dead_code)]
-    pub code: u8,
-}
-
 #[repr(C, align(4096))]
 struct LookupHostQuery([u8; 4096]);
 
@@ -20,12 +13,6 @@ pub struct LookupHost {
     count: usize,
 }
 
-impl LookupHost {
-    pub fn port(&self) -> u16 {
-        self.port
-    }
-}
-
 impl Iterator for LookupHost {
     type Item = SocketAddr;
     fn next(&mut self) -> Option<SocketAddr> {
@@ -72,7 +59,7 @@ impl Iterator for LookupHost {
     }
 }
 
-pub fn lookup(query: &str, port: u16) -> Result<LookupHost, DnsError> {
+pub fn lookup_host(query: &str, port: u16) -> io::Result<LookupHost> {
     let mut result = LookupHost { data: LookupHostQuery([0u8; 4096]), offset: 0, count: 0, port };
 
     // Copy the query into the message that gets sent to the DNS server
@@ -89,7 +76,7 @@ pub fn lookup(query: &str, port: u16) -> Result<LookupHost, DnsError> {
     )
     .unwrap();
     if result.data.0[0] != 0 {
-        return Err(DnsError { code: result.data.0[1] });
+        return Err(io::const_error!(io::ErrorKind::InvalidInput, "DNS failure"));
     }
     assert_eq!(result.offset, 0);
     result.count = result.data.0[1] as usize;
@@ -98,31 +85,3 @@ pub fn lookup(query: &str, port: u16) -> Result<LookupHost, DnsError> {
     result.offset = 2;
     Ok(result)
 }
-
-impl TryFrom<&str> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(s: &str) -> io::Result<LookupHost> {
-        macro_rules! try_opt {
-            ($e:expr, $msg:expr) => {
-                match $e {
-                    Some(r) => r,
-                    None => return Err(io::const_error!(io::ErrorKind::InvalidInput, &$msg)),
-                }
-            };
-        }
-
-        // split the string by ':' and convert the second part to u16
-        let (host, port_str) = try_opt!(s.rsplit_once(':'), "invalid socket address");
-        let port: u16 = try_opt!(port_str.parse().ok(), "invalid port value");
-        (host, port).try_into()
-    }
-}
-
-impl TryFrom<(&str, u16)> for LookupHost {
-    type Error = io::Error;
-
-    fn try_from(v: (&str, u16)) -> io::Result<LookupHost> {
-        lookup(v.0, v.1).map_err(|_e| io::const_error!(io::ErrorKind::InvalidInput, "DNS failure"))
-    }
-}
diff --git a/library/std/src/sys/net/connection/xous/mod.rs b/library/std/src/sys/net/connection/xous/mod.rs
index e44a375b9e3..0f77be5c3fa 100644
--- a/library/std/src/sys/net/connection/xous/mod.rs
+++ b/library/std/src/sys/net/connection/xous/mod.rs
@@ -45,4 +45,4 @@ pub struct GetAddress {
     raw: [u8; 4096],
 }
 
-pub use dns::LookupHost;
+pub use dns::lookup_host;