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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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 <name> --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<Vec<(String, String)>> {
|
||||
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#"<outline text="Same" xmlUrl="http://x/{}.xml"/>"#, i % 3)).collect();
|
||||
feed::read_opml(&format!(r#"<opml version="2.0"><head/><body>{outlines}</body></opml>"#)).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<String> = 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);
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user