Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions client/transport/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ web = [
"thiserror",
]

[dev-dependencies]
# `rt` provides the `#[tokio::test]` runtime and the `spawn_blocking` pool that
# `tokio::net::lookup_host` relies on. `macros`/`time` are already enabled via the `ws`
# feature but are listed here so the test requirements are explicit.
tokio = { workspace = true, features = ["rt", "macros", "time"] }

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
Expand Down
58 changes: 52 additions & 6 deletions client/transport/src/ws/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ impl WsTransportClientBuilder {
let mut connector = self.tls_connector(&target)?;

// The sockaddrs might get reused if the server replies with a relative URI.
let mut target_sockaddrs = uri.socket_addrs(|| None).map_err(WsHandshakeError::ResolutionFailed)?;
let mut target_sockaddrs = resolve_sockaddrs(&uri, self.connection_timeout).await?;

for _ in 0..self.max_redirections {
tracing::debug!(target: LOG_TARGET, "Connecting to target: {:?}", target);
Expand Down Expand Up @@ -408,10 +408,7 @@ impl WsTransportClientBuilder {
// redirection with absolute path => need to lookup.
Ok(uri) => {
// Absolute URI.
target_sockaddrs = uri.socket_addrs(|| None).map_err(|e| {
tracing::debug!(target: LOG_TARGET, "Redirection failed: {:?}", e);
e
})?;
target_sockaddrs = resolve_sockaddrs(&uri, self.connection_timeout).await?;

target = uri.try_into().map_err(|e| {
tracing::debug!(target: LOG_TARGET, "Redirection failed: {:?}", e);
Expand Down Expand Up @@ -530,6 +527,42 @@ impl WsTransportClientBuilder {
}
}

/// Resolve the socket addresses for `uri` using asynchronous DNS resolution.
///
/// This replaces `url::Url::socket_addrs`, which performs *blocking* `getaddrinfo`
/// resolution (via `std::net::ToSocketAddrs`) on the calling thread. Inside the async client
/// that means a Tokio worker thread can be blocked for as long as the system resolver takes.
/// `tokio::net::lookup_host` runs the lookup on Tokio's blocking thread pool instead,
/// and IP literals are turned into a `SocketAddr` directly without any DNS lookup.
///
/// The whole resolution is bounded by `timeout_dur`; on expiry it returns
/// `WsHandshakeError::Timeout`. This budget is per-resolution and is additive with the
/// per-address TCP connect timeout applied in `connect`.
async fn resolve_sockaddrs(uri: &Url, timeout_dur: Duration) -> Result<Vec<SocketAddr>, WsHandshakeError> {
let resolve = async {
let port =
uri.port_or_known_default().ok_or_else(|| WsHandshakeError::Url("No port number in the URL".into()))?;

// NOTE: match on `uri.host()` (the `url::Host` enum) rather than `host_str()`. The latter
// returns IPv6 hosts *with* brackets (e.g. `"[::1]"`), which `getaddrinfo` rejects, whereas
// the enum exposes a parsed `Ipv6Addr`. This mirrors what `url::Url::socket_addrs` does
// internally, so IP literals and default ports behave exactly as before.
match uri.host() {
Some(url::Host::Domain(domain)) => {
tokio::net::lookup_host((domain, port)).await.map(|addrs| addrs.collect()).map_err(|e| {

@DenzelPenzel DenzelPenzel Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout stops waiting for lookup_host, but it does not cancel the underlying spawn_blocking DNS lookup. wdty?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, but I don't know how it can addressed.
But current approach makes situation a little better than on main, because on main broken resolver will stall whole runtime, while here we will have task sitting on blocking pool thread.

tracing::debug!(target: LOG_TARGET, "DNS resolution failed for {domain}: {e:?}");
WsHandshakeError::ResolutionFailed(e)
})
}
Some(url::Host::Ipv4(ip)) => Ok(vec![SocketAddr::from((ip, port))]),
Some(url::Host::Ipv6(ip)) => Ok(vec![SocketAddr::from((ip, port))]),
None => Err(WsHandshakeError::Url("No host name in the URL".into())),
}
};

tokio::time::timeout(timeout_dur, resolve).await.map_err(|_| WsHandshakeError::Timeout(timeout_dur))?
}

#[cfg(feature = "tls")]
async fn connect(
sockaddr: SocketAddr,
Expand Down Expand Up @@ -673,9 +706,11 @@ fn build_tls_config(cert_store: &CertificateStore) -> Result<tokio_rustls::TlsCo

#[cfg(test)]
mod tests {
use std::time::Duration;

use http::HeaderValue;

use super::{Mode, Target, Url, WsHandshakeError};
use super::{Mode, Target, Url, WsHandshakeError, resolve_sockaddrs};

fn assert_ws_target(
target: Target,
Expand Down Expand Up @@ -771,4 +806,15 @@ mod tests {

assert_ws_target(target, "127.0.0.1", "127.0.0.1", Mode::Plain, "/", Some(basic_auth));
}

// A slow or broken resolver must not stall the connection: DNS resolution is bounded by the
// connection timeout. `.invalid` is a reserved TLD (RFC 6761) that never resolves, and with a
// zero timeout the (blocking-pool) lookup cannot complete on its first poll, so the timer fires
// first — deterministic and without touching the network.
#[tokio::test]
async fn dns_resolution_is_bounded_by_timeout() {
let url = Url::parse("ws://host.invalid").unwrap();
let err = resolve_sockaddrs(&url, Duration::ZERO).await.unwrap_err();
assert!(matches!(err, WsHandshakeError::Timeout(_)));
}
}