From fdb1297442598b41971f7212175e8e2f12ea6833 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 8 Apr 2026 15:43:09 -0400 Subject: [PATCH] feat(server): support host-only tcp binds Accept `--bind ` as a TCP bind request while keeping the default Unix socket behavior unchanged. Resolve host-only TCP binds inside the serving process so startup output, server metadata, and status always reflect the concrete host:port, preferring 32276 and falling back to a random port with a warning when needed. --- docs/administration/server-configuration.mdx | 2 +- docs/reference/cli.mdx | 3 +- .../src/commands/server/foreground.rs | 34 +++- .../fabro-cli/src/commands/server/mod.rs | 6 +- .../fabro-cli/src/commands/server/start.rs | 94 +++++----- .../fabro-cli/tests/it/cmd/server_start.rs | 103 ++++++++++- lib/crates/fabro-server/src/bind.rs | 53 ++++-- lib/crates/fabro-server/src/serve.rs | 161 +++++++++++++++--- 8 files changed, 376 insertions(+), 80 deletions(-) diff --git a/docs/administration/server-configuration.mdx b/docs/administration/server-configuration.mdx index 609ccca8a..418acdcb9 100644 --- a/docs/administration/server-configuration.mdx +++ b/docs/administration/server-configuration.mdx @@ -102,7 +102,7 @@ Several `settings.toml` settings can be overridden via `fabro server start` flag | Flag | Default | Description | |---|---|---| -| `--bind` | `~/.fabro/fabro.sock` | Address to bind: `host:port` for TCP, or a path for Unix socket | +| `--bind` | `~/.fabro/fabro.sock` | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket | | `--foreground` | — | Run in the foreground instead of daemonizing | | `--model` | — | Override default LLM model | | `--provider` | — | Override default LLM provider | diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 04974f067..bb1c1531a 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -326,6 +326,7 @@ Start the Fabro server daemon. By default, the server launches as a background p ```bash fabro server start # background daemon on Unix socket +fabro server start --bind 127.0.0.1 # TCP on 32276, or random port if 32276 is busy fabro server start --bind 127.0.0.1:8080 # TCP on a specific port fabro server start --foreground # blocking foreground mode fabro server start --sandbox daytona --max-concurrent-runs 4 @@ -333,7 +334,7 @@ fabro server start --sandbox daytona --max-concurrent-runs 4 | Flag | Description | Default | |---|---|---| -| `--bind ` | Address to bind: `host:port` for TCP, or a path for Unix socket | `~/.fabro/fabro.sock` | +| `--bind ` | Address to bind: `IP` or `IP:port` for TCP, or a path for Unix socket | `~/.fabro/fabro.sock` | | `--foreground` | Run in the foreground instead of daemonizing | — | | `--model ` | Override default LLM model | — | | `--provider ` | Override default LLM provider | — | diff --git a/lib/crates/fabro-cli/src/commands/server/foreground.rs b/lib/crates/fabro-cli/src/commands/server/foreground.rs index a19941d95..45a703c80 100644 --- a/lib/crates/fabro-cli/src/commands/server/foreground.rs +++ b/lib/crates/fabro-cli/src/commands/server/foreground.rs @@ -1,7 +1,9 @@ use std::path::PathBuf; use anyhow::Result; -use fabro_server::bind::Bind; +use chrono::Utc; +use fabro_config::Storage; +use fabro_server::bind::BindRequest; use fabro_server::serve; use fabro_server::serve::ServeArgs; use fabro_util::terminal::Styles; @@ -11,17 +13,17 @@ use super::record; pub(crate) async fn execute( record_path: PathBuf, mut serve_args: ServeArgs, - bind: Bind, + bind: BindRequest, storage_dir: Option, styles: &'static Styles, ) -> Result<()> { serve_args.bind = Some(bind.to_string()); - let _record_guard = scopeguard::guard(record_path, |path| { + let _record_guard = scopeguard::guard(record_path.clone(), |path| { record::remove_server_record(&path); }); - let _socket_guard = if let Bind::Unix(ref path) = bind { + let _socket_guard = if let BindRequest::Unix(ref path) = bind { let path = path.clone(); Some(scopeguard::guard(path, |p| { let _ = std::fs::remove_file(p); @@ -30,5 +32,27 @@ pub(crate) async fn execute( None }; - serve::serve_command(serve_args, styles, storage_dir).await + let log_path = storage_dir + .as_ref() + .map(|dir| Storage::new(dir).server_state().log_path()) + .unwrap_or_else(|| { + record_path + .parent() + .map(|parent| parent.join("server.log")) + .unwrap_or_else(|| PathBuf::from("server.log")) + }); + let pid = std::process::id(); + + serve::serve_command(serve_args, styles, storage_dir, move |resolved_bind| { + record::write_server_record( + &record_path, + &record::ServerRecord { + pid, + bind: resolved_bind.clone(), + log_path: log_path.clone(), + started_at: Utc::now(), + }, + ) + }) + .await } diff --git a/lib/crates/fabro-cli/src/commands/server/mod.rs b/lib/crates/fabro-cli/src/commands/server/mod.rs index b82e42e85..7bebd3aac 100644 --- a/lib/crates/fabro-cli/src/commands/server/mod.rs +++ b/lib/crates/fabro-cli/src/commands/server/mod.rs @@ -8,7 +8,7 @@ use std::time::Duration; use anyhow::Result; use fabro_server::bind; -use fabro_server::bind::Bind; +use fabro_server::bind::BindRequest; use fabro_server::serve::ServeArgs; use fabro_util::terminal::Styles; @@ -31,7 +31,7 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R let storage_dir = settings.storage_dir(); let bind_addr = match serve_args.bind.as_deref() { Some(s) => bind::parse_bind(s)?, - None => Bind::Unix(user_config::default_socket_path()), + None => BindRequest::Unix(user_config::default_socket_path()), }; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); start::execute(bind_addr, foreground, serve_args, storage_dir, styles).await @@ -66,7 +66,7 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R } else { // __serve should always receive an explicit --bind from the parent, // but fall back to the storage dir default if missing. - Bind::Unix(user_config::default_socket_path()) + BindRequest::Unix(user_config::default_socket_path()) }; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); foreground::execute( diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index 082bba25b..4613eafdf 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -6,15 +6,15 @@ use anyhow::{Result, bail}; use chrono::Utc; use fabro_config::Storage; use fabro_config::user::default_socket_path; -use fabro_server::bind::Bind; +use fabro_server::bind::{Bind, BindRequest}; use fabro_server::serve; -use fabro_server::serve::ServeArgs; +use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs}; use fabro_util::terminal::Styles; use super::record; pub(crate) async fn execute( - bind: Bind, + bind: BindRequest, foreground: bool, mut serve_args: ServeArgs, storage_dir: PathBuf, @@ -77,7 +77,12 @@ fn ensure_server_running_with_bind( config: Some(config_path.to_path_buf()), }; - match execute_daemon(&bind, &serve_args, storage_dir, false) { + let bind_request = match &bind { + Bind::Unix(path) => BindRequest::Unix(path.clone()), + Bind::Tcp(addr) => BindRequest::Tcp(*addr), + }; + + match execute_daemon(&bind_request, &serve_args, storage_dir, false) { Ok(()) => Ok(bind), Err(err) => { if let Some(existing) = record::active_server_record(storage_dir) { @@ -101,7 +106,7 @@ fn server_max_concurrent_runs_override() -> Option { // --------------------------------------------------------------------------- async fn execute_foreground( - bind: Bind, + bind: BindRequest, serve_args: ServeArgs, storage_dir: PathBuf, styles: &'static Styles, @@ -119,21 +124,14 @@ async fn execute_foreground( let server_state = Storage::new(&storage_dir).server_state(); let record_path = server_state.record_path(); - record::write_server_record( - &record_path, - &record::ServerRecord { - pid: std::process::id(), - bind: bind.clone(), - log_path: server_state.log_path(), - started_at: Utc::now(), - }, - )?; + let log_path = server_state.log_path(); + let pid = std::process::id(); - let _record_guard = scopeguard::guard(record_path, |path| { + let _record_guard = scopeguard::guard(record_path.clone(), |path| { record::remove_server_record(&path); }); - let _socket_guard = if let Bind::Unix(ref path) = bind { + let _socket_guard = if let BindRequest::Unix(ref path) = bind { let path = path.clone(); Some(scopeguard::guard(path, |p| { let _ = std::fs::remove_file(p); @@ -142,7 +140,23 @@ async fn execute_foreground( None }; - serve::serve_command(serve_args, styles, Some(storage_dir)).await + serve::serve_command( + serve_args, + styles, + Some(storage_dir), + move |resolved_bind| { + record::write_server_record( + &record_path, + &record::ServerRecord { + pid, + bind: resolved_bind.clone(), + log_path: log_path.clone(), + started_at: Utc::now(), + }, + ) + }, + ) + .await } // --------------------------------------------------------------------------- @@ -150,7 +164,7 @@ async fn execute_foreground( // --------------------------------------------------------------------------- fn execute_daemon( - bind: &Bind, + bind: &BindRequest, serve_args: &ServeArgs, storage_dir: &Path, announce: bool, @@ -207,7 +221,7 @@ fn execute_daemon( } cmd.arg("--storage-dir").arg(storage_dir); - if matches!(bind, Bind::Unix(_)) { + if matches!(bind, BindRequest::Unix(_)) { cmd.env("FABRO_LOCAL_NO_AUTH", "1"); } @@ -221,16 +235,6 @@ fn execute_daemon( let mut child = cmd.spawn()?; - record::write_server_record( - &record_path, - &record::ServerRecord { - pid: child.id(), - bind: bind.clone(), - log_path: log_path.clone(), - started_at: Utc::now(), - }, - )?; - if let Ok(Some(status)) = child.try_wait() { record::remove_server_record(&record_path); let tail = read_log_tail(&log_path, 20); @@ -245,18 +249,18 @@ fn execute_daemon( let mut elapsed = Duration::ZERO; while elapsed < timeout { - if try_connect(bind) { - if announce { - eprintln!("Server started (pid {}) on {bind}", child.id()); + if let Some(record) = record::read_server_record(&record_path) { + if try_connect(&record.bind) { + if announce { + maybe_warn_host_port_fallback(bind, &record.bind); + eprintln!("Server started (pid {}) on {}", child.id(), record.bind); + } + return Ok(()); } - return Ok(()); } if let Ok(Some(status)) = child.try_wait() { record::remove_server_record(&record_path); - if let Bind::Unix(ref path) = *bind { - let _ = std::fs::remove_file(path); - } let tail = read_log_tail(&log_path, 20); if !tail.is_empty() { eprintln!("{tail}"); @@ -269,9 +273,6 @@ fn execute_daemon( } record::remove_server_record(&record_path); - if let Bind::Unix(ref path) = *bind { - let _ = std::fs::remove_file(path); - } let _ = child.kill(); let _ = child.wait(); let tail = read_log_tail(&log_path, 20); @@ -320,6 +321,21 @@ fn try_connect(bind: &Bind) -> bool { } } +fn maybe_warn_host_port_fallback(requested: &BindRequest, resolved: &Bind) { + let BindRequest::TcpHost(host) = requested else { + return; + }; + let Bind::Tcp(addr) = resolved else { + return; + }; + if addr.ip() == *host && addr.port() != DEFAULT_TCP_PORT { + eprintln!( + "Warning: TCP port {} is unavailable on {}; falling back to a random port.", + DEFAULT_TCP_PORT, host + ); + } +} + fn read_log_tail(log_path: &Path, lines: usize) -> String { match std::fs::read_to_string(log_path) { Ok(content) => { diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs index d93eec69b..5c937e426 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -32,7 +32,7 @@ fn help() { --foreground Run in the foreground instead of daemonizing --bind - Address to bind to (host:port for TCP, or path containing / for Unix socket) + Address to bind to (IP or IP:port for TCP, or path containing / for Unix socket) --no-upgrade-check Disable automatic upgrade check [env: FABRO_NO_UPGRADE_CHECK=true] --model @@ -133,6 +133,107 @@ fn start_without_bind_uses_home_socket_instead_of_storage_socket() { .success(); } +#[test] +fn start_with_tcp_host_only_bind_resolves_to_host_and_port() { + let context = test_context!(); + let storage_root = isolated_storage_dir(); + let storage_dir = storage_root.path().join("storage"); + + let mut cmd = context.command(); + cmd.env("FABRO_STORAGE_DIR", &storage_dir); + cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); + let output = cmd.output().expect("server start command should run"); + assert!( + output.status.success(), + "server start should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Server started (pid "), + "expected startup message, got {stderr}" + ); + let bind_regex = regex::Regex::new(r"127\.0\.0\.1:\d+").unwrap(); + assert!( + bind_regex.is_match(&stderr), + "expected resolved tcp bind in stderr, got {stderr}" + ); + + let output = context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "status", "--json"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let json: serde_json::Value = serde_json::from_slice(&output).unwrap(); + let bind = json["bind"].as_str().expect("bind should be a string"); + assert!( + bind.starts_with("127.0.0.1:"), + "expected resolved tcp bind, got {bind}" + ); + + context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "stop"]) + .assert() + .success(); +} + +#[test] +fn start_with_tcp_host_only_bind_warns_and_falls_back_when_default_port_is_unavailable() { + let context = test_context!(); + let storage_root = isolated_storage_dir(); + let storage_dir = storage_root.path().join("storage"); + let occupied = std::net::TcpListener::bind(("127.0.0.1", 32276)) + .expect("test requires default TCP port 32276 to be free before occupying it"); + + let mut filters = context.filters(); + filters.push((r"pid \d+".to_string(), "pid [PID]".to_string())); + filters.push((r"127\.0\.0\.1:\d+".to_string(), "[TCP_BIND]".to_string())); + + let mut cmd = context.command(); + cmd.env("FABRO_STORAGE_DIR", &storage_dir); + cmd.args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]); + fabro_snapshot!(filters, cmd, @" + success: true + exit_code: 0 + ----- stdout ----- + ----- stderr ----- + Warning: TCP port 32276 is unavailable on 127.0.0.1; falling back to a random port. + Server started (pid [PID]) on [TCP_BIND] + "); + + let output = context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "status", "--json"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let json: serde_json::Value = serde_json::from_slice(&output).unwrap(); + let bind = json["bind"].as_str().expect("bind should be a string"); + assert_ne!(bind, "127.0.0.1:32276"); + assert!( + bind.starts_with("127.0.0.1:"), + "expected resolved tcp bind, got {bind}" + ); + + drop(occupied); + + context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "stop"]) + .assert() + .success(); +} + #[test] fn default_test_contexts_share_one_eager_session_server() { let context_a = test_context!(); diff --git a/lib/crates/fabro-server/src/bind.rs b/lib/crates/fabro-server/src/bind.rs index ac417c18f..8bea16506 100644 --- a/lib/crates/fabro-server/src/bind.rs +++ b/lib/crates/fabro-server/src/bind.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::path::PathBuf; use serde::{Deserialize, Serialize}; @@ -11,6 +11,13 @@ pub enum Bind { Tcp(SocketAddr), } +#[derive(Debug, Clone, PartialEq)] +pub enum BindRequest { + Unix(PathBuf), + Tcp(SocketAddr), + TcpHost(IpAddr), +} + impl fmt::Display for Bind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -20,25 +27,38 @@ impl fmt::Display for Bind { } } +impl fmt::Display for BindRequest { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Unix(path) => write!(f, "{}", path.display()), + Self::Tcp(addr) => write!(f, "{addr}"), + Self::TcpHost(host) => write!(f, "{host}"), + } + } +} + /// Parse a bind address string into a `Bind` value. /// /// If the string contains `/`, it is treated as a Unix socket path. Otherwise -/// it is parsed as a TCP `host:port` address. +/// it is parsed as either a TCP `ip:port` address or a host-only TCP IP. /// /// # Errors /// /// Returns an error if the TCP address cannot be parsed, or if a Unix socket /// path exceeds the OS limit (104 bytes on macOS, 108 on Linux). -pub fn parse_bind(s: &str) -> anyhow::Result { +pub fn parse_bind(s: &str) -> anyhow::Result { if s.contains('/') { let path = PathBuf::from(s); validate_unix_path_length(&path)?; - Ok(Bind::Unix(path)) + Ok(BindRequest::Unix(path)) + } else if let Ok(addr) = s.parse::() { + Ok(BindRequest::Tcp(addr)) + } else if let Ok(host) = s.parse::() { + Ok(BindRequest::TcpHost(host)) } else { - let addr: SocketAddr = s - .parse() - .map_err(|e| anyhow::anyhow!("invalid TCP address '{s}': {e}"))?; - Ok(Bind::Tcp(addr)) + Err(anyhow::anyhow!( + "invalid TCP address '{s}': invalid socket address syntax" + )) } } @@ -61,17 +81,24 @@ fn validate_unix_path_length(path: &std::path::Path) -> anyhow::Result<()> { #[cfg(test)] mod tests { use super::*; + use std::net::{IpAddr, Ipv4Addr}; #[test] fn parse_tcp_address() { let bind = parse_bind("127.0.0.1:3000").unwrap(); - assert_eq!(bind, Bind::Tcp("127.0.0.1:3000".parse().unwrap())); + assert_eq!(bind, BindRequest::Tcp("127.0.0.1:3000".parse().unwrap())); + } + + #[test] + fn parse_tcp_host_without_port() { + let bind = parse_bind("127.0.0.1").unwrap(); + assert_eq!(bind, BindRequest::TcpHost(IpAddr::V4(Ipv4Addr::LOCALHOST))); } #[test] fn parse_unix_socket_path() { let bind = parse_bind("/tmp/fabro.sock").unwrap(); - assert_eq!(bind, Bind::Unix(PathBuf::from("/tmp/fabro.sock"))); + assert_eq!(bind, BindRequest::Unix(PathBuf::from("/tmp/fabro.sock"))); } #[test] @@ -109,4 +136,10 @@ mod tests { let bind = Bind::Unix(PathBuf::from("/run/fabro.sock")); assert_eq!(bind.to_string(), "/run/fabro.sock"); } + + #[test] + fn display_tcp_host_request() { + let bind = BindRequest::TcpHost(IpAddr::V4(Ipv4Addr::LOCALHOST)); + assert_eq!(bind.to_string(), "127.0.0.1"); + } } diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 6c632eea7..f227f2e8a 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -19,7 +19,7 @@ use clap::Args; use fabro_types::Settings; -use crate::bind::{self, Bind}; +use crate::bind::{self, Bind, BindRequest}; use crate::github_webhooks::WebhookManager; use crate::jwt_auth::{AuthMode, AuthStrategy, resolve_auth_mode_with_lookup}; use crate::secret_store::SecretStore; @@ -32,6 +32,7 @@ use fabro_llm::client::Client as LlmClient; use fabro_sandbox::SandboxProvider; const TEST_IN_MEMORY_STORE_ENV: &str = "FABRO_TEST_IN_MEMORY_STORE"; +pub const DEFAULT_TCP_PORT: u16 = 32276; #[derive(Clone, Copy)] enum ServerTitlePhase { @@ -42,7 +43,7 @@ enum ServerTitlePhase { #[derive(Args, Clone)] pub struct ServeArgs { - /// Address to bind to (host:port for TCP, or path containing / for Unix socket) + /// Address to bind to (IP or IP:port for TCP, or path containing / for Unix socket) #[arg(long)] pub bind: Option, @@ -173,11 +174,15 @@ fn build_artifact_object_store( /// /// Returns an error if the server fails to bind or encounters a fatal error. #[allow(clippy::print_stderr)] -pub async fn serve_command( +pub async fn serve_command( args: ServeArgs, styles: &'static Styles, storage_dir_override: Option, -) -> anyhow::Result<()> { + mut on_ready: F, +) -> anyhow::Result<()> +where + F: FnMut(&Bind) -> anyhow::Result<()>, +{ let _ = fabro_proc::title_init(); set_server_title(ServerTitlePhase::Boot, None); @@ -278,9 +283,9 @@ pub async fn serve_command( spawn_scheduler(Arc::clone(&state)); let router = build_router(Arc::clone(&state), auth_mode); - let bind_addr = match args.bind { + let bind_request = match args.bind { Some(ref s) => bind::parse_bind(s)?, - None => Bind::Tcp("127.0.0.1:3000".parse().unwrap()), + None => BindRequest::Tcp("127.0.0.1:3000".parse().unwrap()), }; // Optionally start webhook listener @@ -385,26 +390,37 @@ pub async fn serve_command( .as_ref() .and_then(|a| a.tls.clone()); - match &bind_addr { - Bind::Unix(path) => { + let bound_listener = bind_listener(&bind_request).await?; + let bind_addr = bound_listener.bind.clone(); + if bound_listener.used_random_port_fallback { + if let BindRequest::TcpHost(host) = bind_request { + warn!( + host = %host, + preferred_port = DEFAULT_TCP_PORT, + "Preferred TCP port unavailable; falling back to a random port" + ); + eprintln!( + "{} TCP port {} is unavailable on {}; falling back to a random port.", + styles.yellow.apply_to("Warning:"), + DEFAULT_TCP_PORT, + host + ); + } + } + + on_ready(&bind_addr)?; + + match bound_listener.listener { + BoundListener::Unix(listener) => { if tls_settings.is_some() { warn!("TLS is configured but not supported on Unix sockets; ignoring TLS settings"); } - - // Remove stale socket file before binding - if path.exists() { - std::fs::remove_file(path)?; - } - - let listener = UnixListener::bind(path)?; announce_server_ready(&bind_addr, styles, dry_run_mode); axum::serve(listener, router) .with_graceful_shutdown(wait_for_shutdown(shutdown_rx.clone())) .await?; } - Bind::Tcp(addr) => { - let listener = TcpListener::bind(addr).await?; - + BoundListener::Tcp(listener) => { if let Some(ref tls_settings) = tls_settings { let client_auth = client_auth.unwrap(); let rustls_config = build_rustls_config(tls_settings, client_auth); @@ -437,6 +453,71 @@ pub async fn serve_command( Ok(()) } +struct BoundServerListener { + listener: BoundListener, + bind: Bind, + used_random_port_fallback: bool, +} + +enum BoundListener { + Unix(UnixListener), + Tcp(TcpListener), +} + +async fn bind_listener(requested: &BindRequest) -> anyhow::Result { + match requested { + BindRequest::Unix(path) => { + if path.exists() { + std::fs::remove_file(path)?; + } + + let listener = UnixListener::bind(path)?; + Ok(BoundServerListener { + listener: BoundListener::Unix(listener), + bind: Bind::Unix(path.clone()), + used_random_port_fallback: false, + }) + } + BindRequest::Tcp(addr) => { + let listener = TcpListener::bind(addr).await?; + let resolved = listener.local_addr()?; + Ok(BoundServerListener { + listener: BoundListener::Tcp(listener), + bind: Bind::Tcp(resolved), + used_random_port_fallback: false, + }) + } + BindRequest::TcpHost(host) => bind_tcp_host_with_fallback(*host, DEFAULT_TCP_PORT).await, + } +} + +async fn bind_tcp_host_with_fallback( + host: std::net::IpAddr, + preferred_port: u16, +) -> anyhow::Result { + let preferred = std::net::SocketAddr::new(host, preferred_port); + match TcpListener::bind(preferred).await { + Ok(listener) => { + let resolved = listener.local_addr()?; + Ok(BoundServerListener { + listener: BoundListener::Tcp(listener), + bind: Bind::Tcp(resolved), + used_random_port_fallback: false, + }) + } + Err(err) if err.kind() == std::io::ErrorKind::AddrInUse => { + let listener = TcpListener::bind(std::net::SocketAddr::new(host, 0)).await?; + let resolved = listener.local_addr()?; + Ok(BoundServerListener { + listener: BoundListener::Tcp(listener), + bind: Bind::Tcp(resolved), + used_random_port_fallback: true, + }) + } + Err(err) => Err(err.into()), + } +} + async fn shutdown_signal() { use tokio::signal; @@ -535,8 +616,8 @@ mod tests { use std::path::PathBuf; use super::{ - ServeArgs, ServerTitlePhase, apply_runtime_settings, build_object_store_with_preference, - server_bind_title, server_title, + ServeArgs, ServerTitlePhase, apply_runtime_settings, bind_tcp_host_with_fallback, + build_object_store_with_preference, server_bind_title, server_title, }; use crate::bind::Bind; use fabro_types::Settings; @@ -607,4 +688,44 @@ mod tests { ); drop(mem_store); } + + #[tokio::test] + async fn tcp_host_request_uses_preferred_port_when_available() { + let preferred = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = preferred.local_addr().unwrap().port(); + drop(preferred); + + let bound = bind_tcp_host_with_fallback("127.0.0.1".parse().unwrap(), port) + .await + .unwrap(); + let resolved = match bound.bind { + Bind::Tcp(addr) => addr, + Bind::Unix(_) => panic!("expected tcp bind"), + }; + assert_eq!( + resolved, + std::net::SocketAddr::new("127.0.0.1".parse().unwrap(), port) + ); + assert!( + !bound.used_random_port_fallback, + "preferred port should be used when available" + ); + } + + #[tokio::test] + async fn tcp_host_request_falls_back_when_preferred_port_is_occupied() { + let occupied = std::net::TcpListener::bind(("127.0.0.1", 0)).unwrap(); + let occupied_port = occupied.local_addr().unwrap().port(); + let bound = bind_tcp_host_with_fallback("127.0.0.1".parse().unwrap(), occupied_port) + .await + .unwrap(); + + let resolved = match bound.bind { + Bind::Tcp(addr) => addr, + Bind::Unix(_) => panic!("expected tcp bind"), + }; + + assert_ne!(resolved.port(), occupied_port); + assert!(bound.used_random_port_fallback); + } }