Skip to content

Commit e34fee7

Browse files
committed
feat: lookup_host_with_cache(): Don't return empty address list (#7596)
All users of this function expect a nonempty list to be returned, otherwise they fail with hard to understand errors like "No connection attempts were made", so it's better to fail early if DNS resolution failed and the cache doesn't contain resolutions as well.
1 parent 7ba4a43 commit e34fee7

File tree

2 files changed

+14
-8
lines changed

2 files changed

+14
-8
lines changed

python/tests/test_1_online.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,16 +1295,17 @@ def test_configure_error_msgs_invalid_server(acfactory):
12951295
ev = ac2._evtracker.get_matching("DC_EVENT_CONFIGURE_PROGRESS")
12961296
if ev.data1 == 0:
12971297
break
1298+
err_lower = ev.data2.lower()
12981299
# Can't connect so it probably should say something about "internet"
12991300
# again, should not repeat itself
13001301
# If this fails then probably `e.msg.to_lowercase().contains("could not resolve")`
13011302
# in configure.rs returned false because the error message was changed
13021303
# (i.e. did not contain "could not resolve" anymore)
1303-
assert (ev.data2.count("internet") + ev.data2.count("network")) == 1
1304+
assert (err_lower.count("internet") + err_lower.count("network")) == 1
13041305
# Should mention that it can't connect:
1305-
assert ev.data2.count("connect") == 1
1306+
assert err_lower.count("connect") == 1
13061307
# The users do not know what "configuration" is
1307-
assert "configuration" not in ev.data2.lower()
1308+
assert "configuration" not in err_lower
13081309

13091310

13101311
def test_status(acfactory):

src/net/dns.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
//! used for successful connection timestamp of
4141
//! retrieving them from in-memory cache is used.
4242
43-
use anyhow::{Context as _, Result};
43+
use anyhow::{Context as _, Result, ensure};
4444
use std::collections::HashMap;
4545
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr};
4646
use std::str::FromStr;
@@ -850,7 +850,7 @@ pub(crate) async fn lookup_host_with_cache(
850850
}
851851
};
852852

853-
if load_cache {
853+
let addrs = if load_cache {
854854
let mut cache = lookup_cache(context, hostname, port, alpn, now).await?;
855855
if let Some(ips) = DNS_PRELOAD.get(hostname) {
856856
for ip in ips {
@@ -861,10 +861,15 @@ pub(crate) async fn lookup_host_with_cache(
861861
}
862862
}
863863

864-
Ok(merge_with_cache(resolved_addrs, cache))
864+
merge_with_cache(resolved_addrs, cache)
865865
} else {
866-
Ok(resolved_addrs)
867-
}
866+
resolved_addrs
867+
};
868+
ensure!(
869+
!addrs.is_empty(),
870+
"Could not find DNS resolutions for {hostname}:{port}. Check server hostname and your network"
871+
);
872+
Ok(addrs)
868873
}
869874

870875
/// Merges results received from DNS with cached results.

0 commit comments

Comments
 (0)