From 335e6519a6942a325fb8fcffcc9ad4590cd3e2a6 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Fri, 11 Sep 2026 20:59:54 -0400 Subject: [PATCH] fix(serve): bound how much of a relay's error body is read The body was buffered whole and then truncated to 512 characters, so a relay answering an endless page was held in memory until the request timeout. Co-Authored-By: Claude Opus 5 (1M context) Change-Id: I1b952dcaa48d20c539d39069f205fcdf192b80fe --- crates/didbot-serve/src/relay.rs | 85 +++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/crates/didbot-serve/src/relay.rs b/crates/didbot-serve/src/relay.rs index 811dea61..f62749a6 100644 --- a/crates/didbot-serve/src/relay.rs +++ b/crates/didbot-serve/src/relay.rs @@ -72,6 +72,12 @@ pub const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); /// How much of a non-success response body [`RelayCrawlError::Refused`] /// keeps: a relay answering with a large page must not flood the log this /// failure is reported to. +/// +/// How much is *read* to get it is +/// [`didbot_http::MAX_JSON_BODY`], the bound this +/// workspace holds any body from a server it does not operate to. Past it the +/// message says the body went unread, which a relay answering an endless +/// error page earns. const MAX_ERROR_BODY: usize = 512; /// Body of both `requestCrawl`'s and `notifyOfUpdate`'s request — the two @@ -192,8 +198,17 @@ impl RelayClient { })?; let status = response.status(); if !status.is_success() { - let text = response.text().await.unwrap_or_default(); - let body: String = text.chars().take(MAX_ERROR_BODY).collect(); + // Bounded on the way in, then truncated. The truncation is what + // keeps the log readable; the bound is what decides how much of a + // relay's answer this process ever holds, and a relay is a server + // nobody here operates. + let body = match didbot_http::read_bounded(response, didbot_http::MAX_JSON_BODY).await { + Ok(bytes) => String::from_utf8_lossy(&bytes) + .chars() + .take(MAX_ERROR_BODY) + .collect(), + Err(err) => format!(""), + }; return Err(RelayCrawlError::Refused { relay: self.base_url.clone(), status: status.as_u16(), @@ -290,6 +305,72 @@ mod tests { } } + /// A response body that declares no length and runs well past the bound + /// a relay's answer is read against. + struct Oversized { + remaining: usize, + } + + impl futures_core::Stream for Oversized { + type Item = Result; + + fn poll_next( + mut self: std::pin::Pin<&mut Self>, + _cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + const CHUNK: usize = 64 * 1024; + if self.remaining == 0 { + return std::task::Poll::Ready(None); + } + let take = self.remaining.min(CHUNK); + self.remaining -= take; + std::task::Poll::Ready(Some(Ok(bytes::Bytes::from(vec![b'x'; take])))) + } + } + + /// The refusal is still reported, and the page it came with is not held + /// in memory to report it. A relay is a server nobody here operates, and + /// the only bound on a `text()` of its error body is how long the request + /// may take. + #[tokio::test] + async fn a_refusal_with_an_outsized_body_is_reported_without_reading_it() { + let app = Router::new().route( + &format!("/xrpc/{REQUEST_CRAWL_NSID}"), + post(|| async { + ( + StatusCode::SERVICE_UNAVAILABLE, + axum::body::Body::from_stream(Oversized { + remaining: didbot_http::MAX_JSON_BODY * 2, + }), + ) + }), + ); + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("loopback binds"); + let addr = listener.local_addr().expect("bound listener has an addr"); + tokio::spawn(async move { + axum::serve(listener, app).await.expect("fake relay serves"); + }); + + let client = RelayClient::new(format!("http://{addr}")); + let err = client + .request_crawl("pds.example.com") + .await + .expect_err("a 503 is a refusal"); + match err { + RelayCrawlError::Refused { status, body, .. } => { + assert_eq!(status, 503); + assert!( + !body.contains("xxxx"), + "a body past the bound was read anyway: {body}" + ); + assert!(body.contains("exceeds"), "body was {body}"); + } + other => panic!("expected Refused, got {other:?}"), + } + } + #[tokio::test] async fn an_unreachable_relay_is_reported_rather_than_hanging_or_panicking() { // A bound-then-dropped listener's port has nothing behind it, so the -- 2.51.2