From a21472730e7a1329d26201d1dfa2c6b0f3ccf521 Mon Sep 17 00:00:00 2001 From: rays Date: Mon, 19 Oct 2026 18:12:55 +0000 Subject: [PATCH] A feed is read up to 50 MB (#295) resp.bytes() read any size whole: an episode's MP3 given as a feed was held in memory at every check. Feeds here are a few MB at most. Co-Authored-By: Claude Opus 5.5 --- crates/directory/CHANGELOG.md | 4 ++++ crates/feed/src/lib.rs | 27 ++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/directory/CHANGELOG.md b/crates/directory/CHANGELOG.md index df98b84..65e9ba5 100644 --- a/crates/directory/CHANGELOG.md +++ b/crates/directory/CHANGELOG.md @@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A feed's items written in Markdown show formatted on its page, from a Markdown description or RSS's `` (#284). +### Fixed + +- A listed feed whose address serves something larger than 50 MB is refused as too large, instead of read whole into memory at every check (#295). + ### Security - ammonia, which cleans publishers' HTML, updated to 4.2.4 for three advisories, among them XSS through SVG animation tags; rustls to 0.23.46 for five and h2 to 0.4.20 for one (#281). diff --git a/crates/feed/src/lib.rs b/crates/feed/src/lib.rs index 5b7da37..27f82a9 100644 --- a/crates/feed/src/lib.rs +++ b/crates/feed/src/lib.rs @@ -114,6 +114,23 @@ pub fn redact_text(text: &str) -> String { out } +/// The most of a feed or page read. The largest feeds here are a few MB; without a limit an +/// episode's MP3 given as a feed was read whole into memory, at every check (#295). +const MAX_BODY: usize = 50 << 20; + +/// A response's body, up to `MAX_BODY`: past it, an error, and the rest is never read. +async fn read_capped(mut resp: reqwest::Response) -> Result> { + if resp.content_length().is_some_and(|n| n > MAX_BODY as u64) { + return Err(anyhow!("larger than {} MB, too large to be a feed", MAX_BODY >> 20)); + } + let mut out = Vec::new(); + while let Some(c) = resp.chunk().await? { + out.extend_from_slice(&c); + anyhow::ensure!(out.len() <= MAX_BODY, "larger than {} MB, too large to be a feed", MAX_BODY >> 20); + } + Ok(out) +} + #[tracing::instrument(skip_all, fields(url = %redact(feed_url), http.response.body.size))] pub async fn fetch( client: &reqwest::Client, @@ -165,7 +182,7 @@ pub async fn fetch( |h: reqwest::header::HeaderName| resp.headers().get(&h).and_then(|v| v.to_str().ok()).map(str::to_owned); let etag = header(ETAG); let last_modified = header(LAST_MODIFIED); - let bytes = resp.bytes().await.context("reading body")?.to_vec(); + let bytes = read_capped(resp).await.context("reading body")?; tracing::Span::current().record("http.response.body.size", bytes.len()); FETCHED_BYTES.fetch_add(bytes.len() as u64, std::sync::atomic::Ordering::Relaxed); return Ok((Fetched::Body { bytes, etag, last_modified }, moved)); @@ -511,7 +528,7 @@ pub async fn find_feed(client: &reqwest::Client, url: &str) -> Result { let resp = client.get(&u).timeout(std::time::Duration::from_secs(30)).send().await.context("connecting")?; anyhow::ensure!(resp.status().is_success(), "HTTP {}", resp.status()); let base = resp.url().clone(); - anyhow::Ok((base, resp.bytes().await.context("reading")?)) + anyhow::Ok((base, read_capped(resp).await.context("reading")?)) }; let is_feed = |b: &[u8]| is_opml(b) || parse(b).is_ok(); let (base, body) = read(url.to_owned()).await.with_context(|| format!("could not read {url}"))?; @@ -544,7 +561,7 @@ pub async fn site_icon(client: &reqwest::Client, site: &str) -> Option { // The icon a page names can be gone: antirez.com names /images/favicon.png, a 404, while its // /favicon.ico is there. Stored unchecked, it was a broken image that was never looked up again. if resp.status().is_success() - && let Ok(page) = resp.bytes().await + && let Ok(page) = read_capped(resp).await && let Some(href) = page_icon(&page) && let Ok(url) = base.join(&href) && is_image(client, url.as_str()).await @@ -1331,6 +1348,8 @@ mod tests { "/chain" => { "HTTP/1.1 308 Permanent Redirect\r\nLocation: /tmp\r\nContent-Length: 0\r\n\r\n".to_owned() } + // An episode's file given as a feed: its size said, its body never sent. + "/huge" => "HTTP/1.1 200 OK\r\nContent-Length: 600000000\r\n\r\n".to_owned(), "/loop" => { "HTTP/1.1 301 Moved Permanently\r\nLocation: /loop\r\nContent-Length: 0\r\n\r\n".to_owned() } @@ -1356,6 +1375,8 @@ mod tests { assert!(matches!(got, super::Fetched::Body { .. })); assert_eq!(moved, Some(format!("{base}/new"))); assert_eq!(get("/tmp").await.unwrap().1, None); // 302: for now + let huge = get("/huge").await.err().map(|e| format!("{e:#}")).unwrap_or_default(); + assert!(huge.contains("too large to be a feed"), "{huge}"); assert_eq!(get("/chain").await.unwrap().1, None); // 308 then 302: not for good assert_eq!(get("/new").await.unwrap().1, None); // never moved let looped = get("/loop").await.err().unwrap().to_string();