[liquidsec]

Closes #176.

Problem: With no resolvers passed, system resolvers were formatted as "{ip}:53" and parsed back. Unbracketed IPv6 doesn't parse as a socket address, so parse_resolver fell back to a bare IP: 2001:db8::1 became [2001:db8::1:53]:53, and addresses with no zero run failed to parse.

Fix: Build the SocketAddrs directly with SocketAddr::new(ip, 53) instead of round-tripping through strings.

Tests: New unit test system_resolver_addrs_keep_ipv6_addresses. fmt and clippy pass.

[github-actions]

DNS Resolver Benchmark

this PR @ 40d8f85 against dev @ 15c9f79, base → head, median of 3 interleaved rounds. Bold is significant across all 56 comparisons (Benjamini-Hochberg at 1%); the rest is noise.

regime	qps	unanswered	p99	attempts/query
ceiling/1-resolver	24,254 → 24,595 (+1.4%)	0.00% → 0.00%	0.822s → 0.811s (-1.4%)	1.00 → 1.00
pool/32-resolvers	24,127 → 24,615 (+2.0%)	0.00% → 0.00%	0.827s → 0.810s (-2.0%)	1.00 → 1.00
pool/32-persistent	27,935 → 27,545 (-1.4%)	0.00% → 0.00%	0.714s → 0.724s (+1.4%)	1.00 → 1.00
impaired/10%-loss	200 → 200 (+0.0%)	0.00% → 0.00%	8.132s → 8.131s (-0.0%)	1.11 → 1.11
impaired/20%-REFUSED	3,434 → 3,422 (-0.3%)	0.00% → 0.00%	0.582s → 0.584s (+0.3%)	1.24 → 1.24
impaired/50ms-latency	620 → 622 (+0.2%)	0.00% → 0.00%	3.223s → 3.217s (-0.2%)	1.00 → 1.00
impaired/20%-then-2.5%-dead-names	252 → 235 (-6.8%)	5.43% → 5.43%	10.331s → 10.893s (+5.4%)	1.05 → 1.05

All metrics

regime	p50	p100	sockets	conntrack
ceiling/1-resolver	0.439s → 0.440s (+0.4%)	0.824s → 0.813s (-1.4%)	42 → 42 (+0.0%)	16,955 → 12,639 (-25%)
pool/32-resolvers	0.438s → 0.439s (+0.2%)	0.829s → 0.812s (-2.0%)	74 → 74 (+0.0%)	19,209 → 19,005 (-1.1%)
pool/32-persistent	0.384s → 0.393s (+2.3%)	0.716s → 0.726s (+1.4%)	42 → 42 (+0.0%)	29 → 30 (+3.4%)
impaired/10%-loss	4.274s → 4.273s (-0.0%)	9.985s → 9.984s (-0.0%)	46 → 46 (+0.0%)	2,070 → 1,239 (-40%)
impaired/20%-REFUSED	0.415s → 0.414s (-0.2%)	0.582s → 0.584s (+0.3%)	46 → 46 (+0.0%)	2,258 → 2,236 (-1.0%)
impaired/50ms-latency	1.839s → 1.838s (-0.1%)	3.223s → 3.218s (-0.2%)	46 → 46 (+0.0%)	481 → 1,776 (+269%)
impaired/20%-then-2.5%-dead-names	3.901s → 3.828s (-1.9%)	11.888s → 12.753s (+7.3%)	39 → 39 (+0.0%)	268 → 0 (-100%)

[en0f]

Reviewed at 40d8f85.

🔴 Validation is stated, not shown

The Tests line names the new unit test and says fmt and clippy pass, with no command or output, and the body is not on the org PR template. The org git standard (§ Pull requests) asks for the commands you ran and their real output, and nothing shows the fix at the user-facing boundary: a client built with no resolvers on a host whose system resolver is IPv6.

Put the body on the template and paste the AGENTS.md Tests and Lint commands with their output: cargo test --all --locked -- --include-ignored with dnsmasq and BIND9 up, both pytest runs, cargo fmt --all -- --check, cargo clippy --all-targets --all-features --locked -- -D warnings, ruff check and ruff format --check. Add one boundary run on a host or container whose /etc/resolv.conf has nameserver 2001:db8::1: python -c 'import blastdns; print(blastdns.Client([]).resolvers)' should print ['[2001:db8::1:53]:53'] on dev and ['[2001:db8::1]:53'] on this branch. On that host test_get_system_resolvers in blastdns/tests/test_client.py fails on its own, because it asserts no : in a resolver. That is pre-existing, so say so rather than skip the suite.

🟢 Fixes the round trip instead of patching the format string

SocketAddr::new(ip, 53) removes the string round trip that caused #176, rather than bracketing IPv6 in the format string. User-supplied resolvers still go through parse_resolver unchanged, IPv4 behaves the same, and the new test fails if the old format-then-parse code is put back inside system_resolver_addrs.

