diff --git a/src/http/client.rs b/src/http/client.rs index baa9237c..e9848b42 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -282,15 +282,10 @@ async fn resolve_dns_for_client_inner( // through a proxy; auto_http3_allowed already blocks that path. if effective_proxy.is_some_and(|proxy| !proxy.uses_local_target_dns()) { if need_ech_svcb { - let ech_timeout = timeout - .remaining() - .ok() - .flatten() - .unwrap_or(Duration::from_secs(5)); let https_records = if let Some(dns_server) = cli.dns_server.as_deref() { - lookup_ech_https_records(cli, Some(dns_server), host, ech_timeout).await? + lookup_ech_https_records(cli, Some(dns_server), host, timeout).await? } else { - lookup_ech_https_records(cli, None, host, ech_timeout).await? + lookup_ech_https_records(cli, None, host, timeout).await? }; return Ok(ClientDnsDiscovery { dns_resolution: None, @@ -321,14 +316,9 @@ async fn resolve_dns_for_client_inner( let (addrs, https_records) = if need_ech_svcb { // ECH requires HTTPS records; don't use the abort-early auto-H3 // pattern which may discard them before the SVCB query finishes. - let ech_timeout = timeout - .remaining() - .ok() - .flatten() - .unwrap_or(Duration::from_secs(5)); let (addrs, https_records) = tokio::join!( lookup_custom_ips_with_doh_tls(cli, dns_server, host, timeout), - lookup_ech_https_records(cli, Some(dns_server), host, ech_timeout), + lookup_ech_https_records(cli, Some(dns_server), host, timeout), ); (addrs?, https_records?) } else if let Some(auto_http3_budget) = auto_http3_discovery { @@ -403,15 +393,8 @@ async fn resolve_dns_for_client_inner( let (socket_addrs, https_records) = if need_ech_svcb { // ECH requires HTTPS records; await the SVCB query properly instead // of using the abort-early auto-H3 pattern. - let ech_timeout = timeout - .remaining() - .ok() - .flatten() - .unwrap_or(Duration::from_secs(5)); - let (socket_addrs, https_records) = tokio::join!( - lookup, - lookup_ech_https_records(cli, None, host, ech_timeout), - ); + let (socket_addrs, https_records) = + tokio::join!(lookup, lookup_ech_https_records(cli, None, host, timeout),); ( socket_addrs .map_err(|err| FetchError::Runtime(format!("lookup {host}: {err}")))? @@ -531,16 +514,17 @@ async fn lookup_ech_https_records( cli: &Cli, dns_server: Option<&str>, host: &str, - timeout: Duration, + timeout: TimeoutBudget, ) -> Result, FetchError> { + let ech_timeout = timeout.remaining()?.unwrap_or(Duration::from_secs(5)); let resolver = dns_server .map(HttpsRecordResolver::Custom) .unwrap_or(HttpsRecordResolver::System); - match TimeoutBudget::new(Some(timeout)) + match TimeoutBudget::new(Some(ech_timeout)) .run(crate::dns::svcb::lookup_https_records_with_doh_tls_config( resolver, host, - Some(timeout), + Some(ech_timeout), doh_tls_config_for_cli(cli)?, )) .await @@ -822,29 +806,7 @@ pub(crate) async fn resolve_websocket_ech_mode( if host.parse::().is_ok() { return Ok(None); } - let ech_timeout = timeout - .remaining() - .ok() - .flatten() - .unwrap_or(Duration::from_secs(5)); - let resolver = cli - .dns_server - .as_deref() - .map(crate::dns::svcb::HttpsRecordResolver::Custom) - .unwrap_or(crate::dns::svcb::HttpsRecordResolver::System); - let records = tokio::time::timeout( - ech_timeout, - crate::dns::svcb::lookup_https_records_with_doh_tls_config( - resolver, - host, - Some(ech_timeout), - doh_tls_config_for_cli(cli)?, - ), - ) - .await - .ok() - .and_then(Result::ok) - .unwrap_or_default(); + let records = lookup_ech_https_records(cli, cli.dns_server.as_deref(), host, timeout).await?; let candidates = ech_candidates_from_records(&records); crate::tls::ech::resolve_ech_mode(cli, &candidates) } @@ -1710,6 +1672,50 @@ mod tests { record } + #[tokio::test] + async fn http_ech_discovery_propagates_exhausted_budget() { + let cli = + Cli::try_parse_from(["fetch", "--ech", "on", "https://ech-timeout.invalid"]).unwrap(); + let timeout = TimeoutBudget::started_at( + Some(Duration::from_millis(1)), + Instant::now() - Duration::from_millis(10), + ); + + let err = resolve_dns_for_client_inner( + &cli, + &Url::parse("https://ech-timeout.invalid").unwrap(), + timeout, + Some(EffectiveProxy { + uses_local_target_dns: false, + }), + false, + ) + .await + .unwrap_err(); + + assert_eq!(err.to_string(), "request timed out after 1ms"); + } + + #[tokio::test] + async fn websocket_ech_discovery_propagates_exhausted_budget() { + let cli = Cli::try_parse_from(["fetch", "--ech", "on", "wss://ech-timeout.invalid/socket"]) + .unwrap(); + let timeout = TimeoutBudget::started_at( + Some(Duration::from_millis(1)), + Instant::now() - Duration::from_millis(10), + ); + + let err = resolve_websocket_ech_mode( + &cli, + &Url::parse("wss://ech-timeout.invalid/socket").unwrap(), + timeout, + ) + .await + .unwrap_err(); + + assert_eq!(err.to_string(), "request timed out after 1ms"); + } + #[test] fn ech_candidates_empty_records() { let candidates = ech_candidates_from_records(&[]); diff --git a/tests/websocket.rs b/tests/websocket.rs index d14f2673..f69f4ba0 100644 --- a/tests/websocket.rs +++ b/tests/websocket.rs @@ -13,7 +13,10 @@ use support::common::{ FetchOpts, FetchOutput, assert_exit, fetch_bin, run_fetch, run_fetch_once, run_fetch_opts, start_read_capture, url_host_port, wait_child, }; -use support::dns::{start_udp_dns_server, start_unresponsive_udp_dns_server}; +use support::dns::{ + start_udp_dns_server, start_udp_dns_server_with_failing_https, + start_unresponsive_udp_dns_server, +}; use support::http::{TestResponse, TestServer, read_request, write_response}; use support::proxy::{ assert_proxy_seen, assert_socks_seen, start_authenticated_http_connect_proxy, @@ -1282,6 +1285,54 @@ fn ech_rejected_for_plain_ws() { ); } +#[test] +fn websocket_ech_discovery_uses_http_error_policy() { + let (wss, _seen) = start_wss_echo_server(|_| Ok(())); + let host = "fetch-websocket-ech-failure.test."; + let dns_addr = start_udp_dns_server_with_failing_https(host, Ipv4Addr::new(127, 0, 0, 1)); + let url = wss + .url + .replace("localhost", "fetch-websocket-ech-failure.test"); + + let required = run_fetch(&[ + &url, + "--insecure", + "--dns-server", + &dns_addr, + "--ech", + "on", + "--ws-interactive", + "off", + ]); + assert_exit(&required, 1); + assert!( + required.stderr.contains("mismatched DNS response ID"), + "{}", + required.stderr + ); + assert!(!required.stderr.contains("does not advertise ECH")); + + let automatic = run_fetch(&[ + &url, + "--insecure", + "--dns-server", + &dns_addr, + "--ech", + "auto", + "-vvv", + "-d", + "ech fallback", + "--format", + "off", + "--ws-interactive", + "off", + ]); + assert_exit(&automatic, 0); + assert!(automatic.stdout.contains("echo: ech fallback")); + assert!(automatic.stderr.contains("ECH discovery failed")); + assert!(automatic.stderr.contains("falling back to GREASE")); +} + #[test] fn ech_allowed_for_wss() { // --ech auto on wss:// should be allowed (not produce a scheme conflict error)