From a20fade8ad839ad4bcdcb90e58ec61712b9d1cf5 Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 23 Oct 2026 15:00:29 +0000 Subject: [PATCH] OPML uploads are capped at 2,000 feeds and checked by address once (#338) subscribe_opml, reachable by anyone signed in through POST /api/opml, held the catalogue lock while, for each outline, it searched every subscription and the whole catalogue for its address and probed unique_slug's suffixes in order. 2 MB holds about 40,000 outlines, so one upload was quadratic work with every other catalogue change waiting, and its feeds stayed in the catalogue for good. opml_feeds lists a document's feeds and refuses more than MAX_LISTED, the limit a subscribed list already has (#337): a 400 from the web, a message from `ipx opml`. subscribe_opml takes that list and looks addresses up in a map built once, the first feed with an address winning as the search in order did. Co-Authored-By: Claude Opus 5.5 --- crates/ipx/CHANGELOG.md | 1 + crates/ipx/src/main.rs | 59 +++++++++++++++++++++++++++++++---------- crates/ipx/src/web.rs | 3 ++- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index 4fd964e..9a80dff 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Live updates about a feed go only to the people who read it, and admins. Everyone signed in had every feed's, errors included, which carried a paid feed's address with its key (#325). - An open page stops getting live updates within half a minute of its account being removed. They went on until the page was closed (#335). - Only someone subscribed to a feed, or an admin, may mark it private. Anyone signed in could keep anyone's feed, or a whole OPML's, out of the directory, and only an admin could undo it (#333). +- An OPML upload may add up to 2,000 feeds; one with more is refused and adds none. Importing tens of thousands held up every change to the feed list for minutes and left them all in it (#338). ## [1.4.0] - 2026-10-19 diff --git a/crates/ipx/src/main.rs b/crates/ipx/src/main.rs index e0ce05d..38ed81e 100644 --- a/crates/ipx/src/main.rs +++ b/crates/ipx/src/main.rs @@ -1022,11 +1022,25 @@ async fn import(ctx: &Ctx, file: &std::path::Path) -> Result<()> { .find(|u| u.is_admin) .ok_or_else(|| anyhow::anyhow!("no admin account to subscribe: ipx user add --admin"))?; let doc = feed::read_opml(&text).map_err(|e| anyhow::anyhow!("{} is not OPML: {e}", file.display()))?; - let (added, had) = subscribe_opml(ctx, &doc, admin.id).await?; + let (added, had) = subscribe_opml(ctx, opml_feeds(&doc)?, admin.id).await?; println!("subscribed {} to {added} feed(s); {had} already there", admin.name); Ok(()) } +/// The feeds an OPML document lists, up to as many as a subscribed list may bring in: each becomes +/// a feed in the catalogue, scanned for everyone, and one upload of tens of thousands held the +/// catalogue for minutes and stayed in it (#338). +pub fn opml_feeds(doc: &opml::OPML) -> Result> { + let mut found = vec![]; + feed::collect_outlines(&doc.body.outlines, &mut found); + anyhow::ensure!( + found.len() <= MAX_LISTED, + "it lists {} feeds, more than the {MAX_LISTED} one may have", + found.len() + ); + Ok(found) +} + /// Subscribes one person to every feed in an OPML document, for the CLI and the web alike. /// A feed already in the catalogue costs nothing; an unknown one is added under the OPML's /// title rather than refetching each. Returns (newly subscribed, already subscribed). @@ -1035,27 +1049,27 @@ async fn import(ctx: &Ctx, file: &std::path::Path) -> Result<()> { /// decided what each person sees, that imported nothing at all for a feed someone else /// already had, and a new one had no subscriber, so it was never scanned. /// -/// The caller parses the document, so each refuses a file that is not OPML in its own terms, -/// before anything is touched: a 400 from the web, a message from the CLI. -pub async fn subscribe_opml(ctx: &Ctx, doc: &opml::OPML, user_id: i64) -> Result<(usize, usize)> { - let mut found = vec![]; - feed::collect_outlines(&doc.body.outlines, &mut found); - +/// The caller parses the document and lists its feeds (`opml_feeds`), so each refuses a file in +/// its own terms, before anything is touched: a 400 from the web, a message from the CLI. +pub async fn subscribe_opml(ctx: &Ctx, found: Vec<(String, String)>, user_id: i64) -> Result<(usize, usize)> { let mut cfg = ctx.edit_cfg().await; - let known = subscriptions(ctx).await?; + // By address, once: each feed looked through every subscription and the whole catalogue, and + // the catalogue's lock is held throughout (#338). + let mut known = std::collections::HashMap::new(); + for s in subscriptions(ctx).await? { + // The first with an address wins, as a search in order found it. + known.entry(s.cfg.url).or_insert(s.id); + } let mut ids = vec![]; let mut grew = false; for (title, url) in found { - let existing = known - .iter() - .find(|s| s.cfg.url == url) - .map(|s| s.id.clone()) - // The same URL listed twice in one file. - .or_else(|| cfg.feeds.iter().find(|(_, f)| f.url == url).map(|(id, _)| id.clone())); + // Known already, or the same URL listed twice in one file. + let existing = known.get(&url).cloned(); let id = match existing { Some(id) => id, None => { let id = config::unique_slug(&title, |s| cfg.feeds.contains_key(s)); + known.insert(url.clone(), id.clone()); cfg.feeds.insert( id.clone(), config::Feed { @@ -3091,6 +3105,23 @@ mod tests { assert!(ctx.db.feed_summary("has-file").await.unwrap().orphaned, "and flagged as orphaned"); } + #[tokio::test] + async fn an_opml_upload_is_capped_and_takes_a_repeated_address_once() { + let opml = |n: usize| { + let outlines: String = + (0..n).map(|i| format!(r#""#, i % 3)).collect(); + feed::read_opml(&format!(r#"{outlines}"#)).unwrap() + }; + assert!(opml_feeds(&opml(MAX_LISTED + 1)).unwrap_err().to_string().contains("more than the 2000")); + let ctx = test_ctx(config::Config::default()).await; + let me = ctx.db.create_user("me", None, false).await.unwrap(); + // Six outlines, three addresses, one title: three feeds, named apart. + let (added, had) = subscribe_opml(&ctx, opml_feeds(&opml(6)).unwrap(), me).await.unwrap(); + let ids: std::collections::BTreeSet = ctx.cfg().feeds.keys().cloned().collect(); + assert_eq!(ids, ["same", "same-2", "same-3"].map(String::from).into()); + assert_eq!((added, had), (3, 3)); + } + #[tokio::test] async fn a_list_of_more_feeds_than_the_limit_is_refused() { let ctx = Arc::new(test_ctx(config::Config::default()).await); diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index 8c487b7..f0298c1 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -2447,7 +2447,8 @@ async fn import_opml( // file arrives as text, is read here, and is gone when the request ends. let doc = crate::feed::read_opml(&body.xml) .map_err(|e| ApiError::bad_request(format!("that is not an OPML file: {e}")))?; - let (added, already) = crate::subscribe_opml(&state.ctx, &doc, user.id).await?; + let found = crate::opml_feeds(&doc).map_err(|e| ApiError::bad_request(format!("{e:#}")))?; + let (added, already) = crate::subscribe_opml(&state.ctx, found, user.id).await?; if added > 0 { scan_soon(&state, None).await; }