dns: distinguish "this name does not resolve" from "this resolver failed"

crates/flint-dns/src/codec.rs

@@ -101,9 +101,10 @@ pub fn parse_response_with_id(buf: &[u8], expected_id: u16) -> Result<Vec<IpAddr
     parse_response(buf)
 }
 
-/// Parse a DNS response, returning the A/AAAA addresses in its answer section. Errors on a truncated
-/// message, a query (not a response), or a non-zero RCODE. An empty answer list is *not* an error here
-/// (the validation layer decides what an empty/poisoned answer means).
+/// Parse a DNS response, returning the A/AAAA addresses in its answer section. Errors on a query
+/// (not a response), a non-zero RCODE, or a truncated answer — either a message that ended before a
+/// field it declared, or one whose **TC flag** says the server cut it short. An empty answer list is
+/// *not* an error here (the validation layer decides what an empty/poisoned answer means).
 ///
 /// Does **not** check the transaction ID — safe for DoH/DoT (the channel binds the response) but not
 /// for plaintext; see [`parse_response_with_id`].
@@ -119,6 +120,17 @@ pub fn parse_response(buf: &[u8]) -> Result<Vec<IpAddr>, DnsError> {
     if rcode != 0 {
         return Err(DnsError::Rcode(rcode));
     }
+    // TC: the answer did not fit and the server cut it short (RFC 1035 §4.1.1). Checked *after* the
+    // RCODE so a truncated NXDOMAIN still reports the more specific NXDOMAIN.
+    //
+    // Rejecting rather than parsing what arrived: a partial answer looks like a complete one, and if
+    // truncation happened to remove every address record we would report "this name has no address"
+    // — which callers now act on (see `flint_dns::indicts_resolver`) and would be exactly backwards,
+    // since the resolver is the thing that failed to serve us. We do no TCP-retry, so a resolver that
+    // truncates cannot answer this query and should lose to one that can.
+    if flags & 0x0200 != 0 {
+        return Err(DnsError::Truncated);
+    }
     let qdcount = u16::from_be_bytes([buf[4], buf[5]]) as usize;
     let ancount = u16::from_be_bytes([buf[6], buf[7]]) as usize;
 
@@ -263,6 +275,35 @@ mod tests {
         assert_eq!(ips, vec!["93.184.216.34".parse::<IpAddr>().unwrap()]);
     }
 
+    #[test]
+    fn a_truncated_answer_is_rejected_rather_than_parsed_partially() {
+        // A TC response is syntactically fine and carries *some* records, so without this check it
+        // would parse as a success and silently return a partial answer.
+        let mut resp = build_response(
+            "example.com",
+            TYPE_A,
+            0,
+            &[(TYPE_A, vec![93, 184, 216, 34])],
+        );
+        resp[2] |= 0x02; // set TC in the high flags byte
+        assert_eq!(parse_response(&resp), Err(DnsError::Truncated));
+
+        // Clearing it again parses normally — proving TC is what rejected it, not the fixture.
+        resp[2] &= !0x02;
+        assert_eq!(
+            parse_response(&resp).unwrap(),
+            vec!["93.184.216.34".parse::<IpAddr>().unwrap()]
+        );
+    }
+
+    #[test]
+    fn a_truncated_nxdomain_still_reports_nxdomain() {
+        // The RCODE is the more specific fact, so it wins over truncation.
+        let mut resp = build_response("nope.example", TYPE_A, 3, &[]);
+        resp[2] |= 0x02;
+        assert_eq!(parse_response(&resp), Err(DnsError::Rcode(3)));
+    }
+
     #[test]
     fn rejects_an_overlong_label() {
         let long = "a".repeat(64);

crates/flint-dns/src/lib.rs

@@ -70,6 +70,63 @@ const DEFAULT_WINDOW: usize = 16;
 /// instead of hanging on the slowest resolver.
 const ATTEMPT_TIMEOUT: Duration = Duration::from_secs(5);
 
+/// Whether a resolve error indicts the **resolver** rather than the name.
+///
+/// `false` means the resolver did its job and the name simply does not resolve — NXDOMAIN, or an
+/// answer carrying no usable address. A caller must not read that as reason to distrust the resolver:
+/// users mistype hosts and visit dead domains constantly, and discarding a working resolver every time
+/// one does would throw away a good strategy for an entirely normal event.
+///
+/// `true` means the resolver could not be reached, timed out, or answered in a way that cannot be
+/// believed (SERVFAIL, a malformed or mismatched response, bogon-only records). Those *are* grounds to
+/// stop using it and pick another.
+///
+/// This distinction is the whole reason the resolve paths map failures onto meaningful
+/// [`io::ErrorKind`]s instead of flattening everything to `Other` — without it a caller sees one
+/// undifferentiated error and cannot tell a broken network from a typo.
+///
+/// **[`Kind::System`] is the exception.** `getaddrinfo` reports "no such host" and "the network is
+/// down" through the same opaque failure, so a system-resolver error is reported as indicting. That
+/// errs toward re-selecting, which is the safe direction: moving off the OS resolver is cheap, and it
+/// is the right move whenever the network really has changed.
+pub fn indicts_resolver(err: &io::Error) -> bool {
+    !matches!(
+        err.kind(),
+        // The resolver answered and the name has no address.
+        io::ErrorKind::NotFound
+        // We could not even encode the query, so nothing was sent and no resolver was involved.
+        // Blaming one for our own bad input would churn selection over a caller error.
+            | io::ErrorKind::InvalidInput
+    )
+}
+
+/// Map a wire/codec failure onto an [`io::Error`] whose kind carries the distinction
+/// [`indicts_resolver`] depends on.
+fn codec_err(e: codec::DnsError) -> io::Error {
+    let kind = match e {
+        // The resolver answered authoritatively: this name has no address. Not its fault.
+        codec::DnsError::Rcode(3) => io::ErrorKind::NotFound,
+        // A name we could not even encode is our bug, not the resolver's.
+        codec::DnsError::BadName => io::ErrorKind::InvalidInput,
+        // Everything else — SERVFAIL/REFUSED, truncated, not-a-response, a mismatched transaction ID —
+        // is the resolver failing to give a usable answer.
+        _ => io::ErrorKind::InvalidData,
+    };
+    io::Error::new(kind, e)
+}
+
+/// Map an answer-validation failure the same way.
+fn validate_err(e: validate::ValidateError) -> io::Error {
+    let kind = match e {
+        // Answered, but with nothing usable — the name, not the resolver.
+        validate::ValidateError::Empty => io::ErrorKind::NotFound,
+        // Bogons only. The resolver answered and the answer is a lie, which is very much about the
+        // resolver (or whoever is speaking for it).
+        validate::ValidateError::Poisoned => io::ErrorKind::InvalidData,
+    };
+    io::Error::new(kind, e)
+}
+
 /// Resolve `name`/`qtype` through a single `resolver`: reach it over whatever transport its
 /// [`Kind`] names, run the query, parse, and validate. Returns the validated public addresses, or an
 /// `io::Error` (which the smart-dialer funnels into the race's per-resolver failures).
@@ -102,34 +159,34 @@ pub async fn resolve_one_with(
 ) -> io::Result<Vec<IpAddr>> {
     let answers = match resolver.kind {
         Kind::Doh => {
-            let query = codec::build_query(name, qtype).map_err(io::Error::other)?;
+            let query = codec::build_query(name, qtype).map_err(codec_err)?;
             let stream = flint_dial::dial(&tls_strategy(resolver, policy)?).await?;
             let response = doh::query(stream, &resolver.host, &resolver.path, &query).await?;
-            codec::parse_response(&response).map_err(io::Error::other)?
+            codec::parse_response(&response).map_err(codec_err)?
         }
         Kind::Dot => {
             let id = random_id()?;
-            let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?;
+            let query = codec::build_query_with_id(name, qtype, id).map_err(codec_err)?;
             let stream = flint_dial::dial(&tls_strategy(resolver, policy)?).await?;
             let response = plain::query_stream(stream, &query).await?;
-            codec::parse_response_with_id(&response, id).map_err(io::Error::other)?
+            codec::parse_response_with_id(&response, id).map_err(codec_err)?
         }
         Kind::Tcp => {
             let id = random_id()?;
-            let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?;
+            let query = codec::build_query_with_id(name, qtype, id).map_err(codec_err)?;
             let stream = tokio::net::TcpStream::connect(resolver.target).await?;
             let response = plain::query_stream(stream, &query).await?;
-            codec::parse_response_with_id(&response, id).map_err(io::Error::other)?
+            codec::parse_response_with_id(&response, id).map_err(codec_err)?
         }
         Kind::Udp => {
             let id = random_id()?;
-            let query = codec::build_query_with_id(name, qtype, id).map_err(io::Error::other)?;
+            let query = codec::build_query_with_id(name, qtype, id).map_err(codec_err)?;
             let response = plain::query_udp(resolver.target, &query).await?;
-            codec::parse_response_with_id(&response, id).map_err(io::Error::other)?
+            codec::parse_response_with_id(&response, id).map_err(codec_err)?
         }
         Kind::System => system_lookup(name, qtype).await?,
     };
-    validate::validate_answers(answers).map_err(io::Error::other)
+    validate::validate_answers(answers).map_err(validate_err)
 }
 
 /// The TLS strategy for a resolver whose kind is known to be TLS-based.
@@ -291,6 +348,47 @@ pub async fn resolve_cached_with(
 mod tests {
     use super::*;
 
+    #[test]
+    fn a_name_that_does_not_resolve_does_not_indict_the_resolver() {
+        // The distinction that matters: a user mistyping a host must not cost a working resolver.
+        for e in [
+            codec_err(codec::DnsError::Rcode(3)),         // NXDOMAIN
+            validate_err(validate::ValidateError::Empty), // answered, no usable records
+        ] {
+            assert_eq!(e.kind(), io::ErrorKind::NotFound, "{e}");
+            assert!(!indicts_resolver(&e), "{e} must not blame the resolver");
+        }
+    }
+
+    #[test]
+    fn a_resolver_that_misbehaves_is_indicted() {
+        for e in [
+            codec_err(codec::DnsError::Rcode(2)), // SERVFAIL
+            codec_err(codec::DnsError::Truncated),
+            codec_err(codec::DnsError::NotAResponse),
+            codec_err(codec::DnsError::IdMismatch { got: 1, want: 2 }),
+            // Bogons only: the resolver answered, and the answer is a lie.
+            validate_err(validate::ValidateError::Poisoned),
+        ] {
+            assert!(indicts_resolver(&e), "{e} should blame the resolver");
+        }
+
+        // Transport failures are the clearest case of all — they never reach the codec.
+        for kind in [io::ErrorKind::TimedOut, io::ErrorKind::ConnectionRefused] {
+            assert!(indicts_resolver(&io::Error::new(kind, "unreachable")));
+        }
+    }
+
+    #[test]
+    fn an_unencodable_name_is_our_bug_and_indicts_nobody() {
+        // Nothing was ever sent, so no resolver was involved. Blaming one would discard a working
+        // strategy over a caller error — the same class of mistake as blaming it for a typo'd domain,
+        // which is the whole point of this predicate.
+        let e = codec_err(codec::DnsError::BadName);
+        assert_eq!(e.kind(), io::ErrorKind::InvalidInput);
+        assert!(!indicts_resolver(&e), "a caller-input error blames nobody");
+    }
+
     #[tokio::test]
     async fn resolve_on_an_empty_pool_fails() {
         // No network: an empty pool races nothing → AllFailed{0}. Proves resolve still funnels an