bugfixes and public ip change detection

veilid-core/src/intf/native/network/network_class_discovery.rs

@@ -207,7 +207,7 @@ impl DiscoveryContext {
         true
     }
 
-    pub async fn protocol_process_no_nat(&self) {
+    pub async fn protocol_process_no_nat(&self) -> Result<(), String> {
         let (node_b, external1_dial_info) = {
             let inner = self.inner.lock();
             (
@@ -226,7 +226,7 @@ impl DiscoveryContext {
                 RoutingDomain::PublicInternet,
                 external1_dial_info,
                 DialInfoClass::Direct,
-            );
+            )?;
         }
         // Attempt a UDP port mapping via all available and enabled mechanisms
         else if let Some(external_mapped_dial_info) = self.try_port_mapping().await {
@@ -235,19 +235,20 @@ impl DiscoveryContext {
                 RoutingDomain::PublicInternet,
                 external_mapped_dial_info,
                 DialInfoClass::Mapped,
-            );
+            )?;
         } else {
             // Add public dial info with Blocked dialinfo class
             self.routing_table.register_dial_info(
                 RoutingDomain::PublicInternet,
                 external1_dial_info,
                 DialInfoClass::Blocked,
-            );
+            )?;
         }
         self.upgrade_network_class(NetworkClass::InboundCapable);
+        Ok(())
     }
 
-    pub async fn protocol_process_nat(&self) -> bool {
+    pub async fn protocol_process_nat(&self) -> Result<bool, String> {
         let (node_b, external1_dial_info, external1, protocol_type, address_type) = {
             let inner = self.inner.lock();
             (
@@ -266,11 +267,11 @@ impl DiscoveryContext {
                 RoutingDomain::PublicInternet,
                 external_mapped_dial_info,
                 DialInfoClass::Mapped,
-            );
+            )?;
             self.upgrade_network_class(NetworkClass::InboundCapable);
 
             // No more retries
-            return true;
+            return Ok(true);
         }
 
         // Port mapping was not possible, let's see what kind of NAT we have
@@ -286,10 +287,10 @@ impl DiscoveryContext {
                 RoutingDomain::PublicInternet,
                 external1_dial_info,
                 DialInfoClass::FullConeNAT,
-            );
+            )?;
             self.upgrade_network_class(NetworkClass::InboundCapable);
 
-            return true;
+            return Ok(true);
         }
 
         // No, we are restricted, determine what kind of restriction
@@ -301,7 +302,7 @@ impl DiscoveryContext {
         {
             None => {
                 // If we can't get an external address, allow retry
-                return false;
+                return Ok(false);
             }
             Some(v) => v,
         };
@@ -312,7 +313,7 @@ impl DiscoveryContext {
             self.upgrade_network_class(NetworkClass::OutboundOnly);
 
             // No more retries
-            return true;
+            return Ok(true);
         }
 
         // If we're going to end up as a restricted NAT of some sort
@@ -329,19 +330,19 @@ impl DiscoveryContext {
                 RoutingDomain::PublicInternet,
                 external1_dial_info,
                 DialInfoClass::AddressRestrictedNAT,
-            );
+            )?;
         } else {
             // Didn't get a reply from a non-default port, which means we are also port restricted
             self.routing_table.register_dial_info(
                 RoutingDomain::PublicInternet,
                 external1_dial_info,
                 DialInfoClass::PortRestrictedNAT,
-            );
+            )?;
         }
         self.upgrade_network_class(NetworkClass::InboundCapable);
 
         // Allow another retry because sometimes trying again will get us Full Cone NAT instead
-        false
+        Ok(false)
     }
 }
 
@@ -379,7 +380,7 @@ impl Network {
                 };
                 if res {
                     // No NAT
-                    context.protocol_process_no_nat().await;
+                    context.protocol_process_no_nat().await?;
 
                     // No more retries
                     break;
@@ -387,7 +388,7 @@ impl Network {
             }
 
             // There is -some NAT-
-            if context.protocol_process_nat().await {
+            if context.protocol_process_nat().await? {
                 // We either got dial info or a network class without one
                 break;
             }
@@ -435,7 +436,7 @@ impl Network {
         }
 
         // No NAT
-        context.protocol_process_no_nat().await;
+        context.protocol_process_no_nat().await?;
 
         Ok(())
     }

veilid-core/src/intf/native/network/protocol/ws.rs

@@ -237,7 +237,7 @@ impl WebsocketProtocolHandler {
         if tls {
             let connector = TlsConnector::default();
             let tls_stream = connector
-                .connect(domain, tcp_stream)
+                .connect(domain.to_string(), tcp_stream)
                 .await
                 .map_err(map_to_string)
                 .map_err(logthru_net!(error))?;

veilid-core/src/intf/native/network/start_protocols.rs

@@ -300,7 +300,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     di.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
             }
 
@@ -309,7 +309,7 @@ impl Network {
                 RoutingDomain::LocalNetwork,
                 di.clone(),
                 DialInfoClass::Direct,
-            );
+            )?;
         }
 
         // Add static public dialinfo if it's configured