Reproduce this review

# save the script below as verify-177.sh, then:
bash verify-177.sh blacklanternsecurity/blastdns 40d8f852162ba3c24da01c453b2b241a0e368f6c
# expected: SUMMARY 6 PASS, 3 FAIL, 0 SKIP, 0 BROKEN
# control:  bash verify-177.sh blacklanternsecurity/blastdns 15c9f792bc649f14bffe2b29ee61cbc219c88849   -> V01=FAIL V02=FAIL V03=FAIL

#!/usr/bin/env bash
# verify-177.sh: reproducible checks for blacklanternsecurity/blastdns#177
# Usage: bash verify-177.sh <owner/repo> <sha> [<control-patch>]
# Changes nothing outside a mktemp directory and nothing on GitHub (PR-VF-02).
# Uses git, grep, awk and the host rustc (std only, a snippet written by the
# reviewer; no repository code is built or run). Nothing is fetched, so there is
# no tool to pin.
#
# CONTROL: sha 15c9f792bc649f14bffe2b29ee61cbc219c88849
# CONTROL-EXPECT: V01=FAIL V02=FAIL V03=FAIL
set -uo pipefail

REPO="${1:?owner/repo}"; SHA="${2:?sha}"; PATCH="${3:-}"
PASS=0; FAIL=0; SKIP=0; BROKEN=0
pass()   { echo "PASS $1 $2"; PASS=$((PASS+1)); }
fail()   { echo "FAIL $1 $2 -- $3"; FAIL=$((FAIL+1)); }
skip()   { echo "SKIP $1 $2 -- $3"; SKIP=$((SKIP+1)); }
broken() { echo "BROKEN $1 $2 -- $3"; BROKEN=$((BROKEN+1)); }
summary() { echo "SUMMARY $PASS PASS, $FAIL FAIL, $SKIP SKIP, $BROKEN BROKEN"; [ "$FAIL" -eq 0 ] && [ "$BROKEN" -eq 0 ]; }

echo "script_sha256 $(shasum -a 256 "$0" | cut -d' ' -f1)"
echo "git $(git --version | cut -d' ' -f3)"
echo "rustc $(rustc --version 2>/dev/null | cut -d' ' -f2 || echo absent)"

WORK="$(mktemp -d)"; trap 'rm -rf "$WORK"' EXIT
export GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_TERMINAL_PROMPT=0
export GIT_CONFIG_COUNT=2 GIT_CONFIG_KEY_0=credential.helper GIT_CONFIG_VALUE_0= \
       GIT_CONFIG_KEY_1=credential.helper GIT_CONFIG_VALUE_1='!gh auth git-credential'
GIT=(git)
"${GIT[@]}" clone --quiet --filter=blob:none --no-checkout "https://github.com/$REPO.git" "$WORK/src" || { broken V00 "clone" "clone failed"; summary; exit 1; }
cd "$WORK/src"
"${GIT[@]}" cat-file -e "$SHA^{commit}" 2>/dev/null || "${GIT[@]}" fetch --quiet origin "+refs/pull/*/head:refs/remotes/pr/*" || true
"${GIT[@]}" checkout --quiet --detach "$SHA" || { broken V00 "checkout $SHA" "checkout failed"; summary; exit 1; }
[ "$(git rev-parse HEAD)" = "$SHA" ] || { broken V00 "HEAD is $SHA" "HEAD mismatch"; summary; exit 1; }
if [ -n "$PATCH" ]; then git apply "$PATCH" || { broken V00 "apply control patch" "git apply failed"; summary; exit 1; }; fi

C=src/client.rs; U=src/utils.rs
[ -f "$C" ] && [ -f "$U" ] || { broken V00 "source files present" "src/client.rs or src/utils.rs missing"; summary; exit 1; }

# body of a Rust fn: from its `fn NAME` line to the next #[test]/#[tokio::test] or EOF
fnbody() { awk -v n="fn $2(" 'index($0,n){on=1;next} on&&/#\[(tokio::)?test\]/{exit} on{print}' "$1"; }

# ---- checks: each line states the desired property --------------------------
# non-test source of a file: everything before its #[cfg(test)]
nontest() { awk '/^#\[cfg\(test\)\]/{exit} {print}' "$1"; }

# V01 no non-test code builds a resolver string by appending ":53" to an IP
pat='format!\("\{[A-Za-z_]*\}:53"'
hit="$( { nontest "$C"; nontest "$U"; } | grep -nE "$pat" | head -1)"
if [ -n "$hit" ]; then fail V01 "system resolver IPs are never formatted into \"{ip}:53\" strings" "$hit"
else pass V01 "system resolver IPs are never formatted into \"{ip}:53\" strings"; fi

