From be3820bbbd8e709464abbb3242cf581e100b0488 Mon Sep 17 00:00:00 2001 From: rays Date: Mon, 14 Sep 2026 14:53:45 +0000 Subject: [PATCH] Release 0.5.3: OPML orphan scan, feed error UI, small UI fixes - Stop scanning an OPML/Patreon feed's derived rows once nobody subscribes to it; retire them (drop or orphan) the way sync_group already does when the list itself drops one. This is what let 922 defunct davewiner feeds keep scanning hourly after the OPML left config. - Repair feed XML with a bare `&`, and give a plain reason (moved web page with its new address when linked, or nothing yet for an empty body) instead of a raw parser error. - Show a failing feed's plain-English reason and next step (Unsubscribe / Use the new address) in the sidebar and on its own page, once it has been down a day. - Fix four small UI bugs: show-note links open in a new tab, video files play as video, an opened item no longer disappears from the Unread tab, and Subscribe/Unsubscribe get their own icons. - Fix Settings disappearing for non-admin accounts: it was hiding the whole modal instead of just the admin-only parts (Users, the editable schedule/quota, Save), which are the only parts the server actually refuses them. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DmQfE1eFPApnXWyPHBWqUA --- CHANGELOG.md | 40 ++++++++- Cargo.lock | 2 +- Cargo.toml | 2 +- TODO.md | 26 ++++-- docs/history.md | 22 +++++ src/db.rs | 47 ++++++++-- src/feed.rs | 206 ++++++++++++++++++++++++++++++++++++++++++- src/main.rs | 77 ++++++++++++++++ src/web.rs | 25 +++++- tests/ui/app.spec.js | 14 ++- web/index.html | 105 +++++++++++++++++----- 11 files changed, 520 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1efc3ef..8ac8e4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,43 @@ The long form, with what was wrong before and how it was found, is in ## [Unreleased] +## [0.5.3] - 2026-09-14 + +### Added + +- A feed that has been failing for a day shows a plain-English reason in the sidebar and on its + own page, sorted from a 404, a 401/403, a 402, a name that no longer resolves, or a web page in + place of the feed -- with Unsubscribe or, when the page links its new feed, Use the new address. + A feed that fails once and reads fine again within a day is never flagged. + +### Changed + +- Unsubscribing from the last person's OPML or Patreon subscription now retires the feeds it + listed, the same as a feed the list itself drops: removed if nothing was downloaded, kept and + marked orphaned otherwise. Until now they stayed in the database and kept being scanned hourly + with auto-download on, which is how 922 defunct `davewiner` feeds outlived the OPML that listed + them. + +### Fixed + +- A feed whose XML uses a bare `&` instead of `&` (kcpw, both feedland feeds) is now read + instead of refused. +- A feed URL that now serves a web page says so, and names the feed the page links to when it has + one, instead of a raw XML parser error. +- A publisher answering with an empty body (British Antarctic Survey's 202) is read as nothing new + to report, not a parse failure. +- A link in an item's show notes opens in a new tab instead of navigating away from ipx. +- A video file plays as video, in a small floating pane above the player bar, instead of silently + as sound only. +- On the Unread tab, opening an item no longer makes it disappear from the list -- it stays until + you open a different one, even if a scan finishes and refreshes the list while it is open. +- Subscribe and Unsubscribe have their own icons (a circled check and a circled minus) instead of + sharing the generic plus and minus used for adding feeds, users and imports. +- Settings no longer disappears for a non-admin account. It was hiding the whole Settings modal + along with the log and the users screen, but a non-admin has settings of their own in there -- + their subscriptions' Export and Import, and the schedule and quota are worth seeing even without + a say in them. Only the log and the users screen, which the server also refuses them, are gone. + ## [0.5.2] - 2026-09-12 ### Added @@ -295,7 +332,8 @@ The long form, with what was wrong before and how it was found, is in - Torrent enclosures through librqbit, seeding to a ratio or a time, with a stall timeout. - `ipx import` and `ipx export` for OPML, and systemd units in `contrib/`. -[unreleased]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.5.2...main +[unreleased]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.5.3...main +[0.5.3]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.5.2...v0.5.3 [0.5.2]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.5.1...v0.5.2 [0.5.1]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.5.0...v0.5.1 [0.5.0]: https://git.sdf1.net/rays/ipodderx-rs/compare/v0.4.0...v0.5.0 diff --git a/Cargo.lock b/Cargo.lock index 77b3ac2..cb67755 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1605,7 +1605,7 @@ checksum = "791930b43c0d5973160d90a8f3894509f2b273430f5c5c73b668636d0287c5c0" [[package]] name = "ipx" -version = "0.5.2" +version = "0.5.3" dependencies = [ "ammonia", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index 1e6152c..eebb6f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ipx" -version = "0.5.2" +version = "0.5.3" edition = "2024" [dependencies] diff --git a/TODO.md b/TODO.md index 0d397be..9872c44 100644 --- a/TODO.md +++ b/TODO.md @@ -6,7 +6,7 @@ From the production log and the feeds' stored errors on 2026-09-13. The Docker l to 12:32 UTC, so the list comes from `feeds.last_error`: 57 of 1,059 feeds. None of it is ipx's User-Agent; a browser gets the same answers. -- [ ] **Stop scanning an OPML's feeds once nobody subscribes to it.** 55 of the 57 are feeds from +- [x] **Stop scanning an OPML's feeds once nobody subscribes to it.** 55 of the 57 are feeds from `davewiner` (lists.opml.org/davefeeds.xml). The list left `config.toml` about 14 hours before this was written, but its 922 feeds are still in the database and still scanned every hour, with auto-download on: `subscriptions()` adds every derived feed, and with no parent to copy from, @@ -15,19 +15,19 @@ User-Agent; a browser gets the same answers. when the last subscriber leaves an OPML, treat its feeds the way `sync_group` treats ones the list dropped: remove those with nothing downloaded, mark the rest orphaned. `remove_feed` and `ipx rm` both leave them behind today. (`src/main.rs`, `src/web.rs`) -- [ ] **Read feeds with a bare `&`.** kcpw has `https://kcpw.org/?post_type=post&p=125715`, +- [x] **Read feeds with a bare `&`.** kcpw has `https://kcpw.org/?post_type=post&p=125715`, and both feedland feeds have the same fault. Strict XML refuses them; browsers and other readers do not. When `feed::parse` fails, try once more with every `&` that does not start an entity written as `&`. Nobody subscribes to these three now, but the next feed like them will fail the same way. (`src/feed.rs`) -- [ ] **Say what came back when it is not a feed.** Thirteen errors read "not RSS (the input did not +- [x] **Say what came back when it is not a feed.** Thirteen errors read "not RSS (the input did not begin with an rss tag) and not Atom (...)". Each one checked was a web page: the feed moved and its old URL redirects to the site, or the domain lapsed. Say "got a web page, not a feed", and when the page links a feed (``), name it. That link found the new feed for om.co, ms.now, Letters of Note, the Daily Dot, Hell Gate, The Frame Lab and Daily Kos. A `202` with an empty body (British Antarctic Survey) should read as "nothing yet", not as a parse failure. (`src/feed.rs`) -- [ ] **Show a publisher's error in the UI.** Today a failing feed shows its raw error in red only +- [x] **Show a publisher's error in the UI.** Today a failing feed shows its raw error in red only once you open it (`web/index.html:877`, `:960`); the OPML view marks a failing child "error" (`:988`), and the sidebar shows nothing. Mark a failing feed in the sidebar too, and say whose problem it is and what to do, in plain words: a 404 means the publisher took the feed down or moved @@ -41,9 +41,17 @@ User-Agent; a browser gets the same answers. gives `joanwestenberg.com/rss`, which is a 404; the feed is now `joanwestenberg.com/feed`. Nothing for ipx to fix; subscribe to the new URL directly. -The rest are the publishers' doing: 404 (24 feeds), 403 from sites that refuse anything but a -browser (5), no longer resolving or connecting (7), and one each of 400 (an invite-only Substack), -401 (a site gone private) and 402 (an expired rss.app trial). Once the first item lands, all of them -but Westenberg stop being scanned. +## Other Fixes and Features -After these: `cargo test`, `node tests/page-smoke.js`, `npx playwright test`. +- [ ] Remember which feed is selected and view (all, unread, flagged, etc) user as selected between visits. If unknown default to All Subscriptions +- [x] When clicking any link it should open in a new tab +- [ ] In mobile (iOS) sometimes the top line items like the hamburger menu are not clickable unless you do a hard refresh +- [x] video files play as audio files, they should play as video. +- [ ] Move Light/Dark/Classic options to user settings. Include an Auto mode that uses system preferences for light/dark modes +- [ ] Below Popular, have a currently listening section to show what podcasts have been started and not finnished +- [x] Update subscribe/unsubscribe icons to be circle-minus (unsubscribe) and circle-check (subscribe) +- [x] If I'm on the Unread tab, and I click to read an item the entry in the list will disappear. it should remain until I click to another item. + +## Directory Overhaul + +- [ ] Directory needs to be more functional, with categories and a more interesting layout. use /frontend-design to help diff --git a/docs/history.md b/docs/history.md index 4f587e0..d877e32 100644 --- a/docs/history.md +++ b/docs/history.md @@ -6,6 +6,28 @@ reasoning lives. New write-ups go at the top. See [README.md](../README.md) for what the thing is. +## 2026-09-14 — Settings, for everyone with an account + +A user reported that Settings disappeared shortly after they signed in: it showed for a moment, +then was gone. `#prefs` sat inside the same `.tgroup` as `#logs`, and `api('/api/me')` hid the +whole group -- `$('#admintools').hidden=true` -- the moment it learned the account was not an +admin. Nothing wrong with that check timing; it was hiding the wrong thing. + +The Settings modal is not actually all-or-nothing. `GET /api/settings`, and Export and Import +OPML, carry no admin check server-side -- `export_opml` and `import_opml` work from a user's own +subscriptions, and the schedule/quota page is read-only information, not a control. Only the +`PATCH` that changes those settings, and the Users screen behind it, return 403 for anyone but an +admin. The comment above the old hide -- "scanning, quotas, accounts and the log are the +operator's business" -- was wrong about quotas and half wrong about accounts: reading them is +everyone's; changing them is the operator's. + +`prefsModal()` now branches on `S.me.admin` the way the per-feed settings modal already does for +its URL field: a non-admin gets the schedule and quota as text, Subscriptions (Export/Import) +in full, and no Users section or Save button. Only `#logs` stays hidden, since the log names every +account and every failed sign-in. The browser test for a second account asserted the old +behaviour outright (`#prefs` hidden, not an admin) rather than what the server actually allows; +fixing the UI meant fixing the test's premise too, not just the assertion. + ## 2026-09-12 — Healthy while busy After a deploy the container sat at "starting" for a minute, and Docker's health log showed two diff --git a/src/db.rs b/src/db.rs index 5ff16c5..fcc761c 100644 --- a/src/db.rs +++ b/src/db.rs @@ -22,6 +22,10 @@ CREATE TABLE IF NOT EXISTS feeds ( last_checked INTEGER, ttl_mins INTEGER, last_error TEXT, + -- When the current run of failures began; NULL while the feed is healthy. Kept + -- through repeated failures so the UI can tell a blip (macmanx: failed once, fine an + -- hour later) from a feed that has been down for a day. + error_since INTEGER, -- Came from a subscribed OPML that no longer lists it, but has downloads, so kept. orphaned INTEGER NOT NULL DEFAULT 0, -- The OPML subscription this feed came from. @@ -165,6 +169,7 @@ fn migrate(conn: &Connection) -> Result<()> { // and it came back the same day with `last_login` beside it. ("users", "created", "INTEGER"), ("users", "last_login", "INTEGER"), + ("feeds", "error_since", "INTEGER"), ]; let retired: &[(&str, &str)] = &[ // Read state from before accounts, long since moved to entry_state. Two bugs came from @@ -208,6 +213,8 @@ pub struct FeedSummary { pub orphaned: bool, pub last_checked: Option, pub last_error: Option, + /// When this run of failures began; see the `error_since` column. + pub error_since: Option, pub entries: i64, pub downloaded: i64, } @@ -249,7 +256,7 @@ impl Db { let conn = self.conn.lock().unwrap(); let mut sum: FeedSummary = conn .query_row( - "SELECT title, image, last_checked, last_error, coalesce(orphaned, 0) + "SELECT title, image, last_checked, last_error, coalesce(orphaned, 0), error_since FROM feeds WHERE id = ?1", [feed_id], |r| { @@ -259,6 +266,7 @@ impl Db { last_checked: r.get(2)?, last_error: r.get(3)?, orphaned: r.get::<_, i64>(4)? != 0, + error_since: r.get(5)?, ..Default::default() }) }, @@ -333,7 +341,8 @@ impl Db { last_checked = excluded.last_checked, ttl_mins = excluded.ttl_mins, image = coalesce(excluded.image, feeds.image), - last_error = NULL", + last_error = NULL, + error_since = NULL", rusqlite::params![feed_id, url, title, etag, last_modified, now(), ttl_mins.map(|t| t as i64), image], )?; Ok(()) @@ -344,7 +353,8 @@ impl Db { let conn = self.conn.lock().unwrap(); conn.execute( "INSERT INTO feeds (id, url, last_checked) VALUES (?1, ?2, ?3) - ON CONFLICT(id) DO UPDATE SET last_checked = excluded.last_checked, last_error = NULL", + ON CONFLICT(id) DO UPDATE SET last_checked = excluded.last_checked, + last_error = NULL, error_since = NULL", rusqlite::params![feed_id, url, now()], )?; Ok(()) @@ -363,10 +373,15 @@ impl Db { pub fn set_feed_error(&self, feed_id: &str, url: &str, msg: &str) -> Result<()> { let conn = self.conn.lock().unwrap(); + let now = now(); conn.execute( - "INSERT INTO feeds (id, url, last_checked, last_error) VALUES (?1, ?2, ?3, ?4) - ON CONFLICT(id) DO UPDATE SET last_checked = excluded.last_checked, last_error = excluded.last_error", - rusqlite::params![feed_id, url, now(), msg], + "INSERT INTO feeds (id, url, last_checked, last_error, error_since) + VALUES (?1, ?2, ?3, ?4, ?3) + ON CONFLICT(id) DO UPDATE SET + last_checked = excluded.last_checked, + last_error = excluded.last_error, + error_since = coalesce(feeds.error_since, excluded.error_since)", + rusqlite::params![feed_id, url, now, msg], )?; Ok(()) } @@ -1766,6 +1781,26 @@ mod tests { assert_eq!(explicit(None), [None, Some(false)], "outside a group nothing is inherited"); } + #[test] + fn error_since_marks_the_start_of_a_run_of_failures_and_clears_on_success() { + let db = Db::memory().unwrap(); + db.set_feed_error("f", "http://x", "HTTP 404").unwrap(); + // Backdate it, as if this feed had already been failing a while, so a second + // failure landing "now" is distinguishable from the first. + db.exec_for_test("UPDATE feeds SET error_since = error_since - 3600 WHERE id = 'f'").unwrap(); + let first = db.feed_summary("f").unwrap().error_since.unwrap(); + + // macmanx: failed once, read fine an hour later. A second failure must not push + // error_since forward -- the UI decides "failing for a day" from the first one. + db.set_feed_error("f", "http://x", "HTTP 404").unwrap(); + assert_eq!(db.feed_summary("f").unwrap().error_since, Some(first)); + + db.touch_feed("f", "http://x").unwrap(); + let after = db.feed_summary("f").unwrap(); + assert_eq!(after.last_error, None); + assert_eq!(after.error_since, None, "a clean check ends the run of failures"); + } + #[test] fn enclosure_url_is_the_dedupe_key() { let db = Db::memory().unwrap(); diff --git a/src/feed.rs b/src/feed.rs index f3f5564..e1bfda4 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -87,6 +87,50 @@ pub async fn fetch( Ok(Fetched::Body { bytes, etag, last_modified }) } +/// A stored `last_error`, translated into plain words for whoever subscribes: whose problem +/// it is, and whether there is a new address to switch to. +pub struct Failure { + pub reason: &'static str, + pub new_url: Option, +} + +/// Reads a `last_error` the same way `set_feed_error` received it (`format!("{e:#}")` on the +/// anyhow chain from `fetch` or `parse`) and says what it means, for the errors worth telling +/// someone about. Everything else -- a timeout, a 5xx, a 429, a feed that is simply garbled -- +/// comes back `None`: transient by nature, or with nothing more useful to say than the raw +/// text already shown once a feed is open. +/// +/// ponytail: matches on the fixed strings this crate itself produces (`anyhow!("HTTP +/// {status}")`, and the "got a web page" message above) plus the substrings a DNS failure +/// reliably contains. Fragile if reqwest's own wording changes; the fallback is just showing +/// nothing extra, so a miss costs a clearer message, not a wrong one. +pub fn explain_failure(msg: &str) -> Option { + if let Some(rest) = msg.strip_prefix("got a web page, not a feed") { + let new_url = rest + .strip_prefix("; it links ") + .and_then(|r| r.strip_suffix(" as its feed")) + .map(str::to_owned); + return Some(Failure { reason: "The feed moved; this address now shows a web page.", new_url }); + } + let low = msg.to_ascii_lowercase(); + if low.contains("http 404") { + return Some(Failure { reason: "The publisher took this feed down, or moved it.", new_url: None }); + } + if low.contains("http 401") || low.contains("http 403") { + return Some(Failure { reason: "The site refuses ipx's requests.", new_url: None }); + } + if low.contains("http 402") { + return Some(Failure { reason: "The feed now needs a paid plan.", new_url: None }); + } + if low.contains("dns error") + || low.contains("failed to lookup address") + || low.contains("no address associated") + { + return Some(Failure { reason: "This address no longer resolves; the site is gone.", new_url: None }); + } + None +} + /// True when a body is an OPML document rather than a feed. /// /// The original matched on the URL ending in ".opml" (iPXClass.py:34), which misses an @@ -220,11 +264,110 @@ pub fn parse(bytes: &[u8]) -> Result { Ok(ch) => Ok(from_rss(ch, bytes)), Err(rss_err) => match atom_syndication::Feed::read_from(bytes) { Ok(feed) => Ok(from_atom(feed)), - Err(atom_err) => Err(anyhow!("not RSS ({rss_err}) and not Atom ({atom_err})")), + Err(atom_err) => { + // Some publishers (kcpw, feedland) write a bare "&" in a URL instead of + // "&". Strict XML parsers refuse it; browsers don't. Retry once with + // every offending "&" escaped rather than fail outright. + let escaped = escape_bare_ampersands(bytes); + if escaped != bytes { + if let Ok(ch) = rss::Channel::read_from(escaped.as_slice()) { + return Ok(from_rss(ch, &escaped)); + } + if let Ok(feed) = atom_syndication::Feed::read_from(escaped.as_slice()) { + return Ok(from_atom(feed)); + } + } + Err(match alternate_feed_link(bytes) { + Some(href) if looks_like_html(bytes) => { + anyhow!("got a web page, not a feed; it links {href} as its feed") + } + None if looks_like_html(bytes) => anyhow!("got a web page, not a feed"), + _ => anyhow!("not RSS ({rss_err}) and not Atom ({atom_err})"), + }) + } }, } } +/// Whether a body is a web page rather than a feed: most of the errors traced back to a feed +/// that moved or a domain that lapsed, with the old URL now serving the site instead (or a +/// redirect to it). `is_opml` already sniffs the other "not actually a feed" case. +fn looks_like_html(bytes: &[u8]) -> bool { + let head = String::from_utf8_lossy(&bytes[..bytes.len().min(2048)]).to_lowercase(); + head.contains("` (or the Atom equivalent) -- how the new address was found for om.co, ms.now, +/// Letters of Note, the Daily Dot, Hell Gate, The Frame Lab and Daily Kos. +fn alternate_feed_link(bytes: &[u8]) -> Option { + let text = String::from_utf8_lossy(bytes); + let lower = text.to_lowercase(); + let mut pos = 0; + while let Some(rel) = lower[pos..].find("').map(|e| start + e) else { break }; + pos = end + 1; + let tag = &text[start..end]; + let tag_lower = &lower[start..end]; + let is_alternate = tag_lower.contains("rel=\"alternate\"") || tag_lower.contains("rel='alternate'"); + let is_feed_type = tag_lower.contains("rss+xml") || tag_lower.contains("atom+xml"); + if is_alternate && is_feed_type + && let Some(href) = tag_attr(tag, "href") + { + return Some(href); + } + } + None +} + +/// The value of one attribute in an HTML/XML start tag, however it is quoted. +fn tag_attr(tag: &str, name: &str) -> Option { + let key = format!("{name}="); + let idx = tag.to_lowercase().find(&key)?; + let after = &tag[idx + key.len()..]; + let quote = after.chars().next()?; + if quote != '"' && quote != '\'' { + return None; + } + let rest = &after[1..]; + let close = rest.find(quote)?; + Some(rest[..close].trim().to_owned()) +} + +/// Escapes every `&` that does not already start a recognized XML entity +/// (`&`, `<`, `>`, `"`, `'`, or a numeric reference like `'`). +fn escape_bare_ampersands(bytes: &[u8]) -> Vec { + fn is_entity_start(rest: &[u8]) -> bool { + for named in [&b"amp;"[..], b"lt;", b"gt;", b"quot;", b"apos;"] { + if rest.starts_with(named) { + return true; + } + } + let digits = if rest.starts_with(b"#x") || rest.starts_with(b"#X") { + &rest[2..] + } else if rest.starts_with(b"#") { + &rest[1..] + } else { + return false; + }; + let len = digits.iter().take_while(|b| b.is_ascii_alphanumeric()).count(); + len > 0 && digits.get(len) == Some(&b';') + } + + let mut out = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'&' && !is_entity_start(&bytes[i + 1..]) { + out.extend_from_slice(b"&"); + } else { + out.push(bytes[i]); + } + i += 1; + } + out +} + /// Every `` of every ``, in document order. /// /// The `rss` crate models an item as having at most one enclosure -- which is what RSS 2.0 @@ -591,6 +734,46 @@ mod tests { ); } + #[test] + fn explain_failure_translates_the_errors_the_ui_should_flag() { + assert_eq!( + explain_failure("HTTP 404 Not Found").unwrap().reason, + "The publisher took this feed down, or moved it." + ); + assert_eq!(explain_failure("HTTP 401 Unauthorized").unwrap().reason, "The site refuses ipx's requests."); + assert_eq!(explain_failure("HTTP 403 Forbidden").unwrap().reason, "The site refuses ipx's requests."); + assert_eq!(explain_failure("HTTP 402 Payment Required").unwrap().reason, "The feed now needs a paid plan."); + let dns = explain_failure("connecting: dns error: failed to lookup address information").unwrap(); + assert_eq!(dns.reason, "This address no longer resolves; the site is gone."); + let moved = explain_failure("got a web page, not a feed; it links https://x/feed as its feed").unwrap(); + assert_eq!(moved.new_url.as_deref(), Some("https://x/feed")); + assert!(explain_failure("got a web page, not a feed").unwrap().new_url.is_none()); + for transient in ["HTTP 500 Internal Server Error", "HTTP 429 Too Many Requests", "operation timed out"] { + assert!(explain_failure(transient).is_none(), "{transient} must not be flagged"); + } + } + + #[test] + fn a_web_page_says_so_and_names_the_feed_it_links() { + let html = br#" + + not a feed"#; + let err = parse(html).unwrap_err().to_string(); + assert_eq!(err, "got a web page, not a feed; it links https://x.example/feed as its feed"); + } + + #[test] + fn a_web_page_with_no_feed_link_still_says_so() { + let html = b"moved"; + assert_eq!(parse(html).unwrap_err().to_string(), "got a web page, not a feed"); + } + + #[test] + fn garbage_that_is_not_html_gets_the_original_parser_errors() { + let err = parse(b"not xml at all").unwrap_err().to_string(); + assert!(err.starts_with("not RSS ("), "{err}"); + } + #[test] fn parses_atom_enclosure_links() { let bytes = include_bytes!("../tests/data/atom.xml"); @@ -613,6 +796,27 @@ mod tests { ); } + #[test] + fn a_bare_ampersand_in_a_link_is_repaired_and_parsed() { + // kcpw.org: https://kcpw.org/?post_type=post&p=125715 -- a bare "&" + // that strict XML rejects but browsers accept. + let xml = br#" + Xhttps://xd + ag1 + https://kcpw.org/?post_type=post&p=125715 + + "#; + let feed = parse(xml).unwrap(); + assert_eq!(feed.entries[0].link.as_deref(), Some("https://kcpw.org/?post_type=post&p=125715")); + assert_eq!(feed.entries[0].enclosures[0].url, "https://x/a.mp3?a=1&b=2"); + } + + #[test] + fn escape_bare_ampersands_leaves_real_entities_alone() { + let out = escape_bare_ampersands(b"a&b <x> ' / c&d"); + assert_eq!(out, b"a&b <x> ' / c&d"); + } + #[test] fn the_rss_title_always_wins_and_episode_numbers_stay_metadata() { // Some feeds set a different itunes:title. The displayed title is always the RSS diff --git a/src/main.rs b/src/main.rs index 7d7a3d8..e8bb7ba 100644 --- a/src/main.rs +++ b/src/main.rs @@ -637,6 +637,7 @@ fn rm(ctx: &Ctx, config_path: &std::path::Path, feed: &str) -> Result<()> { cfg.save(config_path)?; // State and files stay: re-adding the feed should not re-download its back catalogue. println!("removed {feed}; downloads and history kept"); + retire_group(ctx, feed)?; Ok(()) } @@ -842,6 +843,10 @@ async fn fetch(ctx: &Arc, only: Option<&str>, force: bool) -> Result<()> { feed: id.clone(), reason: "not modified".into(), }), + Ok(Outcome::Empty) => ctx.out.emit(Event::FeedSkip { + feed: id.clone(), + reason: "nothing yet".into(), + }), Ok(Outcome::Opml { added, removed, kept, total }) => { ctx.out.emit(Event::FeedSkip { feed: id.clone(), @@ -919,6 +924,12 @@ pub fn subscriptions(ctx: &Ctx) -> Result> { continue; // promoted to config at some point; that entry wins } let parent = cfg.feeds.get(&m.group_id); + if parent.is_none() { + // The OPML or Patreon feed this was derived from is no longer in config -- + // removing it should have retired these rows too (see `retire_group`), but + // skip them here regardless so a row that slips through is never scanned. + continue; + } let base = parent .and_then(|p| p.folder.clone()) .or_else(|| ctx.db.feed_summary(&m.group_id).ok().and_then(|s| s.title)) @@ -947,6 +958,22 @@ pub fn subscriptions(ctx: &Ctx) -> Result> { Ok(out) } +/// Retires every feed derived from `parent_id`, now that nothing subscribes to the OPML or +/// Patreon feed that listed them: the same rule `sync_group` applies to one the list drops -- +/// removed if nothing was downloaded, orphaned and kept otherwise. Called once the parent +/// itself is removed, since `subscriptions()` would otherwise keep scanning them under a +/// fallback policy meant for a feed with no parent at all. +pub fn retire_group(ctx: &Ctx, parent_id: &str) -> Result<()> { + for m in ctx.db.managed_feeds()?.into_iter().filter(|m| m.group_id == parent_id) { + if ctx.db.downloaded_count(&m.id).unwrap_or(1) > 0 { + ctx.db.set_orphaned(&m.id, true)?; + } else { + ctx.db.drop_managed(&m.id)?; + } + } + Ok(()) +} + /// Seconds to wait before re-checking a feed. /// /// A per-feed schedule is an explicit instruction and wins outright. Without one, the @@ -971,6 +998,9 @@ struct Scan { /// What a scan of one feed turned out to be. enum Outcome { NotModified, + /// A response with nothing in it -- the British Antarctic Survey answers a 202 with an + /// empty body when it has nothing new to publish. Not a parse failure; try again later. + Empty, Feed(Scan), /// The URL is a list of feeds rather than a feed: an OPML, or a Patreon creator's shows. Opml { added: Vec, removed: usize, kept: usize, total: usize }, @@ -1033,6 +1063,11 @@ async fn scan_one( feed::Fetched::Body { bytes, etag, last_modified } => (bytes, etag, last_modified), }; + if bytes.iter().all(u8::is_ascii_whitespace) { + ctx.db.touch_feed(id, &feed_cfg.url)?; + return Ok(Outcome::Empty); + } + // A subscribed OPML is a list of feeds, not a feed. The original matched on a ".opml" // URL; sniffing the body also catches one served from a URL without that extension. if feed::is_opml(&bytes) { @@ -1635,4 +1670,46 @@ mod tests { let p = merge_policy(&[sub(None, Some(false), None), sub(None, Some(true), None)], &feed(), 3); assert!(p.auto_download); } + + fn test_ctx(cfg: config::Config) -> Ctx { + Ctx { + cfg: std::sync::RwLock::new(Arc::new(cfg)), + db: db::Db::memory().unwrap(), + client: reqwest::Client::new(), + out: Emitter::terminal(), + torrents: tokio::sync::OnceCell::new(), + torrent_slots: Arc::new(tokio::sync::Semaphore::new(2)), + config_path: PathBuf::new(), + detach_torrents: false, + } + } + + #[test] + fn a_derived_feed_is_not_scanned_once_its_opml_leaves_config() { + // davewiner: the OPML subscription left config.toml, but its 922 derived rows + // stayed in the database and kept being scanned under the no-parent fallback. + let ctx = test_ctx(config::Config::default()); + ctx.db.upsert_managed("child", "http://x/child.xml", "Child", "gone-opml").unwrap(); + assert!( + subscriptions(&ctx).unwrap().iter().all(|s| s.id != "child"), + "a derived feed whose parent is gone from config must not be scanned" + ); + } + + #[test] + fn retiring_a_group_drops_what_was_never_downloaded_and_orphans_the_rest() { + let ctx = test_ctx(config::Config::default()); + ctx.db.upsert_managed("empty", "http://x/empty.xml", "Empty", "parent").unwrap(); + ctx.db.upsert_managed("has-file", "http://x/has-file.xml", "Has File", "parent").unwrap(); + let enc = feed::Enclosure { url: "http://x/ep.mp3".into(), mime: None, length: None }; + ctx.db.record_enclosure("has-file", "g1", &enc).unwrap(); + ctx.db.mark_downloaded(&enc.url, std::path::Path::new("/downloads/ep.mp3"), 1).unwrap(); + + retire_group(&ctx, "parent").unwrap(); + + let managed = ctx.db.managed_feeds().unwrap(); + assert!(!managed.iter().any(|m| m.id == "empty"), "nothing downloaded, so it is forgotten"); + assert!(managed.iter().any(|m| m.id == "has-file"), "has a file on disk, so it is kept"); + assert!(ctx.db.feed_summary("has-file").unwrap().orphaned, "and flagged as orphaned"); + } } diff --git a/src/web.rs b/src/web.rs index 078e554..10f3209 100644 --- a/src/web.rs +++ b/src/web.rs @@ -488,6 +488,9 @@ struct FeedRow { last_checked: Option, next_check: Option, last_error: Option, + /// Set once `last_error` is a kind worth telling someone about and it has held for a + /// day -- a feed that fails once and reads fine an hour later (macmanx) never gets here. + failing: Option, entries: i64, downloaded: i64, unread: i64, @@ -495,6 +498,15 @@ struct FeedRow { subscribers: i64, } +#[derive(Serialize)] +struct FailingRow { + reason: &'static str, + new_url: Option, +} + +/// A day, in seconds: how long an error has to hold before the UI mentions it. +const FLAG_AFTER_SECS: i64 = 86_400; + async fn feeds( State(state): State, user: crate::db::User, @@ -558,6 +570,12 @@ async fn feeds( next_check: s .last_checked .map(|t| t + crate::due_after(&cfg, feed, st.ttl_mins) as i64), + failing: s + .error_since + .filter(|since| crate::db::now() - since >= FLAG_AFTER_SECS) + .and_then(|_| s.last_error.as_deref()) + .and_then(crate::feed::explain_failure) + .map(|f| FailingRow { reason: f.reason, new_url: f.new_url }), last_error: s.last_error, entries: s.entries, downloaded: s.downloaded, @@ -917,9 +935,13 @@ fn entry_page( let mut rows = db.entries_in(user_id, feed, filter, search, page.offset, page.limit.clamp(1, 200), &order)?; // Feed HTML is untrusted: it reaches the page only after ammonia has been through it. + // Every link opens in a new tab -- ammonia's default rel="noopener noreferrer" already + // keeps that safe -- so following one in show notes never navigates away from ipx. + let mut sanitizer = ammonia::Builder::new(); + sanitizer.add_tag_attributes("a", &["target"]).set_tag_attribute_value("a", "target", "_blank"); for row in &mut rows { if let Some(d) = &row.description { - row.description = Some(ammonia::clean(d)); + row.description = Some(sanitizer.clean(d).to_string()); } } let total = db.count_in(user_id, feed, filter, search)?; @@ -1147,6 +1169,7 @@ async fn remove_feed( } cfg.save(&state.config_path)?; state.ctx.reload_cfg(&state.config_path)?; + crate::retire_group(&state.ctx, &id)?; Ok(StatusCode::NO_CONTENT) } diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index dc5372e..2f43d1b 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -359,7 +359,15 @@ test('a second person has their own feeds and their own read state', async ({ br // Sam subscribes to nothing yet, so sees nothing -- the admin's feeds are not theirs. await expect(page.locator('#feedlist')).toContainText('No feeds.'); - await expect(page.locator('#prefs')).toBeHidden(); // not an admin + // Settings stays: Sam has their own subscriptions to export and import, and the + // schedule and quota are worth seeing even without a say in them. Only the log and the + // users screen -- and the server -- are an admin's alone. + await expect(page.locator('#prefs')).toBeVisible(); + await page.locator('#prefs').click(); + await expect(page.locator('#modalCard')).toContainText('Subscriptions'); + await expect(page.locator('#gsave')).toBeHidden(); + await expect(page.locator('#gusers')).toBeHidden(); + await page.locator('#modalCard .cardacts .btn').first().click(); // Hiding the button is not the guard; the server is. expect((await page.request.get('/api/users')).status()).toBe(403); await expect(page.locator('#logs')).toBeHidden(); @@ -790,7 +798,9 @@ test('play in the Files pane plays once, in the player bar', async ({ page }) => await page.locator('.ep', { has: page.locator('.kind.here') }).first().click(); await page.locator('#files [data-a="play"]').click(); await expect(page.locator('#player')).toBeVisible(); - await expect(page.locator('audio')).toHaveCount(1); // the player bar's, and nothing else + // The player bar's element doubles as a