From 5cba02a6c8d2fb78ac9a68dc31ed897581e32fbd Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Tue, 21 Apr 2026 18:06:22 +0300 Subject: [PATCH] refactor(bootstrap): BTreeMap for overrides + simplify review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Switch overrides from HashMap to BTreeMap — deterministic iteration by type, drops the manual sort when logging. - Rename the flat_map closure's inner `ips` to `addrs` to stop shadowing the outer Vec. - Trim the Suite 8 TEST-NET-1 comment to keep the "why" and drop mechanism narration. - Drop a redundant sleep 1 after wait — wait already blocks on exit. --- src/bootstrap_resolver.rs | 19 +++++++++---------- src/config.rs | 4 ++-- src/serve.rs | 2 +- tests/integration.sh | 10 ++++------ 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/bootstrap_resolver.rs b/src/bootstrap_resolver.rs index 94b03ea..c3be8bd 100644 --- a/src/bootstrap_resolver.rs +++ b/src/bootstrap_resolver.rs @@ -13,7 +13,7 @@ //! servers, with TCP fallback on UDP timeout (for networks that block //! outbound UDP:53 — see memory: `project_network_udp_hostile.md`). -use std::collections::HashMap; +use std::collections::BTreeMap; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::time::Duration; @@ -34,7 +34,7 @@ const DEFAULT_BOOTSTRAP: &[SocketAddr] = &[ pub struct NumaResolver { bootstrap: Vec, - overrides: HashMap>, + overrides: BTreeMap>, } impl NumaResolver { @@ -44,7 +44,7 @@ impl NumaResolver { /// `fallback` entries are filtered to IP literals only — hostnames would /// re-introduce the self-loop inside the resolver itself. Empty or /// unusable fallback yields the hardcoded default (Quad9 + Cloudflare). - pub fn new(fallback: &[String], overrides: HashMap>) -> Self { + pub fn new(fallback: &[String], overrides: BTreeMap>) -> Self { let mut bootstrap: Vec = Vec::with_capacity(fallback.len()); for entry in fallback { match crate::forward::parse_upstream_addr(entry, 53) { @@ -71,11 +71,10 @@ impl NumaResolver { source ); if !overrides.is_empty() { - let mut pairs: Vec = overrides + let pairs: Vec = overrides .iter() - .flat_map(|(host, ips)| ips.iter().map(move |ip| format!("{}={}", host, ip))) + .flat_map(|(host, addrs)| addrs.iter().map(move |ip| format!("{}={}", host, ip))) .collect(); - pairs.sort(); info!( "bootstrap resolver: host overrides (skip DNS, connect direct): {}", pairs.join(", ") @@ -185,7 +184,7 @@ mod tests { #[test] fn empty_fallback_uses_defaults() { - let r = NumaResolver::new(&[], HashMap::new()); + let r = NumaResolver::new(&[], BTreeMap::new()); let got: Vec = r.bootstrap().iter().map(|s| s.to_string()).collect(); assert_eq!(got, vec!["9.9.9.9:53", "1.1.1.1:53"]); } @@ -197,14 +196,14 @@ mod tests { "dns.quad9.net".to_string(), "1.1.1.1:5353".to_string(), ]; - let r = NumaResolver::new(&fallback, HashMap::new()); + let r = NumaResolver::new(&fallback, BTreeMap::new()); let got: Vec = r.bootstrap().iter().map(|s| s.to_string()).collect(); assert_eq!(got, vec!["9.9.9.9:53", "1.1.1.1:5353"]); } #[test] fn override_returns_configured_ips_without_dns() { - let mut overrides = HashMap::new(); + let mut overrides = BTreeMap::new(); overrides.insert( "odoh-relay.example".to_string(), vec![IpAddr::V4(Ipv4Addr::new(178, 104, 229, 30))], @@ -220,7 +219,7 @@ mod tests { #[test] fn override_supports_multiple_ips_including_ipv6() { - let mut overrides = HashMap::new(); + let mut overrides = BTreeMap::new(); overrides.insert( "dual.example".to_string(), vec![ diff --git a/src/config.rs b/src/config.rs index 272e6c6..6daf430 100644 --- a/src/config.rs +++ b/src/config.rs @@ -245,8 +245,8 @@ impl OdohUpstream { /// Per-host IP overrides for the bootstrap resolver, lifted from /// `relay_ip`/`target_ip`. Keeps the "zero plain-DNS leak for ODoH /// endpoints" property when numa is its own system resolver. - pub fn host_ip_overrides(&self) -> std::collections::HashMap> { - let mut out = std::collections::HashMap::new(); + pub fn host_ip_overrides(&self) -> std::collections::BTreeMap> { + let mut out = std::collections::BTreeMap::new(); if let Some(addr) = self.relay_bootstrap { out.entry(self.relay_host.clone()) .or_insert_with(Vec::new) diff --git a/src/serve.rs b/src/serve.rs index 288f6f8..c76d174 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -59,7 +59,7 @@ pub async fn run(config_path: String) -> crate::Result<()> { .odoh_upstream() .map(|o| o.host_ip_overrides()) .unwrap_or_default(), - _ => std::collections::HashMap::new(), + _ => std::collections::BTreeMap::new(), }; let bootstrap_resolver: Arc = Arc::new(NumaResolver::new( &config.upstream.fallback, diff --git a/tests/integration.sh b/tests/integration.sh index 1773c11..76b1dab 100755 --- a/tests/integration.sh +++ b/tests/integration.sh @@ -975,11 +975,10 @@ check "Same-host relay+target rejected at startup" \ "same host" \ "$STARTUP_OUT" -# relay_ip / target_ip must land in the bootstrap resolver's override map, -# so reqwest connects direct to the configured IPs instead of resolving the -# hostnames via plain DNS (ODoH's zero-plain-DNS-leak property). Using -# RFC 5737 TEST-NET-1 IPs — never routable, so the OdohConfigCache won't -# actually connect, but the override-map wiring is visible in the startup log. +# Guards ODoH's zero-plain-DNS-leak property: relay_ip / target_ip must +# land in the bootstrap resolver's override map so reqwest connects direct +# to the configured IPs instead of resolving the hostnames via plain DNS. +# RFC 5737 TEST-NET-1 IPs (unroutable). cat > "$CONFIG" << 'CONF' [server] bind_addr = "127.0.0.1:5354" @@ -1019,7 +1018,6 @@ check "target_ip wired into bootstrap override map" \ kill "$NUMA_PID" 2>/dev/null || true wait "$NUMA_PID" 2>/dev/null || true -sleep 1 fi # end Suite 8