refactor(iroh-net): Avoid using .unwrap() calls (#2046)

## Description

This is an attempt at an audit of all the .unwrap() calls in iroh-net.
A few are replaced by .expect() with clear reasons why they're
reasonable.  Some are replaces by non-panicking versions which will
never take the failure branch because of the invariants that are
supposed to be upheld.

Closes #1528

## Notes & open questions

This was surprisingly horrible and long to do.  Oh well.
Worth reviewing in detail I'm afraid.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

iroh-net/src/bin/derper.rs

@@ -13,7 +13,7 @@ use std::{
 use anyhow::{anyhow, bail, Context as _, Result};
 use clap::Parser;
 use futures::{Future, StreamExt};
-use http::response::Builder as ResponseBuilder;
+use http::{response::Builder as ResponseBuilder, HeaderMap};
 use hyper::body::Incoming;
 use hyper::{Method, Request, Response, StatusCode};
 use iroh_metrics::inc;
@@ -89,12 +89,13 @@ impl CertMode {
 
                 tokio::spawn(
                     async move {
-                        loop {
-                            match state.next().await.unwrap() {
+                        while let Some(event) = state.next().await {
+                            match event {
                                 Ok(ok) => debug!("acme event: {:?}", ok),
                                 Err(err) => error!("error: {:?}", err),
                             }
                         }
+                        debug!("event stream finished");
                     }
                     .instrument(info_span!("acme")),
                 );
@@ -125,7 +126,8 @@ impl CertMode {
 }
 
 fn escape_hostname(hostname: &str) -> Cow<'_, str> {
-    let unsafe_hostname_characters = regex::Regex::new(r"[^a-zA-Z0-9-\.]").unwrap();
+    let unsafe_hostname_characters =
+        regex::Regex::new(r"[^a-zA-Z0-9-\.]").expect("regex manually checked");
     unsafe_hostname_characters.replace_all(hostname, "")
 }
 
@@ -249,7 +251,7 @@ impl Default for Config {
     fn default() -> Self {
         Self {
             secret_key: SecretKey::generate(),
-            addr: "[::]:443".parse().unwrap(),
+            addr: (Ipv6Addr::UNSPECIFIED, 443).into(),
             stun_port: DEFAULT_DERP_STUN_PORT,
             hostname: NA_DERP_HOSTNAME.into(),
             enable_stun: true,
@@ -444,7 +446,10 @@ async fn run(
                 tls_config.cert_dir.unwrap_or_else(|| PathBuf::from(".")),
             )
             .await?;
-        let headers: Vec<(&str, &str)> = TLS_HEADERS.into();
+        let mut headers = HeaderMap::new();
+        for (name, value) in TLS_HEADERS.iter() {
+            headers.insert(*name, value.parse()?);
+        }
         (
             Some(DerpTlsConfig { config, acceptor }),
             headers,
@@ -453,7 +458,7 @@ async fn run(
                 .unwrap_or(DEFAULT_CAPTIVE_PORTAL_PORT),
         )
     } else {
-        (None, Vec::new(), 0)
+        (None, HeaderMap::new(), 0)
     };
 
     let mut builder = DerpServerBuilder::new(addr)
@@ -592,8 +597,8 @@ impl hyper::service::Service<Request<Incoming>> for CaptivePortalService {
                 let r = Response::builder()
                     .status(StatusCode::NOT_FOUND)
                     .body(NOTFOUND.into())
-                    .unwrap();
-                Box::pin(async move { Ok(r) })
+                    .map_err(|err| Box::new(err) as HyperError);
+                Box::pin(async move { r })
             }
         }
     }
@@ -603,47 +608,43 @@ fn derp_disabled_handler(
     _r: Request<Incoming>,
     response: ResponseBuilder,
 ) -> HyperResult<Response<BytesBody>> {
-    Ok(response
+    response
         .status(StatusCode::NOT_FOUND)
         .body(DERP_DISABLED.into())
-        .unwrap())
+        .map_err(|err| Box::new(err) as HyperError)
 }
 
 fn root_handler(
     _r: Request<Incoming>,
     response: ResponseBuilder,
 ) -> HyperResult<Response<BytesBody>> {
-    let response = response
+    response
         .status(StatusCode::OK)
         .header("Content-Type", "text/html; charset=utf-8")
         .body(INDEX.into())
-        .unwrap();
-
-    Ok(response)
+        .map_err(|err| Box::new(err) as HyperError)
 }
 
 /// HTTP latency queries
 fn probe_handler(
     _r: Request<Incoming>,
     response: ResponseBuilder,
 ) -> HyperResult<Response<BytesBody>> {
-    let response = response
+    response
         .status(StatusCode::OK)
         .header("Access-Control-Allow-Origin", "*")
         .body(body_empty())
-        .unwrap();
-
-    Ok(response)
+        .map_err(|err| Box::new(err) as HyperError)
 }
 
 fn robots_handler(
     _r: Request<Incoming>,
     response: ResponseBuilder,
 ) -> HyperResult<Response<BytesBody>> {
-    Ok(response
+    response
         .status(StatusCode::OK)
         .body(ROBOTS_TXT.into())
-        .unwrap())
+        .map_err(|err| Box::new(err) as HyperError)
 }
 
 /// For captive portal detection.
@@ -666,10 +667,10 @@ fn serve_no_content_handler<B: hyper::body::Body>(
         }
     }
 
-    Ok(response
+    response
         .status(StatusCode::NO_CONTENT)
         .body(body_empty())
-        .unwrap())
+        .map_err(|err| Box::new(err) as HyperError)
 }
 
 fn is_challenge_char(c: char) -> bool {
@@ -714,14 +715,20 @@ async fn server_stun_listener(sock: UdpSocket) {
                     }
                     match tokio::task::spawn_blocking(move || stun::parse_binding_request(&pkt))
                         .await
-                        .unwrap()
                     {
-                        Ok(txid) => {
+                        Ok(Ok(txid)) => {
                             debug!(%src_addr, %txid, "STUN: received binding request");
-                            let res =
-                                tokio::task::spawn_blocking(move || stun::response(txid, src_addr))
-                                    .await
-                                    .unwrap();
+                            let res = match tokio::task::spawn_blocking(move || {
+                                stun::response(txid, src_addr)
+                            })
+                            .await
+                            {
+                                Ok(res) => res,
+                                Err(err) => {
+                                    error!("JoinError: {err:#}");
+                                    return;
+                                }
+                            };
                             match sock.send_to(&res, src_addr).await {
                                 Ok(len) => {
                                     if len != res.len() {
@@ -743,10 +750,11 @@ async fn server_stun_listener(sock: UdpSocket) {
                                 }
                             }
                         }
-                        Err(err) => {
+                        Ok(Err(err)) => {
                             inc!(StunMetrics, bad_requests);
                             warn!(%src_addr, "STUN: invalid binding request: {:?}", err);
                         }
+                        Err(err) => error!("JoinError parsing STUN binding: {err:#}"),
                     }
                 });
             }

iroh-net/src/defaults.rs

@@ -21,7 +21,9 @@ pub fn default_derp_map() -> DerpMap {
 /// Get the default [`DerpNode`] for NA.
 pub fn default_na_derp_node() -> DerpNode {
     // The default NA derper run by number0.
-    let url: Url = format!("https://{NA_DERP_HOSTNAME}").parse().unwrap();
+    let url: Url = format!("https://{NA_DERP_HOSTNAME}")
+        .parse()
+        .expect("default url");
     DerpNode {
         url: url.into(),
         stun_only: false,
@@ -32,7 +34,9 @@ pub fn default_na_derp_node() -> DerpNode {
 /// Get the default [`DerpNode`] for EU.
 pub fn default_eu_derp_node() -> DerpNode {
     // The default EU derper run by number0.
-    let url: Url = format!("https://{EU_DERP_HOSTNAME}").parse().unwrap();
+    let url: Url = format!("https://{EU_DERP_HOSTNAME}")
+        .parse()
+        .expect("default_url");
     DerpNode {
         url: url.into(),
         stun_only: false,

iroh-net/src/derp/client_conn.rs

@@ -428,7 +428,10 @@ where
     async fn send_packet(&mut self, packet: Packet) -> Result<()> {
         let src_key = packet.src;
         let content = packet.bytes;
-        inc_by!(Metrics, bytes_sent, content.len().try_into().unwrap());
+
+        if let Ok(len) = content.len().try_into() {
+            inc_by!(Metrics, bytes_sent, len);
+        }
         write_frame(
             &mut self.io,
             Frame::RecvPacket { src_key, content },

iroh-net/src/derp/codec.rs

@@ -506,8 +506,8 @@ impl Frame {
                     "invalid restarting frame length: {}",
                     content.len()
                 );
-                let reconnect_in = u32::from_be_bytes(content[..4].try_into().unwrap());
-                let try_for = u32::from_be_bytes(content[4..].try_into().unwrap());
+                let reconnect_in = u32::from_be_bytes(content[..4].try_into()?);
+                let try_for = u32::from_be_bytes(content[4..].try_into()?);
                 Self::Restarting {
                     reconnect_in,
                     try_for,
@@ -555,9 +555,18 @@ impl Decoder for DerpCodec {
             return Ok(None);
         }
 
-        // Can't use the `get_` Buf api, as that advances the buffer
-        let frame_type: FrameType = src[0].into();
-        let frame_len = u32::from_be_bytes(src[1..5].try_into().unwrap()) as usize;
+        // Can't use the `Buf::get_*` APIs, as that advances the buffer.
+        let Some(frame_type) = src.first().map(|b| FrameType::from(*b)) else {
+            return Ok(None); // Not enough bytes
+        };
+        let Some(frame_len) = src
+            .get(1..5)
+            .and_then(|s| TryInto::<[u8; 4]>::try_into(s).ok())
+            .map(u32::from_be_bytes)
+            .map(|l| l as usize)
+        else {
+            return Ok(None); // Not enough bytes
+        };
 
         if frame_len > MAX_FRAME_SIZE {
             anyhow::bail!("Frame of length {} is too large.", frame_len);

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

@@ -78,9 +78,12 @@ pub enum ClientError {
     /// No local addresses exist
     #[error("no local addr: {0}")]
     NoLocalAddr(String),
-    /// There was http [`hyper::Error`]
+    /// There was http server [`hyper::Error`]
     #[error("http connection error")]
     Hyper(#[from] hyper::Error),
+    /// There was an http error [`http::Error`].
+    #[error("http error")]
+    Http(#[from] http::Error),
     /// There was an unexpected status code
     #[error("unexpected status code: expected {0}, got {1}")]
     UnexpectedStatusCode(hyper::StatusCode, hyper::StatusCode),
@@ -751,7 +754,7 @@ impl Actor {
     }
 
     /// Sends the HTTP upgrade request to the derper.
-    async fn start_upgrade<T>(io: T) -> Result<hyper::Response<Incoming>, hyper::Error>
+    async fn start_upgrade<T>(io: T) -> Result<hyper::Response<Incoming>, ClientError>
     where
         T: AsyncRead + AsyncWrite + Send + Unpin + 'static,
     {
@@ -774,9 +777,8 @@ impl Actor {
         let req = Request::builder()
             .uri("/derp")
             .header(UPGRADE, super::HTTP_UPGRADE_PROTOCOL)
-            .body(http_body_util::Empty::<hyper::body::Bytes>::new())
-            .unwrap();
-        request_sender.send_request(req).await
+            .body(http_body_util::Empty::<hyper::body::Bytes>::new())?;
+        request_sender.send_request(req).await.map_err(From::from)
     }
 
     async fn note_preferred(&mut self, is_preferred: bool) {