@@ -329,7 +329,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     pdi.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
 
                 // See if this public address is also a local interface address we haven't registered yet
                 let is_interface_address = self.with_interface_addresses(|ip_addrs| {
@@ -345,7 +345,7 @@ impl Network {
                         RoutingDomain::LocalNetwork,
                         DialInfo::udp_from_socketaddr(pdi_addr),
                         DialInfoClass::Direct,
-                    );
+                    )?;
                 }
 
                 static_public = true;
@@ -412,7 +412,7 @@ impl Network {
 
             // Resolve static public hostnames
             let global_socket_addrs = split_url
-                .host
+                .host_port(80)
                 .to_socket_addrs()
                 .await
                 .map_err(map_to_string)
@@ -427,7 +427,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     pdi.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
 
                 // See if this public address is also a local interface address
@@ -444,7 +444,7 @@ impl Network {
                         RoutingDomain::LocalNetwork,
                         pdi,
                         DialInfoClass::Direct,
-                    );
+                    )?;
                 }
 
                 registered_addresses.insert(gsa.ip());
@@ -468,7 +468,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     local_di.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
             }
 
@@ -477,7 +477,7 @@ impl Network {
                 RoutingDomain::LocalNetwork,
                 local_di,
                 DialInfoClass::Direct,
-            );
+            )?;
         }
 
         if static_public {
@@ -544,7 +544,7 @@ impl Network {
 
             // Resolve static public hostnames
             let global_socket_addrs = split_url
-                .host
+                .host_port(443)
                 .to_socket_addrs()
                 .await
                 .map_err(map_to_string)
@@ -559,7 +559,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     pdi.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
 
                 // See if this public address is also a local interface address
@@ -576,7 +576,7 @@ impl Network {
                         RoutingDomain::LocalNetwork,
                         pdi,
                         DialInfoClass::Direct,
-                    );
+                    )?;
                 }
 
                 registered_addresses.insert(gsa.ip());
@@ -643,7 +643,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     di.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
             }
             // Register interface dial info
@@ -651,7 +651,7 @@ impl Network {
                 RoutingDomain::LocalNetwork,
                 di.clone(),
                 DialInfoClass::Direct,
-            );
+            )?;
             registered_addresses.insert(socket_address.to_ip_addr());
         }
 
@@ -675,7 +675,7 @@ impl Network {
                     RoutingDomain::PublicInternet,
                     pdi.clone(),
                     DialInfoClass::Direct,
-                );
+                )?;
                 static_public = true;
 
                 // See if this public address is also a local interface address
@@ -692,7 +692,7 @@ impl Network {
                         RoutingDomain::LocalNetwork,
                         pdi,
                         DialInfoClass::Direct,
-                    );
+                    )?;
                 }
             }
         }

veilid-core/src/network_manager.rs

