From 706c46303c95789ae101b6722e7deb40efd15a34 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Mon, 13 Jul 2026 23:06:28 -0400 Subject: [PATCH 1/2] fix: share connect timeout with DNS resolution --- src/http/client.rs | 8 +++--- tests/network.rs | 63 ++++++++++++++++++++++++++++++++++++++++++-- tests/support/dns.rs | 30 +++++++++++++++++++++ 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/src/http/client.rs b/src/http/client.rs index 4ad5269..bc56871 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -97,12 +97,12 @@ pub(crate) async fn build_client_for_url( ) -> Result { let http_version = context.mode.http_version(); validate_client_auth_for_url(cli, url)?; - let dns_timeout = TimeoutBudget::for_connect( + let connect_budget = TimeoutBudget::for_connect( context.connect_timeout, context.request_timeout, context.request_start, - )? - .timeout(); + )?; + let dns_timeout = connect_budget.remaining()?; let effective_proxy = effective_proxy_for_url(cli.proxy.as_deref(), http_version, url)?; let auto_http3 = auto_http3_allowed(context.mode, url, cli.unix.as_deref(), effective_proxy); let discovery = if dynamic_dns_for_client(cli, url, effective_proxy) { @@ -177,7 +177,7 @@ pub(crate) async fn build_client_for_url( .unwrap_or_else(|| request_timeout_message(timeout)); builder = builder.timeout_with_message(timeout, timeout_message); } - if let Some(timeout) = context.connect_timeout { + if let Some(timeout) = connect_budget.remaining()? { builder = builder.connect_timeout(timeout); } if let Some(session) = context.session { diff --git a/tests/network.rs b/tests/network.rs index 867056d..63ac772 100644 --- a/tests/network.rs +++ b/tests/network.rs @@ -12,8 +12,9 @@ use support::common::{ }; use support::dns::{ parse_dns_question, start_udp_dns_server, start_udp_dns_server_dropping_https, - start_udp_dns_server_with_delayed_aaaa, start_udp_dns_server_with_hosts, - start_udp_dns_server_with_https, start_udp_dns_server_with_https_target_dropping_target, + start_udp_dns_server_with_delayed_aaaa, start_udp_dns_server_with_delayed_resolution, + start_udp_dns_server_with_hosts, start_udp_dns_server_with_https, + start_udp_dns_server_with_https_target_dropping_target, start_udp_dns_server_with_https_targets_dropping_targets, start_udp_dns_server_with_toggleable_https, start_unresponsive_udp_dns_server, }; @@ -54,6 +55,26 @@ fn parse_timing_duration(value: &str) -> Option { } } +fn timeout_duration(stderr: &str) -> Option { + let value = stderr + .split("request timed out after ") + .nth(1)? + .split_whitespace() + .next()? + .trim_end_matches(':'); + if let Some(milliseconds) = value.strip_suffix("ms") { + return milliseconds + .parse::() + .ok() + .map(|value| Duration::from_secs_f64(value / 1000.0)); + } + value + .strip_suffix('s')? + .parse::() + .ok() + .map(Duration::from_secs_f64) +} + #[test] fn custom_dns_connects_after_fast_a_without_waiting_for_slow_aaaa() { let server = TestServer::start(|req| { @@ -89,6 +110,44 @@ fn custom_dns_connects_after_fast_a_without_waiting_for_slow_aaaa() { assert!(res.stderr.contains("* TCP: 127.0.0.1:")); } +#[test] +fn connect_timeout_is_shared_between_preresolved_dns_and_tls() { + let tls = start_h2_tls_server_with_accept_delay( + |_| TestResponse::ok("too late"), + Duration::from_millis(700), + ); + let port = Url::parse(&tls.url).unwrap().port().unwrap(); + let dns_addr = start_udp_dns_server_with_delayed_resolution( + "localhost.", + Ipv4Addr::new(127, 0, 0, 1), + Duration::from_millis(700), + ); + + let res = run_fetch(&[ + "--dns-server", + &dns_addr, + "--ca-cert", + tls.ca_cert_path.to_str().unwrap(), + "--http", + "2", + // ECH discovery makes the client pre-resolve the origin. + "--ech", + "auto", + "--connect-timeout", + "1", + &format!("https://localhost:{port}/shared-connect-timeout"), + ]); + + assert_exit(&res, 1); + let timeout = timeout_duration(&res.stderr) + .unwrap_or_else(|| panic!("missing timeout diagnostic\nstderr:\n{}", res.stderr)); + assert!( + timeout < Duration::from_millis(500), + "TLS received the full connect timeout after DNS used most of it: {timeout:?}\nstderr:\n{}", + res.stderr + ); +} + #[test] fn proxy_config_environment_and_curl_http1_cases() { let proxy = TestServer::start(|req| { diff --git a/tests/support/dns.rs b/tests/support/dns.rs index 3bf9ff1..fc7938b 100644 --- a/tests/support/dns.rs +++ b/tests/support/dns.rs @@ -160,6 +160,36 @@ pub(crate) fn start_udp_dns_server_dropping_https(host: &'static str, ip: Ipv4Ad addr } +pub(crate) fn start_udp_dns_server_with_delayed_resolution( + host: &'static str, + ip: Ipv4Addr, + delay: Duration, +) -> String { + let socket = UdpSocket::bind("127.0.0.1:0").expect("bind udp dns server"); + let addr = socket.local_addr().unwrap().to_string(); + thread::spawn(move || { + let mut buf = [0_u8; 512]; + while let Ok((n, peer)) = socket.recv_from(&mut buf) { + let Some((name, qtype, question_end)) = parse_dns_question(&buf[..n]) else { + continue; + }; + let answer = + (name == host && qtype == TYPE_A).then_some((TYPE_A, ip.octets().to_vec())); + let response = dns_response(&buf[..n], question_end, answer); + if name == host && matches!(qtype, TYPE_A | TYPE_AAAA) { + let socket = socket.try_clone().expect("clone udp dns server socket"); + thread::spawn(move || { + thread::sleep(delay); + let _ = socket.send_to(&response, peer); + }); + } else { + let _ = socket.send_to(&response, peer); + } + } + }); + addr +} + pub(crate) fn start_udp_dns_server_with_delayed_aaaa( host: &'static str, ip: Ipv4Addr, From a9ca2a9341723d6bdd39ee82176017605a6fcae0 Mon Sep 17 00:00:00 2001 From: Ryan Fowler Date: Mon, 13 Jul 2026 23:13:43 -0400 Subject: [PATCH 2/2] fix: preserve connect timeout diagnostics --- src/http/client.rs | 15 ++++++++++++++- src/http/transport/client.rs | 37 ++++++++++++++++++++++++++++-------- tests/network.rs | 26 ++----------------------- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/http/client.rs b/src/http/client.rs index bc56871..2ec8689 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -102,6 +102,16 @@ pub(crate) async fn build_client_for_url( context.request_timeout, context.request_start, )?; + let connect_timeout_message = connect_budget.timeout().map(|timeout| { + if context.connect_timeout == Some(timeout) { + request_timeout_message(timeout) + } else { + context + .request_timeout + .map(request_timeout_message) + .unwrap_or_else(|| request_timeout_message(timeout)) + } + }); let dns_timeout = connect_budget.remaining()?; let effective_proxy = effective_proxy_for_url(cli.proxy.as_deref(), http_version, url)?; let auto_http3 = auto_http3_allowed(context.mode, url, cli.unix.as_deref(), effective_proxy); @@ -178,7 +188,10 @@ pub(crate) async fn build_client_for_url( builder = builder.timeout_with_message(timeout, timeout_message); } if let Some(timeout) = connect_budget.remaining()? { - builder = builder.connect_timeout(timeout); + builder = builder.connect_timeout_with_message( + timeout, + connect_timeout_message.expect("finite connect budget has timeout message"), + ); } if let Some(session) = context.session { builder = builder.cookie_provider(session.cookie_provider()); diff --git a/src/http/transport/client.rs b/src/http/transport/client.rs index 5c307f3..b880496 100644 --- a/src/http/transport/client.rs +++ b/src/http/transport/client.rs @@ -60,6 +60,7 @@ pub(super) struct ClientConfig { pub(super) request_timeout: Option, pub(super) request_timeout_message: Option, pub(super) connect_timeout: Option, + pub(super) connect_timeout_message: Option, pub(super) session: Option>, pub(super) connection_timing: Option, pub(super) dns_resolution: Option, @@ -95,6 +96,7 @@ impl Client { request_timeout: None, request_timeout_message: None, connect_timeout: None, + connect_timeout_message: None, session: None, connection_timing: None, dns_resolution: None, @@ -146,7 +148,7 @@ impl Client { .unwrap_or_else(|| request_timeout_message(timeout)), ) }); - timeout + let result = timeout .run(self.send( request.method, request.url, @@ -156,7 +158,15 @@ impl Client { body_deadline, )) .await - .map_err(|err| Error::from_fetch(ErrorKind::Request, err))? + .map_err(|err| Error::from_fetch(ErrorKind::Request, err))?; + result.map_err(|err| { + if err.is_timeout() + && let Some(message) = &self.config.connect_timeout_message + { + return Error::timeout(message.clone()); + } + err + }) } async fn send( @@ -486,8 +496,17 @@ impl ClientBuilder { self } - pub(crate) fn connect_timeout(mut self, timeout: Duration) -> Self { + pub(crate) fn connect_timeout(self, timeout: Duration) -> Self { + self.connect_timeout_with_message(timeout, request_timeout_message(timeout)) + } + + pub(crate) fn connect_timeout_with_message( + mut self, + timeout: Duration, + timeout_message: impl Into, + ) -> Self { self.config.connect_timeout = Some(timeout); + self.config.connect_timeout_message = Some(timeout_message.into()); self } @@ -720,15 +739,17 @@ pub(super) async fn tls_stream_for_config( } fn map_pooled_client_error(err: hyper_util::client::legacy::Error) -> Error { - let kind = if err.is_connect() { - ErrorKind::Connect - } else { - ErrorKind::Request - }; let message = err .source() .map(ToString::to_string) .unwrap_or_else(|| err.to_string()); + let kind = if message.starts_with("request timed out after ") { + ErrorKind::Timeout + } else if err.is_connect() { + ErrorKind::Connect + } else { + ErrorKind::Request + }; Error::with_source(kind, message, err) } diff --git a/tests/network.rs b/tests/network.rs index 63ac772..346211f 100644 --- a/tests/network.rs +++ b/tests/network.rs @@ -55,26 +55,6 @@ fn parse_timing_duration(value: &str) -> Option { } } -fn timeout_duration(stderr: &str) -> Option { - let value = stderr - .split("request timed out after ") - .nth(1)? - .split_whitespace() - .next()? - .trim_end_matches(':'); - if let Some(milliseconds) = value.strip_suffix("ms") { - return milliseconds - .parse::() - .ok() - .map(|value| Duration::from_secs_f64(value / 1000.0)); - } - value - .strip_suffix('s')? - .parse::() - .ok() - .map(Duration::from_secs_f64) -} - #[test] fn custom_dns_connects_after_fast_a_without_waiting_for_slow_aaaa() { let server = TestServer::start(|req| { @@ -139,11 +119,9 @@ fn connect_timeout_is_shared_between_preresolved_dns_and_tls() { ]); assert_exit(&res, 1); - let timeout = timeout_duration(&res.stderr) - .unwrap_or_else(|| panic!("missing timeout diagnostic\nstderr:\n{}", res.stderr)); assert!( - timeout < Duration::from_millis(500), - "TLS received the full connect timeout after DNS used most of it: {timeout:?}\nstderr:\n{}", + res.stderr.contains("request timed out after 1s"), + "{}", res.stderr ); }