From 81c8ff7bfadf0bb7b389be149d338977b5c14156 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 7 Nov 2024 11:15:20 +0100 Subject: [PATCH] fix(iroh-net): Do not return a port for reqwest DNS resolver (#2906) ## Description A non-zero port is interpreted as overriding the port specified or implied by the URL. Despite the docs not telling you this. That's not really what we want here. ## Breaking Changes ## Notes & open questions https://github.com/seanmonstar/reqwest/issues/2413#issuecomment-2405545067 ## Change checklist - [x] Self-review. - [x] Documentation updates following the [style guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text), if relevant. - [x] Tests if relevant. - [x] All breaking changes documented. --- iroh-net/src/netcheck/reportgen.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/iroh-net/src/netcheck/reportgen.rs b/iroh-net/src/netcheck/reportgen.rs index 8cd20b0c40..153654bf56 100644 --- a/iroh-net/src/netcheck/reportgen.rs +++ b/iroh-net/src/netcheck/reportgen.rs @@ -891,15 +891,14 @@ async fn check_captive_portal( if let Some(Host::Domain(domain)) = url.host() { // Use our own resolver rather than getaddrinfo // - // For some reason reqwest wants SocketAddr rather than IpAddr but then proceeds to - // ignore the port, extracting it from the URL instead. We supply a dummy port. + // Be careful, a non-zero port will override the port in the URI. // // Ideally we would try to resolve **both** IPv4 and IPv6 rather than purely race // them. But our resolver doesn't support that yet. let addrs: Vec<_> = dns_resolver .lookup_ipv4_ipv6_staggered(domain, DNS_TIMEOUT, DNS_STAGGERING_MS) .await? - .map(|ipaddr| SocketAddr::new(ipaddr, 80)) + .map(|ipaddr| SocketAddr::new(ipaddr, 0)) .collect(); builder = builder.resolve_to_addrs(domain, &addrs); } @@ -1062,8 +1061,7 @@ async fn measure_https_latency( if let Some(Host::Domain(domain)) = url.host() { // Use our own resolver rather than getaddrinfo // - // For some reason reqwest wants SocketAddr rather than IpAddr but then proceeds to - // ignore the port, extracting it from the URL instead. We supply a dummy port. + // Be careful, a non-zero port will override the port in the URI. // // The relay Client uses `.lookup_ipv4_ipv6` to connect, so use the same function // but staggered for reliability. Ideally this tries to resolve **both** IPv4 and @@ -1071,7 +1069,7 @@ async fn measure_https_latency( let addrs: Vec<_> = dns_resolver .lookup_ipv4_ipv6_staggered(domain, DNS_TIMEOUT, DNS_STAGGERING_MS) .await? - .map(|ipaddr| SocketAddr::new(ipaddr, 80)) + .map(|ipaddr| SocketAddr::new(ipaddr, 0)) .collect(); builder = builder.resolve_to_addrs(domain, &addrs); }