change: use DERP port from host_name URL (#1143)

* change: use DERP port from host_name URL

* fix: typo

* change: DerpRegion::host_name -> DerpRegion::url

* fix: leftover renames

* chore: clippy

iroh-net/examples/magic.rs

@@ -52,20 +52,7 @@ async fn main() -> anyhow::Result<()> {
 
     let derp_map = match args.derp_url {
         None => default_derp_map(),
-        Some(url) => {
-            // TODO: This should be done by the DERP client.
-            let derp_port = match url.port() {
-                Some(port) => port,
-                None => match url.scheme() {
-                    "http" => 80,
-                    "https" => 443,
-                    _ => anyhow::bail!(
-                        "Invalid scheme in DERP URL, only http: and https: schemes are supported."
-                    ),
-                },
-            };
-            DerpMap::default_from_node(url, 3478, derp_port, UseIpv4::None, UseIpv6::None)
-        }
+        Some(url) => DerpMap::default_from_node(url, 3478, UseIpv4::None, UseIpv6::None),
     };
 
     let endpoint = MagicEndpoint::builder()

iroh-net/src/defaults.rs

@@ -13,12 +13,11 @@ pub fn default_derp_region() -> DerpRegion {
     let default_n0_derp = DerpNode {
         name: "default-1".into(),
         region_id: 1,
-        host_name: "https://derp.iroh.network".parse().unwrap(),
+        url: "https://derp.iroh.network".parse().unwrap(),
         stun_only: false,
         stun_port: 3478,
         ipv4: UseIpv4::Some([35, 175, 99, 113].into()),
         ipv6: UseIpv6::None,
-        derp_port: 443,
         stun_test_ip: None,
     };
     DerpRegion {

iroh-net/src/hp/derp/http.rs

@@ -80,13 +80,12 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "http://localhost".parse().unwrap(),
+                url: format!("http://localhost:{port}").parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,
                 ipv4: UseIpv4::Some(addr),
                 ipv6: UseIpv6::Disabled,
-                derp_port: port,
             }],
             region_code: "test_region".to_string(),
         };
@@ -221,13 +220,12 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "https://localhost".parse().unwrap(),
+                url: format!("https://localhost:{port}").parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,
                 ipv4: UseIpv4::Some(addr),
                 ipv6: UseIpv6::Disabled,
-                derp_port: port,
             }],
             region_code: "test_region".to_string(),
         };

iroh-net/src/hp/derp/http/client.rs

