rays/ipx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 18 Wiki Activity

Bound the scan work one feed's author can ask for (#339 #337)

Browse Source 
Both run on the daemon's single worker, so every other feed's check
and download waited behind them.

#339: prefer_https asked each new artwork host whether it served the
image on https, up to 20 s each, one after another, and the per-scan
cache is keyed by the authority, which ports and user names multiply
for one server. It now asks at most eight hosts a read; past that an
http image stays http, for /api/art to fetch, as one that fails does.

#337: a list (OPML or Patreon) may bring in 2,000 feeds; past that
sync_group refuses it whole, so the feed shows an error and nothing
changes. davewiner's, the largest seen, had 922. sync_group also read
the whole feeds table and rebuilt an id map with a Feed cloned per
entry for every new feed; it now reads the table once and checks ids
against sets, and unique_slug takes a predicate rather than a map.
same_feed skips parsing two URLs unless both are Patreon's, since only
those can differ and match.

sync_group and sync_opml lose the parent argument, which nothing used
once promote_kept stopped taking it (#332).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ray Slakinski
2026-10-23 14:18:32 +00:00
parent 3bfd8f848b
commit d35b8054b4
4 changed files with 71 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
crates/feed/src/lib.rs
View File

@@ -452,7 +452,11 @@ pub fn expand_input(input: &str) -> String {
/// creator's name, by number, or with no creator at all -- and the token and show are what
/// identify it.
pub fn same_feed(a: &str, b: &str) -> bool {
a == b || patreon_parts(a).is_some_and(|p| Some(p) == patreon_parts(b))
// Parsing both is the slow part, and only Patreon's addresses can differ and still match.
a == b
|| (a.contains("patreon.com")
&& b.contains("patreon.com")
&& patreon_parts(a).is_some_and(|p| Some(p) == patreon_parts(b)))
}
/// A Patreon creator's name and shows, each show as (title, feed URL).
@@ -666,6 +670,9 @@ pub async fn site_icon(client: &reqwest::Client, site: &str) -> Option<String> {
is_image(client, ico.as_str()).await.then(|| ico.into())
}
/// How many hosts one scan of a feed asks whether its artwork is on https.
const MAX_ART_HOSTS: usize = 8;
/// Artwork's address on https when its host serves it there, else as it was. The page is https
/// and must not load http; the host is asked once per `known` (one feed's read), and an http
/// address it does not serve on https stays, for /api/art to fetch (#90). Four of the five
@@ -681,6 +688,10 @@ pub async fn prefer_https(
let secure = format!("https://{rest}");
let ok = match known.get(&host) {
Some(ok) => *ok,
// Each new host is a probe of up to 20 s, one after another on the scan, and the feed's
// author decides how many hosts there are: ports and user names make as many as they like
// of one server (#339). Real feeds put their artwork on a few.
None if known.len() >= MAX_ART_HOSTS => false,
None => {
let ok = is_image(client, &secure).await;
known.insert(host, ok);
@@ -1838,6 +1849,16 @@ Some **bold** and [a link](https://x.example/).</description></item>
assert_eq!((p1.season, p1.episode), (Some(8), Some(1)));
}
#[tokio::test]
async fn artwork_on_more_hosts_than_the_limit_is_not_asked_about() {
let mut known: std::collections::HashMap<String, bool> =
(0..MAX_ART_HOSTS).map(|n| (format!("h{n}:80"), false)).collect();
// No server answers here: were it asked, this would wait on a connection, not return.
let url = "http://192.0.2.1:9/a.png";
assert_eq!(prefer_https(&reqwest::Client::new(), url, &mut known).await, url);
assert_eq!(known.len(), MAX_ART_HOSTS);
}
#[test]
fn deeply_nested_opml_is_refused_before_it_is_parsed() {
let nest = |n: usize| {

2
crates/ipx/CHANGELOG.md
View File

@@ -31,6 +31,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- A new install's `admin` gets a random password, written to `admin-password` beside config.toml and readable by iPX's user alone, or the one in `IPX_ADMIN_PASSWORD`. Every install began as admin / ipodderx, a password published in the README, and the log printed it (#321).
- A show kept from a private OPML or Patreon list stays private, with the list's username and password, once the list is removed or left. It became public and was shared to the directory (#332).
- A feed's username and password go to its own host alone: not to its episodes on other hosts, nor to the feeds an OPML lists on other hosts, which took the OPML's password on every check (#327, #328).
- A check of one feed asks at most eight hosts whether its artwork is on https. A feed could name artwork on as many hosts as it liked, each asked in turn for up to 20 seconds, and hold up every other feed's check for hours (#339).
- An OPML or Patreon list may bring in up to 2,000 feeds; one with more shows an error and adds none. A list of tens of thousands held up checking for everyone, and adding its feeds read the whole feeds table once for each (#337).
## [1.4.0] - 2026-10-19

8
crates/ipx/src/config.rs
View File

@@ -522,12 +522,12 @@ pub fn slug(text: &str) -> String {
}
/// `slug`, with a numeric suffix if that id is taken.
pub fn unique_slug(text: &str, taken: &BTreeMap<String, Feed>) -> String {
pub fn unique_slug(text: &str, taken: impl Fn(&str) -> bool) -> String {
let base = slug(text);
if !taken.contains_key(&base) {
if !taken(&base) {
return base;
}
(2..).map(|n| format!("{base}-{n}")).find(|s| !taken.contains_key(s)).unwrap()
(2..).map(|n| format!("{base}-{n}")).find(|s| !taken(s)).unwrap()
}
/// `$var`, or `~/fallback` when it is unset or empty, as the XDG base directory spec says.
@@ -732,7 +732,7 @@ mod tests {
private: false,
},
);
assert_eq!(unique_slug("The Daily", &taken), "the-daily-2");
assert_eq!(unique_slug("The Daily", |s| taken.contains_key(s)), "the-daily-2");
}
#[test]

67
crates/ipx/src/main.rs
View File

@@ -980,7 +980,7 @@ async fn add_one(
taken.entry(id).or_insert_with(|| probe.clone());
}
}
let id = config::unique_slug(&title, &taken);
let id = config::unique_slug(&title, |s| taken.contains_key(s));
cfg.feeds.insert(id.clone(), probe);
Ok(id)
}
@@ -1050,7 +1050,7 @@ pub async fn subscribe_opml(ctx: &Ctx, doc: &opml::OPML, user_id: i64) -> Result
let id = match existing {
Some(id) => id,
None => {
let id = config::unique_slug(&title, &cfg.feeds);
let id = config::unique_slug(&title, |s| cfg.feeds.contains_key(s));
cfg.feeds.insert(
id.clone(),
config::Feed {
@@ -1758,7 +1758,7 @@ async fn scan_one(
// heap. The items go; its files and read state move to each show as the show
// lists them (`Db::adopt`), so no show comes up empty for want of a URL.
ctx.db.clear_entries(id).await?;
return sync_group(ctx, id, feed_cfg, &shows).await;
return sync_group(ctx, id, &shows).await;
}
Ok(_) => {} // One show: the creator's feed is that show.
// Already split: keep the shows it has rather than read the creator as one heap.
@@ -1836,7 +1836,7 @@ async fn scan_one(
// URL; sniffing the body also catches one served from a URL without that extension.
if feed::is_opml(&bytes) {
ctx.db.touch_feed(id, &feed_cfg.url).await?;
return sync_opml(ctx, id, feed_cfg, &bytes).await;
return sync_opml(ctx, id, &bytes).await;
}
let mut parsed = feed::parse(&bytes)?;
@@ -2071,14 +2071,17 @@ async fn scan_one(
/// A feed's body fetched by the scan ahead of its turn, and the site's icon if one was looked up.
type Prefetched = (Result<(feed::Fetched, Option<String>)>, Option<String>);
async fn sync_opml(ctx: &Arc<Ctx>, parent_id: &str, parent: &config::Feed, bytes: &[u8]) -> Result<Outcome> {
async fn sync_opml(ctx: &Arc<Ctx>, parent_id: &str, bytes: &[u8]) -> Result<Outcome> {
let listed = feed::parse_opml(bytes)?;
if let Some(title) = feed::opml_title(bytes) {
ctx.db.set_title(parent_id, &title).await?;
}
sync_group(ctx, parent_id, parent, &listed).await
sync_group(ctx, parent_id, &listed).await
}
/// How many feeds an OPML or Patreon list may bring in.
const MAX_LISTED: usize = 2000;
/// Brings the feed list in step with a list of feeds: a subscribed OPML, or a Patreon
/// creator's shows.
///
@@ -2086,15 +2089,22 @@ async fn sync_opml(ctx: &Arc<Ctx>, parent_id: &str, parent: &config::Feed, bytes
/// the list is unsubscribed *only if nothing was ever downloaded for it* -- otherwise it
/// is kept and flagged, because dropping it would orphan files on disk with nothing in
/// the UI to explain them.
async fn sync_group(
ctx: &Arc<Ctx>,
parent_id: &str,
parent: &config::Feed,
listed: &[(String, String)],
) -> Result<Outcome> {
async fn sync_group(ctx: &Arc<Ctx>, parent_id: &str, listed: &[(String, String)]) -> Result<Outcome> {
// Each is a feed scanned for everyone subscribed to the list, and the list is its author's to
// write: one of tens of thousands held the scan for hours (#337). davewiner's had 922.
anyhow::ensure!(
listed.len() <= MAX_LISTED,
"it lists {} feeds, more than the {MAX_LISTED} a list may have",
listed.len()
);
let cfg = ctx.cfg();
let existing = ctx.db.managed_feeds().await?;
let mut added = vec![];
// Read once, not once a new feed: each read is the whole feeds table.
let known = ctx.db.feed_urls().await?;
let fixed: std::collections::HashSet<&str> =
cfg.feeds.keys().chain(existing.iter().map(|m| &m.id)).map(String::as_str).collect();
let known_ids: std::collections::HashSet<&str> = known.iter().map(|(id, _)| id.as_str()).collect();
for (title, url) in listed {
// Already known, whether derived or promoted into the config.
@@ -2107,16 +2117,11 @@ async fn sync_group(
continue;
}
// A removed feed keeps its rows, so its id is only free again for the same feed.
let known = ctx.db.feed_urls().await?;
let taken: std::collections::BTreeMap<String, config::Feed> = cfg
.feeds
.keys()
.chain(existing.iter().map(|m| &m.id))
.chain(added.iter())
.chain(known.iter().filter(|(_, u)| !feed::same_feed(u, url)).map(|(id, _)| id))
.map(|id| (id.clone(), parent.clone()))
.collect();
let id = config::unique_slug(title, &taken);
let its_own: Vec<&str> =
known.iter().filter(|(_, u)| feed::same_feed(u, url)).map(|(id, _)| id.as_str()).collect();
let id = config::unique_slug(title, |s| {
fixed.contains(s) || added.iter().any(|a| a == s) || (known_ids.contains(s) && !its_own.contains(&s))
});
ctx.db.upsert_managed(&id, url, title, parent_id).await?;
added.push(id);
}
@@ -3081,6 +3086,21 @@ mod tests {
assert!(ctx.db.feed_summary("has-file").await.unwrap().orphaned, "and flagged as orphaned");
}
#[tokio::test]
async fn a_list_of_more_feeds_than_the_limit_is_refused() {
let ctx = Arc::new(test_ctx(config::Config::default()).await);
let listed: Vec<(String, String)> =
(0..=MAX_LISTED).map(|n| (format!("Show {n}"), format!("http://x/{n}.xml"))).collect();
let err = sync_group(&ctx, "opml", &listed).await.err().expect("refused");
assert!(err.to_string().contains("more than the 2000"), "{err:#}");
assert!(ctx.db.managed_feeds().await.unwrap().is_empty(), "nothing taken from it");
// At the limit, all of them, each its own id.
sync_group(&ctx, "opml", &listed[..MAX_LISTED]).await.unwrap();
let ids: std::collections::HashSet<String> =
ctx.db.managed_feeds().await.unwrap().into_iter().map(|m| m.id).collect();
assert_eq!(ids.len(), MAX_LISTED);
}
#[tokio::test]
async fn a_kept_feed_outlives_its_opml_for_whoever_kept_it() {
let mut cfg = config::Config::default();
@@ -3102,8 +3122,7 @@ mod tests {
assert!(ctx.db.set_kept(me, "b", true).await.unwrap());
// The OPML drops a: mine to keep, a feed of its own; yours, unkept, goes.
let parent = ctx.cfg().feeds["opml"].clone();
sync_group(&ctx, "opml", &parent, &[("b".into(), "http://x/b.xml".into())]).await.unwrap();
sync_group(&ctx, "opml", &[("b".into(), "http://x/b.xml".into())]).await.unwrap();
let a = ctx.cfg().feeds.get("a").cloned().expect("a is in the catalogue");
assert_eq!((a.url.as_str(), a.group, a.private), ("http://x/a.xml", None, true));
assert!(!ctx.db.managed_feeds().await.unwrap().iter().any(|m| m.id == "a"));