use IpAddr type for validation and add error messages

From: https://github.com/ekzhang/bore/pull/162#discussion_r2042195219

src/main.rs

@@ -1,3 +1,4 @@
+use std::net::IpAddr;
 use anyhow::Result;
 use bore_cli::{client::Client, server::Server};
 use clap::{error::ErrorKind, CommandFactory, Parser, Subcommand};
@@ -84,7 +85,24 @@ async fn run(command: Command) -> Result<()> {
                     .error(ErrorKind::InvalidValue, "port range is empty")
                     .exit();
             }
-            Server::new(port_range, secret.as_deref(), control_addr, tunnels_addr).listen().await?;
+
+            let ipaddr_control = control_addr.parse::<IpAddr>();
+            if ipaddr_control.is_err() {
+                Args::command()
+                    .error(ErrorKind::InvalidValue, "invalid ip address for control server")
+                    .exit();
+            }
+
+            let ipaddr_tunnels = tunnels_addr.parse::<IpAddr>();
+            if ipaddr_tunnels.is_err() {
+                Args::command()
+                    .error(ErrorKind::InvalidValue, "invalid ip address for tunnel connections")
+                    .exit();
+            }
+
+            Server::new(port_range, secret.as_deref(), ipaddr_control.unwrap(), ipaddr_tunnels.unwrap())
+                .listen()
+                .await?;
         }
     }
 

src/server.rs

@@ -1,6 +1,7 @@
 //! Server implementation for the `bore` service.
 
 use std::{io, ops::RangeInclusive, sync::Arc, time::Duration};
+use std::net::IpAddr;
 
 use anyhow::Result;
 use dashmap::DashMap;
@@ -25,15 +26,20 @@ pub struct Server {
     conns: Arc<DashMap<Uuid, TcpStream>>,
 
     /// IP address for the control server. Bore clients must reach this address.
-    control_addr: String,
+    control_addr: IpAddr,
 
     /// IP address where tunnels will listen on.
-    tunnels_addr: String,
+    tunnels_addr: IpAddr,
 }
 
 impl Server {
     /// Create a new server with a specified minimum port number.
-    pub fn new(port_range: RangeInclusive<u16>, secret: Option<&str>, control_addr: String, tunnels_addr: String) -> Self {
+    pub fn new(
+        port_range: RangeInclusive<u16>,
+        secret: Option<&str>,
+        control_addr: IpAddr,
+        tunnels_addr: IpAddr,
+    ) -> Self {
         assert!(!port_range.is_empty(), "must provide at least one port");
         Server {
             port_range,
@@ -47,7 +53,7 @@ impl Server {
     /// Start the server, listening for new connections.
     pub async fn listen(self) -> Result<()> {
         let this = Arc::new(self);
-        let listener = TcpListener::bind((this.control_addr.as_ref(), CONTROL_PORT)).await?;
+        let listener = TcpListener::bind((this.control_addr, CONTROL_PORT)).await?;
         info!(?this.control_addr, "server listening");
 
         loop {
@@ -69,7 +75,7 @@ impl Server {
 
     async fn create_listener(&self, port: u16) -> Result<TcpListener, &'static str> {
         let try_bind = |port: u16| async move {
-            TcpListener::bind((self.tunnels_addr.as_ref(), port))
+            TcpListener::bind((self.tunnels_addr, port))
                 .await
                 .map_err(|err| match err.kind() {
                     io::ErrorKind::AddrInUse => "port already in use",

tests/e2e_test.rs

@@ -1,6 +1,7 @@
 #![allow(clippy::items_after_test_module)]
 
 use std::net::SocketAddr;
+use std::net::IpAddr;
 use std::time::Duration;
 
 use anyhow::{anyhow, Result};
@@ -23,8 +24,8 @@ async fn spawn_server(secret: Option<&str>) {
         Server::new(
             1024..=65535,
             secret,
-            "0.0.0.0".to_string(),
+            "0.0.0.0".parse::<IpAddr>().unwrap(),
-            "0.0.0.0".to_string(),
+            "0.0.0.0".parse::<IpAddr>().unwrap(),
         )
         .listen(),
     );
@@ -136,7 +137,7 @@ fn empty_port_range() {
     let _ = Server::new(
         min_port..=max_port,
         None,
-        "0.0.0.0".to_string(),
+        "0.0.0.0".parse::<IpAddr>().unwrap(),
-        "0.0.0.0".to_string(),
+        "0.0.0.0".parse::<IpAddr>().unwrap(),
     );
 }