An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.

fix(MM-97): address code review feedback for resolveHandle

Critical fix: NXDOMAIN in HickoryTxtResolver now returns Ok(vec![]) instead
of Err, so DNS absence falls through to HTTP well-known → HandleNotFound (404)
rather than returning 500. Uses e.is_no_records_found() to distinguish
NXDOMAIN/NODATA from genuine infrastructure failures.

Also:
- Log tracing::warn! for non-UTF-8 TXT bytes instead of silently dropping
- Add ErrTxtResolver test double + dns_infrastructure_error_returns_500
to pin the 500 behavior for real DNS failures
- Add CapturingTxtResolver + dns_lookup_uses_atproto_prefix to verify
the _atproto. prefix is sent in DNS queries
- Add local_db_takes_priority_over_dns: seeds both DB and DNS with
different DIDs, asserts DB result wins
- Add body assertion to unknown_handle_with_empty_dns_response_returns_404

authored by malpercio.dev and committed by

Tangled df94c456 ef26a124

+121 -8
+16 -7
crates/relay/src/dns.rs
··· 61 61 name: &'a str, 62 62 ) -> Pin<Box<dyn Future<Output = Result<Vec<String>, DnsError>> + Send + 'a>> { 63 63 Box::pin(async move { 64 - let lookup = self 65 - .inner 66 - .txt_lookup(name) 67 - .await 68 - .map_err(|e| DnsError(e.to_string()))?; 64 + let lookup = match self.inner.txt_lookup(name).await { 65 + Ok(l) => l, 66 + // NXDOMAIN / NODATA — the name doesn't exist; this is the normal case for 67 + // handles that are not registered in DNS. Treat as empty, not as an error, 68 + // so the resolver falls through to the next step (HTTP well-known → 404). 69 + Err(e) if e.is_no_records_found() => return Ok(vec![]), 70 + Err(e) => return Err(DnsError(e.to_string())), 71 + }; 69 72 70 73 let mut results = Vec::new(); 71 74 for record in lookup.iter() { 72 75 for part in record.txt_data() { 73 - if let Ok(s) = std::str::from_utf8(part) { 74 - results.push(s.to_string()); 76 + match std::str::from_utf8(part) { 77 + Ok(s) => results.push(s.to_string()), 78 + Err(_) => { 79 + tracing::warn!( 80 + name, 81 + "TXT record contains non-UTF-8 bytes; skipping part" 82 + ); 83 + } 75 84 } 76 85 } 77 86 }
+105 -1
crates/relay/src/routes/resolve_handle.rs
··· 78 78 79 79 #[cfg(test)] 80 80 mod tests { 81 - use std::{future::Future, pin::Pin, sync::Arc}; 81 + use std::{future::Future, pin::Pin, sync::{Arc, Mutex}}; 82 82 83 83 use axum::{ 84 84 body::Body, ··· 113 113 } 114 114 } 115 115 116 + /// Always returns a transport-level error; simulates a broken DNS resolver. 117 + struct ErrTxtResolver; 118 + 119 + impl TxtResolver for ErrTxtResolver { 120 + fn txt_lookup<'a>( 121 + &'a self, 122 + _name: &'a str, 123 + ) -> Pin<Box<dyn Future<Output = Result<Vec<String>, DnsError>> + Send + 'a>> { 124 + Box::pin(async move { Err(DnsError("connection refused".to_string())) }) 125 + } 126 + } 127 + 128 + /// Records the last name it was queried with; always returns an empty vec. 129 + struct CapturingTxtResolver { 130 + last_name: Arc<Mutex<Option<String>>>, 131 + } 132 + 133 + impl TxtResolver for CapturingTxtResolver { 134 + fn txt_lookup<'a>( 135 + &'a self, 136 + name: &'a str, 137 + ) -> Pin<Box<dyn Future<Output = Result<Vec<String>, DnsError>> + Send + 'a>> { 138 + let captured = self.last_name.clone(); 139 + let name = name.to_string(); 140 + Box::pin(async move { 141 + *captured.lock().unwrap() = Some(name); 142 + Ok(vec![]) 143 + }) 144 + } 145 + } 146 + 116 147 // ── Well-known test doubles ──────────────────────────────────────────────── 117 148 118 149 struct FixedWellKnownResolver { ··· 188 219 assert_eq!(body["did"], did); 189 220 } 190 221 222 + // ── Local DB priority ───────────────────────────────────────────────────── 223 + 224 + #[tokio::test] 225 + async fn local_db_takes_priority_over_dns() { 226 + let state = test_state().await; 227 + let local_did = "did:plc:localuser123456789012345678"; 228 + let dns_did = "did:plc:dnsuser1234567890123456789"; 229 + seed_handle(&state.db, "alice.test.example.com", local_did).await; 230 + let state = state_with_dns(state, vec![format!("did={dns_did}")]); 231 + 232 + let response = app(state) 233 + .oneshot(resolve_handle_request("alice.test.example.com")) 234 + .await 235 + .unwrap(); 236 + 237 + assert_eq!(response.status(), StatusCode::OK); 238 + let body: serde_json::Value = serde_json::from_slice( 239 + &axum::body::to_bytes(response.into_body(), usize::MAX) 240 + .await 241 + .unwrap(), 242 + ) 243 + .unwrap(); 244 + assert_eq!(body["did"], local_did); 245 + } 246 + 191 247 // ── DNS fallback ────────────────────────────────────────────────────────── 192 248 193 249 #[tokio::test] ··· 212 268 } 213 269 214 270 #[tokio::test] 271 + async fn dns_infrastructure_error_returns_500() { 272 + let state = AppState { 273 + txt_resolver: Some(Arc::new(ErrTxtResolver)), 274 + ..test_state().await 275 + }; 276 + 277 + let response = app(state) 278 + .oneshot(resolve_handle_request("alice.external.example.com")) 279 + .await 280 + .unwrap(); 281 + 282 + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); 283 + let body: serde_json::Value = serde_json::from_slice( 284 + &axum::body::to_bytes(response.into_body(), usize::MAX) 285 + .await 286 + .unwrap(), 287 + ) 288 + .unwrap(); 289 + assert_eq!(body["error"]["code"], "INTERNAL_ERROR"); 290 + } 291 + 292 + #[tokio::test] 293 + async fn dns_lookup_uses_atproto_prefix() { 294 + let captured = Arc::new(Mutex::new(None::<String>)); 295 + let state = AppState { 296 + txt_resolver: Some(Arc::new(CapturingTxtResolver { 297 + last_name: captured.clone(), 298 + })), 299 + ..test_state().await 300 + }; 301 + 302 + app(state) 303 + .oneshot(resolve_handle_request("alice.example.com")) 304 + .await 305 + .unwrap(); 306 + 307 + let name = captured.lock().unwrap().clone().expect("txt_lookup not called"); 308 + assert_eq!(name, "_atproto.alice.example.com"); 309 + } 310 + 311 + #[tokio::test] 215 312 async fn dns_fallback_returns_404_when_txt_record_has_no_did_prefix() { 216 313 let state = test_state().await; 217 314 let state = state_with_dns(state, vec!["v=spf1 include:example.com ~all".to_string()]); ··· 262 359 .unwrap(); 263 360 264 361 assert_eq!(response.status(), StatusCode::NOT_FOUND); 362 + let body: serde_json::Value = serde_json::from_slice( 363 + &axum::body::to_bytes(response.into_body(), usize::MAX) 364 + .await 365 + .unwrap(), 366 + ) 367 + .unwrap(); 368 + assert_eq!(body["error"]["code"], "HANDLE_NOT_FOUND"); 265 369 } 266 370 267 371 // ── HTTP well-known fallback ───────────────────────────────────────────────