@@ -93,7 +93,7 @@ struct NetworkManagerInner {
     stats: NetworkManagerStats,
     client_whitelist: LruCache<key::DHTKey, ClientWhitelistEntry>,
     relay_node: Option<NodeRef>,
-    global_address_check_cache: LruCache<key::DHTKey, SocketAddress>,
+    public_address_check_cache: LruCache<key::DHTKey, SocketAddress>,
 }
 
 struct NetworkManagerUnlockedInner {
@@ -119,7 +119,7 @@ impl NetworkManager {
             stats: NetworkManagerStats::default(),
             client_whitelist: LruCache::new_unbounded(),
             relay_node: None,
-            global_address_check_cache: LruCache::new(8),
+            public_address_check_cache: LruCache::new(8),
         }
     }
     fn new_unlocked_inner(_config: VeilidConfig) -> NetworkManagerUnlockedInner {
@@ -1225,25 +1225,80 @@ impl NetworkManager {
         socket_address: SocketAddress,
         reporting_peer: NodeRef,
     ) {
+        let (net, routing_table) = {
             let mut inner = self.inner.lock();
+
+            // Store the reported address
             inner
-            .global_address_check_cache
+                .public_address_check_cache
                 .insert(reporting_peer.node_id(), socket_address);
 
             let net = inner.components.as_ref().unwrap().net.clone();
-
+            let routing_table = inner.routing_table.as_ref().unwrap().clone();
+            (net, routing_table)
+        };
         let network_class = net.get_network_class().unwrap_or(NetworkClass::Invalid);
 
+        // Determine if our external address has likely changed
+        let needs_public_address_detection =
             if matches!(network_class, NetworkClass::InboundCapable) {
+                // Get current external ip/port from registered global dialinfo
+                let current_addresses: BTreeSet<SocketAddress> = routing_table
+                    .all_filtered_dial_info_details(
+                        Some(RoutingDomain::PublicInternet),
+                        &DialInfoFilter::all(),
+                    )
+                    .iter()
+                    .map(|did| did.dial_info.socket_address())
+                    .collect();
+
                 // If we are inbound capable, but start to see inconsistent socket addresses from multiple reporting peers
                 // then we zap the network class and re-detect it
-
+                let inner = self.inner.lock();
-            // If we are inbound capable but start to see consistently different socket addresses from multiple reporting peers
+                let mut inconsistencies = 0;
-            // then we zap the network class and global dial info and re-detect it
+                let mut changed = false;
+                for (p, a) in &inner.public_address_check_cache {
+                    if !current_addresses.contains(a) {
+                        inconsistencies += 1;
+                        if inconsistencies >= GLOBAL_ADDRESS_CHANGE_DETECTION_COUNT {
+                            changed = true;
+                            break;
+                        }
+                    }
+                }
+                changed
             } else {
                 // If we are currently outbound only, we don't have any public dial info
                 // but if we are starting to see consistent socket address from multiple reporting peers
                 // then we may be become inbound capable, so zap the network class so we can re-detect it and any public dial info
+
+                let inner = self.inner.lock();
+                let mut consistencies = 0;
+                let mut consistent = false;
+                let mut current_address = Option::<SocketAddress>::None;
+                for (p, a) in &inner.public_address_check_cache {
+                    if let Some(current_address) = current_address {
+                        if current_address == *a {
+                            consistencies += 1;
+                            if consistencies >= GLOBAL_ADDRESS_CHANGE_DETECTION_COUNT {
+                                consistent = true;
+                                break;
+                            }
+                        }
+                    } else {
+                        current_address = Some(*a);
+                    }
+                }
+                consistent
+            };
+
+        if needs_public_address_detection {
+            // Reset the address check cache now so we can start detecting fresh
+            let mut inner = self.inner.lock();
+            inner.public_address_check_cache.clear();
+
+            // Reset the network class and dial info so we can re-detect it
+            routing_table.clear_dial_info_details(RoutingDomain::PublicInternet);
             net.reset_network_class();
         }
     }

veilid-core/src/routing_table/mod.rs

@@ -255,7 +255,13 @@ impl RoutingTable {
         domain: RoutingDomain,
         dial_info: DialInfo,
         class: DialInfoClass,
-    ) {
+    ) -> Result<(), String> {
+        trace!(
+            "registering dial_info with:\n  domain: {:?}\n  dial_info: {:?}\n  class: {:?}",
+            domain,
+            dial_info,
+            class
+        );
         let enable_local_peer_scope = {
             let config = self.network_manager().config();
             let c = config.get();
@@ -266,12 +272,15 @@ impl RoutingTable {
             && matches!(domain, RoutingDomain::PublicInternet)
             && dial_info.is_local()
         {
-            error!("shouldn't be registering local addresses as public");
+            return Err("shouldn't be registering local addresses as public".to_owned())
-            return;
+                .map_err(logthru_rtab!(error));
         }
         if !dial_info.is_valid() {
-            error!("shouldn't be registering invalid addresses");
+            return Err(format!(
-            return;
+                "shouldn't be registering invalid addresses: {:?}",
+                dial_info
+            ))
+            .map_err(logthru_rtab!(error));
         }
 
         let mut inner = self.inner.lock();
@@ -297,9 +306,12 @@ impl RoutingTable {
             .to_string(),
         );
         debug!("    Class: {:?}", class);
+        Ok(())
     }
 
     pub fn clear_dial_info_details(&self, domain: RoutingDomain) {
+        trace!("clearing dial info domain: {:?}", domain);
+
         let mut inner = self.inner.lock();
         Self::with_routing_domain_mut(&mut *inner, domain, |rd| {
             rd.dial_info_details.clear();

veilid-core/src/tests/common/test_host_interface.rs

@@ -362,25 +362,42 @@ macro_rules! assert_split_url_parse {
     };
 }
 
+fn host<S: AsRef<str>>(s: S) -> SplitUrlHost {
+    SplitUrlHost::Hostname(s.as_ref().to_owned())
+}
+
+fn ip<S: AsRef<str>>(s: S) -> SplitUrlHost {
+    SplitUrlHost::IpAddr(IpAddr::from_str(s.as_ref()).unwrap())
+}
+
 pub async fn test_split_url() {
     info!("testing split_url");
 
-    assert_split_url!("http://foo", "http", "foo");
+    assert_split_url!("http://foo", "http", host("foo"));
-    assert_split_url!("http://foo:1234", "http", "foo", Some(1234));
+    assert_split_url!("http://foo:1234", "http", host("foo"), Some(1234));
-    assert_split_url!("http://foo:1234/", "http", "foo", Some(1234), "");
+    assert_split_url!("http://foo:1234/", "http", host("foo"), Some(1234), "");
     assert_split_url!(
         "http://foo:1234/asdf/qwer",
         "http",
-        "foo",
+        host("foo"),
         Some(1234),
         "asdf/qwer"
     );
-    assert_split_url!("http://foo/", "http", "foo", None, "");
+    assert_split_url!("http://foo/", "http", host("foo"), None, "");
-    assert_split_url!("http://foo/asdf/qwer", "http", "foo", None, "asdf/qwer");
+    assert_split_url!("http://11.2.3.144/", "http", ip("11.2.3.144"), None, "");
+    assert_split_url!("http://[1111::2222]/", "http", ip("1111::2222"), None, "");
+
+    assert_split_url!(
+        "http://foo/asdf/qwer",
+        "http",
+        host("foo"),
+        None,
+        "asdf/qwer"
+    );
     assert_split_url!(
         "http://foo/asdf/qwer#3",
         "http",
-        "foo",
+        host("foo"),
         None,
         "asdf/qwer",
         Some("3"),
@@ -389,7 +406,7 @@ pub async fn test_split_url() {
     assert_split_url!(
         "http://foo/asdf/qwer?xxx",
         "http",
-        "foo",
+        host("foo"),
         None,
         "asdf/qwer",
         Option::<String>::None,
@@ -398,7 +415,7 @@ pub async fn test_split_url() {
     assert_split_url!(
         "http://foo/asdf/qwer#yyy?xxx",
         "http",
-        "foo",
+        host("foo"),
         None,
         "asdf/qwer",
         Some("yyy"),
@@ -423,6 +440,9 @@ pub async fn test_split_url() {
     assert_split_url_parse!("sch://foo:bar@baz.com:1234/fnord/");
     assert_split_url_parse!("sch://foo:bar@baz.com:1234//");
     assert_split_url_parse!("sch://foo:bar@baz.com:1234");
+    assert_split_url_parse!("sch://foo:bar@[1111::2222]:1234");
+    assert_split_url_parse!("sch://foo:bar@[::]:1234");
+    assert_split_url_parse!("sch://foo:bar@1.2.3.4:1234");
     assert_split_url_parse!("sch://@baz.com:1234");
     assert_split_url_parse!("sch://baz.com/asdf/asdf");
     assert_split_url_parse!("sch://baz.com/");

veilid-core/src/veilid_api/mod.rs

@@ -513,14 +513,18 @@ impl Address {
     }
     pub fn is_global(&self) -> bool {
         match self {
-            Address::IPV4(v4) => ipv4addr_is_global(v4),
+            Address::IPV4(v4) => ipv4addr_is_global(v4) && !ipv4addr_is_multicast(v4),
-            Address::IPV6(v6) => ipv6addr_is_global(v6),
+            Address::IPV6(v6) => ipv6addr_is_unicast_global(v6),
         }
     }
     pub fn is_local(&self) -> bool {
         match self {
-            Address::IPV4(v4) => ipv4addr_is_private(v4),
+            Address::IPV4(v4) => ipv4addr_is_private(v4) || ipv4addr_is_link_local(v4),
-            Address::IPV6(v6) => ipv6addr_is_unicast_site_local(v6),
+            Address::IPV6(v6) => {
+                ipv6addr_is_unicast_site_local(v6)
+                    || ipv6addr_is_unicast_link_local(v6)
+                    || ipv6addr_is_unique_local(v6)
+            }
         }
     }
     pub fn to_ip_addr(&self) -> IpAddr {
@@ -827,7 +831,7 @@ impl DialInfo {
                 url
             ));
         }
-        if Address::from_str(&split_url.host).is_ok() {
+        if !matches!(split_url.host, SplitUrlHost::Hostname(_)) {
             return Err(parse_error!(
                 "WSS url can not use address format, only hostname format",
                 url

veilid-core/src/xx/log_thru.rs

@@ -1,24 +1,13 @@
+// LogThru
+// Pass errors through and log them simultaneously via map_err()
+// Also contains common log facilities (net, rpc, rtab, pstore, crypto, etc )
+
 pub use alloc::string::{String, ToString};
 
 pub fn map_to_string<X: ToString>(arg: X) -> String {
     arg.to_string()
 }
 
-/*
-trait LogThru<T, E> {
-    fn log_thru<F, O: FnOnce(E) -> F>(self, op: O) -> Result<T, F>;
-}
-
-impl<T, E> LogThru<T, E> for Result<T, E> {
-    fn log_thru<F, O: FnOnce(E) -> F>(self, op: O) -> Result<T, F> {
-        match self {
-            Ok(t) => Ok(t),
-            Err(e) => Err(op(e)),
-        }
-    }
-}
-*/
-
 #[macro_export]
 macro_rules! fn_string {
     ($text:expr) => {
@@ -110,6 +99,62 @@ macro_rules! log_rtab {
     }
 }
 
+#[macro_export]
+macro_rules! log_pstore {
+    (error $text:expr) => { error!(
+        target: "pstore",
+        "{}",
+        $text,
+    )};
+    (error $fmt:literal, $($arg:expr),+) => {
+        error!(target:"pstore", $fmt, $($arg),+);
+    };
+    (warn $text:expr) => { warn!(
+        target: "pstore",
+        "{}",
+        $text,
+    )};
+    (warn $fmt:literal, $($arg:expr),+) => {
+        warn!(target:"pstore", $fmt, $($arg),+);
+    };
+    ($text:expr) => {trace!(
+        target: "pstore",
+        "{}",
+        $text,
+    )};
+    ($fmt:literal, $($arg:expr),+) => {
+        trace!(target:"pstore", $fmt, $($arg),+);
+    }
+}
+
+#[macro_export]
+macro_rules! log_crypto {
+    (error $text:expr) => { error!(
+        target: "crypto",
+        "{}",
+        $text,
+    )};
+    (error $fmt:literal, $($arg:expr),+) => {
+        error!(target:"crypto", $fmt, $($arg),+);
+    };
+    (warn $text:expr) => { warn!(
+        target: "crypto",
+        "{}",
+        $text,
+    )};
+    (warn $fmt:literal, $($arg:expr),+) => {
+        warn!(target:"crypto", $fmt, $($arg),+);
+    };
+    ($text:expr) => {trace!(
+        target: "crypto",
+        "{}",
+        $text,
+    )};
+    ($fmt:literal, $($arg:expr),+) => {
+        trace!(target:"crypto", $fmt, $($arg),+);
+    }
+}
+
 #[macro_export]
 macro_rules! logthru_net {
     ($($level:ident)?) => {

veilid-core/src/xx/split_url.rs

@@ -7,7 +7,7 @@
 //   URLs must convert to UTF8
 //   Only IP address and DNS hostname host fields are supported
 
-use super::IpAddr;
+use super::{IpAddr, Ipv4Addr, Ipv6Addr};
 use alloc::borrow::ToOwned;
 use alloc::string::String;
 use alloc::vec::Vec;
@@ -43,22 +43,6 @@ fn must_encode_path(c: u8) -> bool {
         ))
 }
 
-fn is_valid_host<H: AsRef<str>>(host: H) -> bool {
-    if host.as_ref().is_empty() {
-        return false;
-    }
-    if IpAddr::from_str(host.as_ref()).is_err() {
-        for ch in host.as_ref().chars() {
-            if !matches!(ch,
-                'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '.' )
-            {
-                return false;
-            }
-        }
-    }
-    true
-}
-
 fn is_valid_scheme<H: AsRef<str>>(host: H) -> bool {
     let mut chars = host.as_ref().chars();
     if let Some(ch) = chars.next() {
@@ -221,37 +205,95 @@ impl fmt::Display for SplitUrlPath {
     }
 }
 
+///////////////////////////////////////////////////////////////////////////////
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+pub enum SplitUrlHost {
+    Hostname(String),
+    IpAddr(IpAddr),
+}
+
+impl SplitUrlHost {
+    pub fn new<S: AsRef<str>>(s: S) -> Result<Self, String> {
+        Self::from_str(s.as_ref())
+    }
+}
+
+impl FromStr for SplitUrlHost {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.is_empty() {
+            return Err("Host is empty".to_owned());
+        }
+        if let Ok(v4) = Ipv4Addr::from_str(s) {
+            return Ok(SplitUrlHost::IpAddr(IpAddr::V4(v4)));
+        }
+        if &s[0..1] == "[" && &s[s.len() - 1..] == "]" {
+            if let Ok(v6) = Ipv6Addr::from_str(&s[1..s.len() - 1]) {
+                return Ok(SplitUrlHost::IpAddr(IpAddr::V6(v6)));
+            }
+            return Err("Invalid ipv6 address".to_owned());
+        }
+        for ch in s.chars() {
+            if !matches!(ch,
+                'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '.' )
+            {
+                return Err("Invalid hostname".to_owned());
+            }
+        }
+        Ok(SplitUrlHost::Hostname(s.to_owned()))
+    }
+}
+impl fmt::Display for SplitUrlHost {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Hostname(h) => {
+                write!(f, "{}", h)
+            }
+            Self::IpAddr(IpAddr::V4(v4)) => {
+                write!(f, "{}", v4)
+            }
+            Self::IpAddr(IpAddr::V6(v6)) => {
+                write!(f, "[{}]", v6)
+            }
+        }
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct SplitUrl {
     pub scheme: String,
     pub userinfo: Option<String>,
-    pub host: String,
+    pub host: SplitUrlHost,
     pub port: Option<u16>,
     pub path: Option<SplitUrlPath>,
 }
 
 impl SplitUrl {
-    pub fn new<S, H>(
+    pub fn new<S>(
         scheme: S,
         userinfo: Option<String>,
-        host: H,
+        host: SplitUrlHost,
         port: Option<u16>,
         path: Option<SplitUrlPath>,
     ) -> Self
     where
         S: AsRef<str>,
-        H: AsRef<str>,
     {
         Self {
             scheme: scheme.as_ref().to_owned(),
             userinfo,
-            host: host.as_ref().to_owned(),
+            host,
             port,
             path,
         }
     }
+
+    pub fn host_port(&self, default_port: u16) -> String {
+        format!("{}:{}", self.host, self.port.unwrap_or(default_port))
+    }
 }
 
 impl FromStr for SplitUrl {
@@ -270,9 +312,7 @@ impl FromStr for SplitUrl {
                 }
             };
             if let Some((host, rest)) = rest.rsplit_once(':') {
-                if !is_valid_host(host) {
+                let host = SplitUrlHost::from_str(host)?;
-                    return Err("Invalid host specified".to_owned());
-                }
                 if let Some((portstr, path)) = rest.split_once('/') {
                     let port = convert_port(portstr)?;
                     let path = SplitUrlPath::from_str(path)?;
@@ -288,16 +328,12 @@ impl FromStr for SplitUrl {
                     Ok(SplitUrl::new(scheme, userinfo, host, Some(port), None))
                 }
             } else if let Some((host, path)) = rest.split_once('/') {
-                if !is_valid_host(host) {
+                let host = SplitUrlHost::from_str(host)?;
-                    return Err("Invalid host specified".to_owned());
-                }
                 let path = SplitUrlPath::from_str(path)?;
                 Ok(SplitUrl::new(scheme, userinfo, host, None, Some(path)))
             } else {
-                if !is_valid_host(rest) {
+                let host = SplitUrlHost::from_str(rest)?;
-                    return Err("Invalid host specified".to_owned());
+                Ok(SplitUrl::new(scheme, userinfo, host, None, None))
-                }
-                Ok(SplitUrl::new(scheme, userinfo, rest, None, None))
             }
         } else {
             Err("No scheme specified".to_owned())
@@ -316,7 +352,7 @@ impl fmt::Display for SplitUrl {
                     format!("{}@{}", userinfo, self.host)
                 }
             } else {
-                self.host.clone()
+                self.host.to_string()
             }
         };
         if let Some(path) = &self.path {