diff --git a/crates/maestro-cli/src/http.rs b/crates/maestro-cli/src/http.rs index e6b9f46..5bba545 100644 --- a/crates/maestro-cli/src/http.rs +++ b/crates/maestro-cli/src/http.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex}; +use std::time::Duration; use reqwest::header::{HeaderMap, HeaderValue, COOKIE, SET_COOKIE}; use reqwest::{Method, Response, StatusCode}; @@ -13,6 +14,30 @@ use crate::config::{CliConfig, ConfigError}; const COOKIE_NAME: &str = "maestro_session"; +/// Time budget for establishing a TCP+TLS connection to the registry/daemon. +const CONNECT_TIMEOUT: Duration = Duration::from_secs(5); +/// Total time budget for a single request (connect + send + receive). Bounds +/// the case where the peer is TCP-alive but never makes forward progress. +const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); + +/// Build a `reqwest::Client` with the CLI's standard timeouts and user-agent. +/// +/// Centralizing construction here guarantees every client the CLI uses gets a +/// connect *and* request timeout, so `maestro` never hangs forever on an +/// unhealthy registry or daemon (NAP-262). +pub fn build_client() -> reqwest::Client { + build_client_with_timeouts(CONNECT_TIMEOUT, REQUEST_TIMEOUT) +} + +fn build_client_with_timeouts(connect: Duration, request: Duration) -> reqwest::Client { + reqwest::Client::builder() + .user_agent(concat!("maestro-cli/", env!("CARGO_PKG_VERSION"))) + .connect_timeout(connect) + .timeout(request) + .build() + .expect("build reqwest client") +} + #[derive(Debug, thiserror::Error)] pub enum HttpError { #[error("http: {0}")] @@ -72,10 +97,7 @@ pub struct RegistryClient { impl RegistryClient { pub fn new(base: String, cookie: Option, config_path: PathBuf) -> Self { - let inner = reqwest::Client::builder() - .user_agent(concat!("maestro-cli/", env!("CARGO_PKG_VERSION"))) - .build() - .expect("build reqwest client"); + let inner = build_client(); Self { base: base.trim_end_matches('/').to_string(), inner, @@ -201,4 +223,33 @@ mod tests { let raw = "other=1; HttpOnly"; assert_eq!(extract_cookie_value(raw, "maestro_session"), None); } + + /// A peer that accepts the TCP connection but never sends a response must + /// not hang the client forever: the request timeout has to fire (NAP-262). + #[tokio::test] + async fn request_timeout_fires_when_peer_never_responds() { + use std::io::Read; + use std::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + // Accept one connection and sit on it without ever writing a response. + let handle = std::thread::spawn(move || { + if let Ok((mut sock, _)) = listener.accept() { + let mut buf = [0u8; 1024]; + let _ = sock.read(&mut buf); + std::thread::sleep(Duration::from_secs(2)); + } + }); + + let client = build_client_with_timeouts(Duration::from_secs(5), Duration::from_millis(300)); + let err = client + .get(format!("http://{addr}/")) + .send() + .await + .expect_err("request should time out, not hang"); + assert!(err.is_timeout(), "expected timeout error, got: {err}"); + + let _ = handle.join(); + } } diff --git a/crates/maestro-cli/src/policy.rs b/crates/maestro-cli/src/policy.rs index 099539c..348c5f2 100644 --- a/crates/maestro-cli/src/policy.rs +++ b/crates/maestro-cli/src/policy.rs @@ -36,7 +36,7 @@ pub struct PolicyView { pub async fn run_reload(config_path: &Path, out: &mut impl Write) -> Result<(), PolicyError> { let url = format!("{}/api/policy/reload", local_base_url(config_path)?); - let resp = reqwest::Client::new().post(&url).send().await?; + let resp = crate::http::build_client().post(&url).send().await?; let status = resp.status(); if !status.is_success() { let body = resp.text().await.unwrap_or_default(); @@ -65,7 +65,7 @@ pub async fn run_show( out: &mut impl Write, ) -> Result<(), PolicyError> { let url = format!("{}/api/policy", local_base_url(config_path)?); - let resp = reqwest::Client::new().get(&url).send().await?; + let resp = crate::http::build_client().get(&url).send().await?; let status = resp.status(); if !status.is_success() { let body = resp.text().await.unwrap_or_default();