Skip to content

Commit

Permalink
fix(iroh-net): Do not return a port for reqwest DNS resolver (#2906)
Browse files Browse the repository at this point in the history
## 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

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions


seanmonstar/reqwest#2413 (comment)

## 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.
  • Loading branch information
flub authored Nov 7, 2024
1 parent 91d44dc commit 81c8ff7
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions iroh-net/src/netcheck/reportgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -1062,16 +1061,15 @@ 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
// IPv6 though. But our resolver does not have a function for 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);
}
Expand Down

0 comments on commit 81c8ff7

Please sign in to comment.