diff --git a/src/http/client.rs b/src/http/client.rs index 4ad5269..2ec8689 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -97,12 +97,22 @@ 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 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); let discovery = if dynamic_dns_for_client(cli, url, effective_proxy) { @@ -177,8 +187,11 @@ 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 { - builder = builder.connect_timeout(timeout); + if let Some(timeout) = connect_budget.remaining()? { + 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 867056d..346211f 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, }; @@ -89,6 +90,42 @@ 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); + assert!( + res.stderr.contains("request timed out after 1s"), + "{}", + 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,