@@ -351,7 +351,7 @@ impl Client {
                 .and_then(|s| rustls::ServerName::try_from(s).ok());
         }
         if let Some(node) = node {
-            if let Some(host) = node.host_name.host_str() {
+            if let Some(host) = node.url.host_str() {
                 return rustls::ServerName::try_from(host).ok();
             }
         }
@@ -380,7 +380,7 @@ impl Client {
             return false;
         }
         if let Some(node) = node {
-            if node.host_name.scheme() == "http" {
+            if node.url.scheme() == "http" {
                 return false;
             }
         }
@@ -664,7 +664,7 @@ impl Client {
             UseIp::Ipv6(UseIpv6::Some(addr)) => addr.into(),
             _ => {
                 let host = node
-                    .host_name
+                    .url
                     .host()
                     .ok_or_else(|| ClientError::InvalidUrl("missing host".into()))?;
                 match host {
@@ -683,10 +683,16 @@ impl Client {
                 }
             }
         };
-        let port = if node.derp_port != 0 {
-            node.derp_port
-        } else {
-            443
+        let port = match node.url.port() {
+            Some(port) => port,
+            None => match node.url.scheme() {
+                "http" => 80,
+                "https" => 443,
+                _ => return Err(ClientError::InvalidUrl(
+                    "Invalid scheme in DERP hostname, only http: and https: schemes are supported."
+                        .into(),
+                )),
+            },
         };
         let dst = format!("{host}:{port}");
         debug!("dialing {}", dst);
@@ -1123,13 +1129,12 @@ mod tests {
             nodes: vec![DerpNode {
                 name: "test_node".to_string(),
                 region_id: 1,
-                host_name: "http://bad.url".parse().unwrap(),
+                url: "https://bad.url".parse().unwrap(),
                 stun_only: false,
                 stun_port: 0,
                 stun_test_ip: None,
                 ipv4: UseIpv4::Some("35.175.99.112".parse().unwrap()),
                 ipv6: UseIpv6::Disabled,
-                derp_port: 443,
             }],
             region_code: "test_region".to_string(),
         };

iroh-net/src/hp/derp/http/mesh_clients.rs

@@ -50,7 +50,7 @@ impl MeshClients {
                 for (_, region) in derp_map.regions.iter() {
                     for node in region.nodes.iter() {
                         // note: `node.host_name` is expected to include the scheme
-                        let mut url = node.host_name.clone();
+                        let mut url = node.url.clone();
                         url.set_path("/derp");
                         urls.push(url);
                     }

iroh-net/src/hp/derp/map.rs

@@ -25,9 +25,8 @@ impl DerpMap {
 
     /// Creates a new [`DerpMap`] with a single Derp server configured.
     pub fn default_from_node(
-        host_name: Url,
+        url: Url,
         stun_port: u16,
-        derp_port: u16,
         derp_ipv4: UseIpv4,
         derp_ipv6: UseIpv6,
     ) -> Self {
@@ -42,12 +41,11 @@ impl DerpMap {
                 nodes: vec![DerpNode {
                     name: "default-1".into(),
                     region_id: 1,
-                    host_name,
+                    url,
                     stun_only: !derp_ipv4.is_enabled() && !derp_ipv6.is_enabled(),
                     stun_port,
                     ipv4: derp_ipv4,
                     ipv6: derp_ipv6,
-                    derp_port,
                     stun_test_ip: None,
                 }],
                 avoid: false,
@@ -79,7 +77,7 @@ pub struct DerpRegion {
 pub struct DerpNode {
     pub name: String,
     pub region_id: u16,
-    pub host_name: Url,
+    pub url: Url,
     pub stun_only: bool,
     pub stun_port: u16,
     pub stun_test_ip: Option<IpAddr>,
@@ -91,7 +89,6 @@ pub struct DerpNode {
     /// If `None`, A record(s) from DNS lookups of HostName are used.
     /// If `Disabled`, IPv6 is not used;
     pub ipv6: UseIpv6,
-    pub derp_port: u16,
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]

iroh-net/src/hp/magicsock/conn.rs

@@ -2661,13 +2661,15 @@ pub(crate) mod tests {
                     nodes: vec![DerpNode {
                         name: "t1".into(),
                         region_id: 1,
-                        host_name: "https://test-node.invalid".parse().unwrap(),
+                        // In test mode, the DERP client does not validate HTTPS certs, so the host
+                        // name is irrelevant, but the port is used.
+                        url: format!("https://test-node.invalid:{}", https_addr.port())
+                            .parse()
+                            .unwrap(),
                         stun_only: false,
                         stun_port: stun_addr.port(),
                         ipv4: UseIpv4::Some("127.0.0.1".parse().unwrap()),
                         ipv6: UseIpv6::None,
-
-                        derp_port: https_addr.port(),
                         stun_test_ip: Some(stun_addr.ip()),
                     }],
                     avoid: false,

iroh-net/src/hp/netcheck.rs

@@ -323,7 +323,7 @@ async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option<u16>) -> Resu
     let node = &dm.regions.get(&preferred_derp).unwrap().nodes[0];
 
     if node
-        .host_name
+        .url
         .host_str()
         .map(|s| s.ends_with(&DOT_INVALID))
         .unwrap_or_default()
@@ -342,7 +342,7 @@ async fn check_captive_portal(dm: &DerpMap, preferred_derp: Option<u16>) -> Resu
     // length is limited; see is_challenge_char in bin/derper for more
     // details.
 
-    let host_name = node.host_name.host_str().unwrap_or_default();
+    let host_name = node.url.host_str().unwrap_or_default();
     let challenge = format!("ts_{}", host_name);
     let portal_url = format!("http://{}/generate_204", host_name);
     let res = client
@@ -441,7 +441,7 @@ async fn get_node_addr(n: &DerpNode, proto: ProbeProto) -> Result<SocketAddr> {
         }
     }
 
-    match n.host_name.host() {
+    match n.url.host() {
         Some(url::Host::Domain(hostname)) => {
             async move {
                 debug!(?proto, %hostname, "Performing DNS lookup for derp addr");
@@ -1742,7 +1742,7 @@ mod tests {
             .await
             .context("failed to create netcheck client")?;
 
-        let stun_servers = vec![("https://derp.iroh.network.", 3478, 0)];
+        let stun_servers = vec![("https://derp.iroh.network.", 3478)];
 
         let mut dm = DerpMap::default();
         dm.regions.insert(
@@ -1752,15 +1752,14 @@ mod tests {
                 nodes: stun_servers
                     .into_iter()
                     .enumerate()
-                    .map(|(i, (host_name, stun_port, derp_port))| DerpNode {
+                    .map(|(i, (host_name, stun_port))| DerpNode {
                         name: format!("default-{}", i),
                         region_id: 1,
-                        host_name: host_name.parse().unwrap(),
+                        url: host_name.parse().unwrap(),
                         stun_only: true,
                         stun_port,
                         ipv4: UseIpv4::None,
                         ipv6: UseIpv6::None,
-                        derp_port,
                         stun_test_ip: None,
                     })
                     .collect(),

iroh-net/src/hp/stun.rs

@@ -178,13 +178,12 @@ pub mod test {
             let node = DerpNode {
                 name: format!("{region_id}a"),
                 region_id,
-                host_name: format!("http://{region_id}.invalid").parse().unwrap(),
+                url: format!("http://{region_id}.invalid").parse().unwrap(),
                 ipv4,
                 ipv6,
                 stun_port: port,
                 stun_only: true,
                 stun_test_ip: None,
-                derp_port: 0,
             };
             m.regions.insert(
                 region_id,

iroh/src/commands/doctor.rs

@@ -206,9 +206,9 @@ async fn report(stun_host: Option<String>, stun_port: u16, config: &Config) -> a
 
     let dm = match stun_host {
         Some(host_name) => {
-            let host_name = host_name.parse()?;
+            let url = host_name.parse()?;
             // creating a derp map from host name and stun port
-            DerpMap::default_from_node(host_name, stun_port, 0, UseIpv4::None, UseIpv6::None)
+            DerpMap::default_from_node(url, stun_port, UseIpv4::None, UseIpv6::None)
         }
         None => config.derp_map().expect("derp map not configured"),
     };
@@ -445,11 +445,10 @@ async fn passive_side(connection: quinn::Connection) -> anyhow::Result<()> {
 
 fn configure_local_derp_map() -> DerpMap {
     let stun_port = 3478;
-    let host_name = "http://derp.invalid".parse().unwrap();
-    let derp_port = 3340;
+    let url = "http://derp.invalid:3340".parse().unwrap();
     let derp_ipv4 = UseIpv4::Some("127.0.0.1".parse().unwrap());
     let derp_ipv6 = UseIpv6::None;
-    DerpMap::default_from_node(host_name, stun_port, derp_port, derp_ipv4, derp_ipv6)
+    DerpMap::default_from_node(url, stun_port, derp_ipv4, derp_ipv6)
 }
 
 const DR_DERP_ALPN: [u8; 11] = *b"n0/drderp/1";