diff --git a/src/dns/svcb/system.rs b/src/dns/svcb/system.rs index 047766c0..4f5b20ea 100644 --- a/src/dns/svcb/system.rs +++ b/src/dns/svcb/system.rs @@ -244,10 +244,9 @@ async fn lookup_with_resolv_conf( host: &str, budget: TimeoutBudget, ) -> Result, FetchError> { - let contents = match tokio::fs::read_to_string("/etc/resolv.conf").await { - Ok(contents) => contents, - Err(_) => return Ok(Vec::new()), - }; + let contents = tokio::fs::read_to_string("/etc/resolv.conf") + .await + .map_err(|err| FetchError::Runtime(format!("read /etc/resolv.conf: {err}")))?; lookup_with_resolv_conf_config(host, budget, &parse_resolv_conf(&contents)).await } @@ -258,7 +257,9 @@ async fn lookup_with_resolv_conf_config( config: &ResolvConf, ) -> Result, FetchError> { if config.nameservers.is_empty() || config.attempts == 0 { - return Ok(Vec::new()); + return Err(FetchError::Runtime( + "system DNS resolver has no usable nameservers or attempts".to_string(), + )); } let start = if config.rotate { RESOLVER_ROTATION.fetch_add(1, Ordering::Relaxed) % config.nameservers.len() @@ -368,6 +369,7 @@ fn lookup_https_records_blocking( type DNSServiceFlags = c_uint; const K_DNS_SERVICE_ERR_NO_ERROR: DNSServiceErrorType = 0; + const K_DNS_SERVICE_ERR_NO_SUCH_RECORD: DNSServiceErrorType = -65554; const K_DNS_SERVICE_FLAGS_MORE_COMING: DNSServiceFlags = 1; type DNSServiceQueryRecordReply = unsafe extern "C" fn( @@ -436,6 +438,10 @@ fn lookup_https_records_blocking( return; } let state = unsafe { &mut *(context.cast::()) }; + if error_code == K_DNS_SERVICE_ERR_NO_SUCH_RECORD { + state.finished = true; + return; + } if error_code != K_DNS_SERVICE_ERR_NO_ERROR { state.error = Some(format!("system HTTPS record lookup failed: {error_code}")); state.finished = true; @@ -480,12 +486,16 @@ fn lookup_https_records_blocking( ) }; if status != K_DNS_SERVICE_ERR_NO_ERROR { - return Ok(Vec::new()); + return Err(FetchError::Runtime(format!( + "system DNS query setup failed with status {status}" + ))); } let _guard = DnsServiceRefGuard(sd_ref); let fd = unsafe { DNSServiceRefSockFD(sd_ref) }; if fd < 0 { - return Ok(Vec::new()); + return Err(FetchError::Runtime( + "system DNS query returned an invalid socket".to_string(), + )); } let deadline = timeout.and_then(|timeout| Instant::now().checked_add(timeout)); @@ -494,7 +504,9 @@ fn lookup_https_records_blocking( break; } let Some(timeout_ms) = poll_timeout_ms(deadline) else { - return Ok(Vec::new()); + return Err(FetchError::Runtime( + "system DNS query timed out".to_string(), + )); }; let mut pollfd = libc::pollfd { fd, @@ -503,7 +515,9 @@ fn lookup_https_records_blocking( }; let ready = unsafe { libc::poll(&mut pollfd, 1, timeout_ms) }; if ready == 0 { - return Ok(Vec::new()); + return Err(FetchError::Runtime( + "system DNS query timed out".to_string(), + )); } if ready < 0 { return Err(FetchError::Runtime(format!( @@ -513,7 +527,9 @@ fn lookup_https_records_blocking( } let status = unsafe { DNSServiceProcessResult(sd_ref) }; if status != K_DNS_SERVICE_ERR_NO_ERROR { - return Ok(Vec::new()); + return Err(FetchError::Runtime(format!( + "system DNS response processing failed with status {status}" + ))); } } @@ -567,7 +583,17 @@ fn lookup_https_records_blocking( std::ptr::null_mut(), ) }; - if status != 0 || records.is_null() { + const DNS_ERROR_RCODE_NAME_ERROR: u32 = 9003; + const DNS_INFO_NO_RECORDS: u32 = 9501; + if status == DNS_ERROR_RCODE_NAME_ERROR || status == DNS_INFO_NO_RECORDS { + return Ok(Vec::new()); + } + if status != 0 { + return Err(FetchError::Runtime(format!( + "system DNS query failed with status {status}" + ))); + } + if records.is_null() { return Ok(Vec::new()); } let _guard = DnsRecordListGuard(records); @@ -794,6 +820,20 @@ mod tests { assert!(config.rotate); } + #[cfg(all(unix, not(target_os = "macos")))] + #[tokio::test] + async fn empty_resolver_config_is_a_lookup_failure() { + let err = lookup_with_resolv_conf_config( + "example.com", + TimeoutBudget::new(Some(std::time::Duration::from_secs(1))), + &ResolvConf::default(), + ) + .await + .unwrap_err(); + + assert!(err.to_string().contains("no usable nameservers")); + } + #[cfg(all(unix, not(target_os = "macos")))] #[tokio::test] async fn resolv_conf_lookup_fails_over_to_second_nameserver() { diff --git a/src/http/client.rs b/src/http/client.rs index 5968cc28..a3360722 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -282,7 +282,7 @@ 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 https_records = if let Some(dns_server) = cli.dns_server.as_deref() { + let (https_records, _) = if let Some(dns_server) = cli.dns_server.as_deref() { lookup_ech_https_records(cli, Some(dns_server), host, timeout).await? } else { lookup_ech_https_records(cli, None, host, timeout).await? @@ -313,14 +313,15 @@ async fn resolve_dns_for_client_inner( if let Some(dns_server) = cli.dns_server.as_deref() { let start = Instant::now(); - let (addrs, https_lookup) = if need_ech_svcb { + let (addrs, https_lookup, https_lookup_succeeded) = 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 (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, timeout), ); - (addrs, https_records?) + let (https_records, succeeded) = https_records?; + (addrs, https_records, succeeded) } else if auto_http3 && custom::dns_server_is_authenticated(dns_server, !cli.insecure)? { // Authenticated HTTPS lookup failures must gate service access. // Run address and service discovery together, but do not abort or @@ -329,7 +330,7 @@ async fn resolve_dns_for_client_inner( lookup_custom_ips_with_doh_tls(cli, dns_server, host, timeout), lookup_authenticated_auto_http3_https_records(cli, dns_server, host, timeout), ); - (addrs, https_records?) + (addrs, https_records?, true) } else if let Some(auto_http3_budget) = auto_http3_discovery { let https = spawn_auto_http3_https_records( Some(dns_server.to_string()), @@ -337,16 +338,19 @@ async fn resolve_dns_for_client_inner( Some(auto_http3_budget), ); let addrs = lookup_custom_ips_with_doh_tls(cli, dns_server, host, timeout).await; - let https_records = if addrs.is_err() { - https.await.unwrap_or_else(|_| empty_https_lookup(host)) + let (https_records, succeeded) = if addrs.is_err() { + https + .await + .unwrap_or_else(|_| (empty_https_lookup(host), false)) } else { take_finished_auto_http3_https_records(https, host).await }; - (addrs, https_records) + (addrs, https_records, succeeded) } else { ( lookup_custom_ips_with_doh_tls(cli, dns_server, host, timeout).await, empty_https_lookup(host), + false, ) }; let addrs = resolve_custom_alias_fallback( @@ -366,7 +370,7 @@ async fn resolve_dns_for_client_inner( &https_lookup.fallback_target, &socket_addrs, ); - if auto_http3 { + if auto_http3 && https_lookup_succeeded { Http3Cache::new() .store_https_records(url, Some(dns_server), &https_records) .await; @@ -408,22 +412,23 @@ async fn resolve_dns_for_client_inner( }); let start = Instant::now(); let lookup = tokio::net::lookup_host((host, port)); - let (socket_addrs, https_records) = if need_ech_svcb { + let (socket_addrs, https_records, https_lookup_succeeded) = if need_ech_svcb { // ECH requires HTTPS records; await the SVCB query properly instead // of using the abort-early auto-H3 pattern. let (socket_addrs, https_records) = tokio::join!(lookup, lookup_ech_https_records(cli, None, host, timeout),); - ( - socket_addrs - .map(|addrs| addrs.collect::>()) - .map_err(|err| FetchError::Runtime(format!("lookup {host}: {err}"))), - https_records?, - ) + let socket_addrs = socket_addrs + .map(|addrs| addrs.collect::>()) + .map_err(|err| FetchError::Runtime(format!("lookup {host}: {err}"))); + let (https_records, succeeded) = https_records?; + (socket_addrs, https_records, succeeded) } else if let Some(auto_http3_budget) = auto_http3_discovery { let https = spawn_auto_http3_https_records(None, host.to_string(), Some(auto_http3_budget)); let socket_addrs = lookup.await; - let https_records = if socket_addrs.is_err() { - https.await.unwrap_or_else(|_| empty_https_lookup(host)) + let (https_records, succeeded) = if socket_addrs.is_err() { + https + .await + .unwrap_or_else(|_| (empty_https_lookup(host), false)) } else { take_finished_auto_http3_https_records(https, host).await }; @@ -432,6 +437,7 @@ async fn resolve_dns_for_client_inner( .map(|addrs| addrs.collect::>()) .map_err(|err| FetchError::Runtime(format!("lookup {host}: {err}"))), https_records, + succeeded, ) } else { ( @@ -440,6 +446,7 @@ async fn resolve_dns_for_client_inner( .map(|addrs| addrs.collect::>()) .map_err(|err| FetchError::Runtime(format!("lookup {host}: {err}"))), empty_https_lookup(host), + false, ) }; let socket_addrs = resolve_system_alias_fallback( @@ -455,7 +462,7 @@ async fn resolve_dns_for_client_inner( let addrs = dns_timing_addrs(socket_addrs.iter().map(|addr| addr.ip())); let auto_http3_config = auto_http3_config_for_records(&https_records, &effective_host, &socket_addrs); - if auto_http3 { + if auto_http3 && https_lookup_succeeded { Http3Cache::new() .store_https_records(url, None, &https_records) .await; @@ -563,21 +570,22 @@ async fn lookup_auto_http3_https_records( dns_server: Option<&str>, host: &str, discovery_budget: Option, -) -> HttpsLookup { +) -> (HttpsLookup, bool) { let Some(timeout) = discovery_budget.and_then(AutoHttp3DiscoveryBudget::remaining) else { - return empty_https_lookup(host); + return (empty_https_lookup(host), false); }; let resolver = dns_server .map(HttpsRecordResolver::Custom) .unwrap_or(HttpsRecordResolver::System); - tokio::time::timeout( + match tokio::time::timeout( timeout, crate::dns::svcb::lookup_https_records(resolver, host, Some(timeout)), ) .await - .ok() - .and_then(Result::ok) - .unwrap_or_else(|| empty_https_lookup(host)) + { + Ok(Ok(lookup)) => (lookup, true), + Ok(Err(_)) | Err(_) => (empty_https_lookup(host), false), + } } async fn lookup_ech_https_records( @@ -585,7 +593,7 @@ async fn lookup_ech_https_records( dns_server: Option<&str>, host: &str, timeout: TimeoutBudget, -) -> Result { +) -> Result<(HttpsLookup, bool), FetchError> { let ech_timeout = timeout.remaining()?.unwrap_or(Duration::from_secs(5)); let resolver = dns_server .map(HttpsRecordResolver::Custom) @@ -599,14 +607,14 @@ async fn lookup_ech_https_records( )) .await { - Ok(lookup) => Ok(lookup), + Ok(lookup) => Ok((lookup, true)), Err(err) => { let authenticated = dns_server .map(|server| custom::dns_server_is_authenticated(server, !cli.insecure)) .transpose()? .unwrap_or(false); crate::tls::ech::handle_ech_discovery_error(cli, err, authenticated)?; - Ok(empty_https_lookup(host)) + Ok((empty_https_lookup(host), false)) } } } @@ -632,24 +640,26 @@ fn spawn_auto_http3_https_records( dns_server: Option, host: String, discovery_budget: Option, -) -> JoinHandle { +) -> JoinHandle<(HttpsLookup, bool)> { tokio::spawn(async move { lookup_auto_http3_https_records(dns_server.as_deref(), &host, discovery_budget).await }) } async fn take_finished_auto_http3_https_records( - handle: JoinHandle, + handle: JoinHandle<(HttpsLookup, bool)>, host: &str, -) -> HttpsLookup { +) -> (HttpsLookup, bool) { if !handle.is_finished() { tokio::task::yield_now().await; } if handle.is_finished() { - handle.await.unwrap_or_else(|_| empty_https_lookup(host)) + handle + .await + .unwrap_or_else(|_| (empty_https_lookup(host), false)) } else { handle.abort(); - empty_https_lookup(host) + (empty_https_lookup(host), false) } } @@ -789,7 +799,8 @@ pub(crate) async fn resolve_websocket_ech_mode( if host.parse::().is_ok() { return Ok(None); } - let records = lookup_ech_https_records(cli, cli.dns_server.as_deref(), host, timeout).await?; + let (records, _) = + lookup_ech_https_records(cli, cli.dns_server.as_deref(), host, timeout).await?; let candidates = ech_candidates_from_records(&records.records); crate::tls::ech::resolve_ech_mode(cli, &candidates) } diff --git a/src/http/http3_cache.rs b/src/http/http3_cache.rs index bc22a699..b3580714 100644 --- a/src/http/http3_cache.rs +++ b/src/http/http3_cache.rs @@ -43,6 +43,7 @@ pub(crate) struct Http3CacheCandidate { pub(crate) alt_host: String, pub(crate) alt_port: u16, pub(crate) priority: Option, + pub(crate) from_alt_svc: bool, } #[derive(Debug, Serialize, Deserialize)] @@ -125,6 +126,7 @@ impl Http3Cache { .candidates .into_iter() .map(|candidate| Http3CacheCandidate { + from_alt_svc: candidate.source == SOURCE_ALT_SVC, alt_host: candidate.alt_host, alt_port: candidate.alt_port, priority: candidate.priority, @@ -223,14 +225,14 @@ impl Http3Cache { }) .collect::>(); candidates.sort_by_key(|(priority, _, _, _)| *priority); - if candidates.is_empty() { - return; - } let cache = self.clone(); let _ = tokio::task::spawn_blocking(move || { cache.update_shard(&path, &key, |shard, now| { prune_expired_candidates(&mut shard.candidates, now); + shard + .candidates + .retain(|candidate| candidate.source != SOURCE_HTTPS); for (priority, alt_host, alt_port, ttl) in &candidates { let ttl = u64::from(*ttl).min(MAX_RETENTION_SECS); let candidate = StoredCandidate { @@ -854,6 +856,7 @@ mod tests { alt_host: "example.com".to_string(), alt_port: 9443, priority: Some(1), + from_alt_svc: false, }] ); assert!( @@ -886,6 +889,72 @@ mod tests { assert!(cache.candidates(&test_url(), None).await.is_empty()); } + #[tokio::test] + async fn fresh_https_records_replace_previous_rrset() { + let dir = TempDir::new().unwrap(); + let cache = Http3Cache::with_dir(dir.path().to_path_buf()); + let old = record("old.example.", Some(9443), Some(60)); + let retained = record("retained.example.", Some(9444), Some(60)); + cache + .store_https_records(&test_url(), None, &[old, retained.clone()]) + .await; + + cache + .store_https_records(&test_url(), None, &[retained]) + .await; + + let got = cache.candidates(&test_url(), None).await; + assert_eq!(got.len(), 1); + assert_eq!(got[0].alt_host, "retained.example"); + } + + #[tokio::test] + async fn fresh_https_records_replace_changed_priority() { + let dir = TempDir::new().unwrap(); + let cache = Http3Cache::with_dir(dir.path().to_path_buf()); + let mut candidate = record("retained.example.", Some(9444), Some(60)); + cache + .store_https_records(&test_url(), None, &[candidate.clone()]) + .await; + + candidate.priority = 7; + cache + .store_https_records(&test_url(), None, &[candidate]) + .await; + + assert_eq!( + cache.candidates(&test_url(), None).await[0].priority, + Some(7) + ); + } + + #[tokio::test] + async fn successful_https_nodata_clears_only_dns_candidates() { + let dir = TempDir::new().unwrap(); + let cache = Http3Cache::with_dir(dir.path().to_path_buf()); + cache + .store_https_records(&test_url(), None, &[record(".", Some(9443), Some(60))]) + .await; + let mut headers = HeaderMap::new(); + headers.insert( + ALT_SVC, + HeaderValue::from_static(r#"h3="alt.example:9555"; ma=60"#), + ); + cache.store_alt_svc(&test_url(), None, &headers).await; + + cache.store_https_records(&test_url(), None, &[]).await; + + assert_eq!( + cache.candidates(&test_url(), None).await, + vec![Http3CacheCandidate { + alt_host: "alt.example".to_string(), + alt_port: 9555, + priority: None, + from_alt_svc: true, + }] + ); + } + #[tokio::test] async fn removes_failed_candidates() { let dir = TempDir::new().unwrap(); diff --git a/src/http/transport/h3.rs b/src/http/transport/h3.rs index 7391ee31..7b013f32 100644 --- a/src/http/transport/h3.rs +++ b/src/http/transport/h3.rs @@ -171,13 +171,13 @@ impl Client { .host_str() .ok_or_else(|| Error::request("URL host is required"))?; let discovery_start = std::time::Instant::now(); - let cached_candidates = self - .config - .http3_cache - .as_ref() - .map(|cache| cache.candidates(url, self.config.dns_server.as_deref())); - let cached_candidates = match cached_candidates { - Some(candidates) => candidates.await, + let alt_svc_candidates = match &self.config.http3_cache { + Some(cache) => cache + .candidates(url, self.config.dns_server.as_deref()) + .await + .into_iter() + .filter(|candidate| candidate.from_alt_svc) + .collect(), None => Vec::new(), }; let fresh = self.connect_fresh_dynamic_auto_http3_client( @@ -187,9 +187,44 @@ impl Client { discovery_start, timeout, ); - let cached = - self.connect_cached_dynamic_auto_http3_client(url, origin, cached_candidates, timeout); - race_dynamic_auto_http3_candidates(fresh, cached).await + let alt_svc = self.connect_cached_dynamic_auto_http3_client( + url, + origin.clone(), + alt_svc_candidates, + timeout, + ); + let fresh_outcome = match race_dynamic_auto_http3_candidates(fresh, alt_svc).await { + Ok(result) => return Ok(result), + Err(outcome) => outcome, + }; + match fresh_outcome { + DynamicHttp3ConnectOutcome::Failed(err) => return Err(err), + DynamicHttp3ConnectOutcome::NoCandidates => { + return Err(Error::connect("no HTTP/3 candidates discovered")); + } + DynamicHttp3ConnectOutcome::Connected(_) => unreachable!("winner returned above"), + DynamicHttp3ConnectOutcome::LookupFailed => {} + } + + // A failed lookup leaves previous DNS-derived candidates available. + let cached_candidates = match &self.config.http3_cache { + Some(cache) => { + cache + .candidates(url, self.config.dns_server.as_deref()) + .await + } + None => Vec::new(), + }; + match self + .connect_cached_dynamic_auto_http3_client(url, origin, cached_candidates, timeout) + .await + { + DynamicHttp3ConnectOutcome::Connected(result) => Ok(result), + DynamicHttp3ConnectOutcome::Failed(err) => Err(err), + DynamicHttp3ConnectOutcome::NoCandidates | DynamicHttp3ConnectOutcome::LookupFailed => { + Err(Error::connect("no HTTP/3 candidates discovered")) + } + } } async fn connect_fresh_dynamic_auto_http3_client( @@ -206,9 +241,13 @@ impl Client { self.config.doh_tls_config.clone(), timeout, ); - let lookup = + let Some(lookup) = lookup_auto_http3_https_records(self.config.dns_server.as_deref(), &host, timeout) - .await; + .await + else { + origin_addrs_task.abort(); + return DynamicHttp3ConnectOutcome::LookupFailed; + }; if let Some(cache) = &self.config.http3_cache { cache .store_https_records(url, self.config.dns_server.as_deref(), &lookup.records) @@ -413,92 +452,46 @@ enum DynamicHttp3ConnectOutcome { Connected(Http3ConnectResult), Failed(Error), NoCandidates, + LookupFailed, } async fn race_dynamic_auto_http3_candidates( fresh: FreshFuture, cached: CachedFuture, -) -> Result +) -> Result where FreshFuture: Future, CachedFuture: Future, { let mut fresh = Box::pin(fresh); - let prompt_fresh = tokio::task::yield_now(); - tokio::pin!(prompt_fresh); - + let mut cached = Box::pin(cached); let mut fresh_done = false; let mut cached_done = false; - let mut fresh_err = None; - let mut cached_err = None; - - tokio::select! { - result = fresh.as_mut() => { - fresh_done = true; - if let Some(result) = record_dynamic_http3_outcome(result, &mut fresh_err) { - return Ok(result); - } - } - _ = &mut prompt_fresh => {} - } - - let mut cached = Box::pin(cached); - - loop { - if fresh_done && cached_done { - return Err(fresh_err - .or(cached_err) - .unwrap_or_else(|| Error::connect("no HTTP/3 candidates discovered"))); - } + let mut fresh_outcome = None; - match (fresh_done, cached_done) { - (false, false) => { - tokio::select! { - result = fresh.as_mut() => { - fresh_done = true; - if let Some(result) = record_dynamic_http3_outcome(result, &mut fresh_err) { - return Ok(result); - } - } - result = cached.as_mut() => { - cached_done = true; - if let Some(result) = record_dynamic_http3_outcome(result, &mut cached_err) { - return Ok(result); - } - } - } - } - (false, true) => { - let result = fresh.as_mut().await; + while !fresh_done || !cached_done { + let (is_fresh, outcome) = tokio::select! { + outcome = fresh.as_mut(), if !fresh_done => { fresh_done = true; - if let Some(result) = record_dynamic_http3_outcome(result, &mut fresh_err) { - return Ok(result); - } + (true, outcome) } - (true, false) => { - let result = cached.as_mut().await; + outcome = cached.as_mut(), if !cached_done => { cached_done = true; - if let Some(result) = record_dynamic_http3_outcome(result, &mut cached_err) { - return Ok(result); - } + (false, outcome) } - (true, true) => unreachable!("handled at top of loop"), + }; + if let DynamicHttp3ConnectOutcome::Connected(result) = outcome { + return Ok(result); } - } -} - -fn record_dynamic_http3_outcome( - outcome: DynamicHttp3ConnectOutcome, - error: &mut Option, -) -> Option { - match outcome { - DynamicHttp3ConnectOutcome::Connected(result) => Some(result), - DynamicHttp3ConnectOutcome::Failed(err) => { - *error = Some(err); - None + if is_fresh { + if matches!(outcome, DynamicHttp3ConnectOutcome::LookupFailed) { + return Err(outcome); + } + fresh_outcome = Some(outcome); } - DynamicHttp3ConnectOutcome::NoCandidates => None, } + + Err(fresh_outcome.unwrap_or(DynamicHttp3ConnectOutcome::LookupFailed)) } pub(super) fn spawn_auto_http3_origin_addrs( @@ -532,13 +525,8 @@ async fn lookup_auto_http3_https_records( dns_server: Option<&str>, host: &str, timeout: TimeoutBudget, -) -> HttpsLookup { - let Some(timeout) = auto_http3_lookup_timeout(timeout) else { - return HttpsLookup { - records: Vec::new(), - fallback_target: host.to_string(), - }; - }; +) -> Option { + let timeout = auto_http3_lookup_timeout(timeout)?; let resolver = dns_server .map(HttpsRecordResolver::Custom) .unwrap_or(HttpsRecordResolver::System); @@ -549,10 +537,6 @@ async fn lookup_auto_http3_https_records( .await .ok() .and_then(Result::ok) - .unwrap_or_else(|| HttpsLookup { - records: Vec::new(), - fallback_target: host.to_string(), - }) } fn auto_http3_lookup_timeout(timeout: TimeoutBudget) -> Option { @@ -1163,3 +1147,26 @@ fn build_h3_request( } } } + +#[cfg(test)] +mod dynamic_cache_tests { + use super::*; + + #[tokio::test] + async fn lookup_failure_does_not_wait_for_stalled_alt_svc() { + let fresh = std::future::ready(DynamicHttp3ConnectOutcome::LookupFailed); + let stalled_alt_svc = std::future::pending::(); + + let outcome = tokio::time::timeout( + Duration::from_millis(50), + race_dynamic_auto_http3_candidates(fresh, stalled_alt_svc), + ) + .await + .expect("lookup failure waited for Alt-Svc"); + + assert!(matches!( + outcome, + Err(DynamicHttp3ConnectOutcome::LookupFailed) + )); + } +} diff --git a/tests/network.rs b/tests/network.rs index 9343b6ce..acaefeca 100644 --- a/tests/network.rs +++ b/tests/network.rs @@ -12,7 +12,8 @@ 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_delayed_https_and_resolution, + start_udp_dns_server_with_controllable_https, start_udp_dns_server_with_delayed_aaaa, + start_udp_dns_server_with_delayed_https_and_resolution, start_udp_dns_server_with_delayed_resolution, start_udp_dns_server_with_failing_https, start_udp_dns_server_with_hosts, start_udp_dns_server_with_https, start_udp_dns_server_with_https_alias, start_udp_dns_server_with_https_target_and_stale_hint, @@ -927,7 +928,50 @@ fn default_https_supplements_stale_hint_with_non_origin_target_lookup() { } #[test] -fn default_https_uses_cached_http3_from_https_dns_record() { +fn default_https_nodata_does_not_use_stale_cached_https_dns_record() { + let cache_dir = TempDir::new().unwrap(); + let h3 = start_http3_server(|_| H3Response::ok("stale h3")); + let h3_port = Url::parse(&h3.url).unwrap().port().unwrap(); + let (dns_addr, advertise_https) = + start_udp_dns_server_with_toggleable_https("localhost.", Ipv4Addr::LOCALHOST, h3_port); + let env = vec![( + "FETCH_INTERNAL_HTTP3_CACHE_DIR".to_string(), + cache_dir.path().display().to_string(), + )]; + let args = [ + "--dns-server", + &dns_addr, + "--ca-cert", + h3.ca_cert_path.to_str().unwrap(), + &format!("https://localhost:{h3_port}/nodata-replacement"), + ]; + + let res = run_fetch_opts( + FetchOpts { + env: env.clone(), + ..Default::default() + }, + &args, + ); + assert_exit(&res, 0); + advertise_https.store(false, Ordering::SeqCst); + + let res = run_fetch_opts( + FetchOpts { + env, + ..Default::default() + }, + &args, + ); + assert!( + !res.status.success(), + "stale DNS candidate served NODATA request" + ); + assert_eq!(wait_for_h3_requests(&h3, 1).len(), 1); +} + +#[test] +fn default_https_lookup_failure_preserves_cached_https_dns_record() { let cache_dir = TempDir::new().unwrap(); let h3 = start_http3_server(|req| match req.path.as_str() { "/learn-h3-cache" => H3Response::ok("learned h3 cache"), @@ -935,7 +979,7 @@ fn default_https_uses_cached_http3_from_https_dns_record() { _ => H3Response::status(404, "not found"), }); let h3_port = Url::parse(&h3.url).unwrap().port().unwrap(); - let (dns_addr, advertise_https) = start_udp_dns_server_with_toggleable_https( + let (dns_addr, _, drop_https) = start_udp_dns_server_with_controllable_https( "localhost.", Ipv4Addr::new(127, 0, 0, 1), h3_port, @@ -962,7 +1006,7 @@ fn default_https_uses_cached_http3_from_https_dns_record() { assert_eq!(res.stdout, "learned h3 cache"); assert!(res.stderr.contains("HTTP/3.0 200 OK"), "{}", res.stderr); - advertise_https.store(false, Ordering::SeqCst); + drop_https.store(true, Ordering::SeqCst); let res = run_fetch_opts( FetchOpts { diff --git a/tests/support/dns.rs b/tests/support/dns.rs index 2961181f..5e638032 100644 --- a/tests/support/dns.rs +++ b/tests/support/dns.rs @@ -123,16 +123,31 @@ pub(crate) fn start_udp_dns_server_with_toggleable_https( ip: Ipv4Addr, https_port: u16, ) -> (String, Arc) { + let (addr, advertise_https, _) = + start_udp_dns_server_with_controllable_https(host, ip, https_port); + (addr, advertise_https) +} + +pub(crate) fn start_udp_dns_server_with_controllable_https( + host: &'static str, + ip: Ipv4Addr, + https_port: u16, +) -> (String, Arc, Arc) { let socket = UdpSocket::bind("127.0.0.1:0").expect("bind udp dns server"); let addr = socket.local_addr().unwrap().to_string(); let advertise_https = Arc::new(AtomicBool::new(true)); let advertise_for_thread = advertise_https.clone(); + let drop_https = Arc::new(AtomicBool::new(false)); + let drop_for_thread = drop_https.clone(); 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; }; + if name == host && qtype == TYPE_HTTPS && drop_for_thread.load(Ordering::SeqCst) { + continue; + } let answer = if name == host && qtype == TYPE_A { Some((TYPE_A, ip.octets().to_vec())) } else if name == host @@ -147,7 +162,7 @@ pub(crate) fn start_udp_dns_server_with_toggleable_https( let _ = socket.send_to(&response, peer); } }); - (addr, advertise_https) + (addr, advertise_https, drop_https) } pub(crate) fn start_udp_dns_server_with_https_target_dropping_target(