# V02 non-test code hands system IPs to system_resolver_addrs (no parse_resolver round trip)
n="$( { nontest "$C"; nontest "$U"; } | grep -v 'fn system_resolver_addrs' | grep -c 'system_resolver_addrs(' )"
if [ "$n" -ge 1 ]; then pass V02 "system IPs reach system_resolver_addrs from non-test code"
else fail V02 "system IPs reach system_resolver_addrs from non-test code" "no non-test call"; fi

# V03 the helper builds each address with SocketAddr::new(ip, 53)
if fnbody "$U" system_resolver_addrs | grep -q 'SocketAddr::new(ip, 53)'; then
  pass V03 "system_resolver_addrs builds SocketAddr::new(ip, 53)"
else fail V03 "system_resolver_addrs builds SocketAddr::new(ip, 53)" "helper absent or different"; fi

# V04/V05 the defect mechanism, in std alone (independent of the SHA)
if command -v rustc >/dev/null 2>&1; then
  cat > "$WORK/m.rs" <<'RS'
use std::net::{IpAddr, SocketAddr};
use std::str::FromStr;
fn main() {
    // old path: format!("{}:53", ip), then SocketAddr parse, then bare-IP fallback
    let a = "2001:db8::1:53";
    let sa = SocketAddr::from_str(a).is_err();
    let ip = IpAddr::from_str(a).ok();
    let wrong = ip.map(|i| i != IpAddr::from_str("2001:db8::1").unwrap()).unwrap_or(false);
    println!("V04 {}", sa && wrong);
    let b = "2001:db8:1:2:3:4:5:6:53";
    println!("V05 {}", SocketAddr::from_str(b).is_err() && IpAddr::from_str(b).is_err());
    // new path keeps the address
    let n = IpAddr::from_str("2001:db8:1:2:3:4:5:6").unwrap();
    let s = SocketAddr::new(n, 53);
    println!("V06 {}", s.ip() == n && s.port() == 53 && s.to_string() == "[2001:db8:1:2:3:4:5:6]:53");
}
RS
  if rustc -O -o "$WORK/m" "$WORK/m.rs" >/dev/null 2>&1 && OUT="$("$WORK/m")"; then
    for id in V04 V05 V06; do
      case $id in
        V04) d="old path: \"2001:db8::1:53\" is not a SocketAddr but is a different IpAddr (2001:db8::1:53)";;
        V05) d="old path: \"2001:db8:1:2:3:4:5:6:53\" parses as neither, so client creation fails";;
        V06) d="new path: SocketAddr::new keeps a full-length IPv6 address, displayed bracketed";;
      esac
      if echo "$OUT" | grep -qx "$id true"; then pass $id "$d"; else fail $id "$d" "$(echo "$OUT" | grep "^$id")"; fi
    done
  else broken V04 "std snippet compiles and runs" "rustc failed"; fi
else
  skip V04 "old path misreads 2001:db8::1" "rustc absent"; skip V05 "old path rejects full IPv6" "rustc absent"; skip V06 "new path keeps IPv6" "rustc absent"
fi

# V07 some test drives an empty resolver list with an IPv6 system address and asserts the result
#     (per test body across src/*.rs: an empty list literal and a 2001:db8 address in the same test)
hit="$(awk '
  function flush(){ if (body ~ /(Vec::new\(\)|vec!\[\])/ && body ~ /2001:db8/ && body ~ /assert/) print name; body=""; name="" }
  /#\[(tokio::)?test\]/ { flush(); intest=1; next }
  intest && name=="" && /fn [a-z_0-9]+\(/ { match($0,/fn [a-z_0-9]+/); name=FILENAME":"substr($0,RSTART+3,RLENGTH-3) }
  intest { body = body "\n" $0 }
  END { flush() }' src/*.rs | head -1)"
if [ -n "$hit" ]; then pass V07 "a test feeds an empty resolver list and an IPv6 system address through the list-building code"
else fail V07 "a test feeds an empty resolver list and an IPv6 system address through the list-building code" "none: the IPv6 test calls only system_resolver_addrs, so restoring the old branch is unguarded"; fi

# V08 the existing empty-resolvers client test asserts on the resulting addresses
body="$(fnbody "$C" empty_resolvers_uses_system)"
if echo "$body" | grep -qE '\.resolvers\(\)'; then pass V08 "empty_resolvers_uses_system asserts on the resolver addresses"
else fail V08 "empty_resolvers_uses_system asserts on the resolver addresses" "only checks that it does not panic"; fi

# V09 the Python system-resolver test accepts IPv6 resolver addresses
if grep -nE 'assert ":" not in resolver' blastdns/tests/test_client.py >/dev/null 2>&1; then
  fail V09 "test_get_system_resolvers accepts IPv6 addresses" "$(grep -nE 'assert ":" not in resolver' blastdns/tests/test_client.py | head -1)"
else pass V09 "test_get_system_resolvers accepts IPv6 addresses"; fi
# ------------------------------------------------------------------------------
summary