From 79bc006a30a4b4d5bb914c0460947f6bfd912b7f Mon Sep 17 00:00:00 2001 From: rays Date: Tue, 29 Sep 2026 20:06:04 +0000 Subject: [PATCH] Add only what is a feed or links one; refuse the rest (#102) Adding an address looked for the feed a web page links and, finding none, added the address as it was: every check then failed, and the sidebar called it a feed that had moved. cnn.com is one; its page links no feed. find_feed replaces feed_behind_page: the address is added if it is a feed or an OPML list, the feed its page links if it is a web page that links one (and that is a feed), and otherwise the add is refused with the reason, from the web page (400, the dialog stays open) and from `ipx add`. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/feed.rs | 42 +++++++++++++++++++++++++---------- src/main.rs | 6 ++--- src/web.rs | 7 ++++-- tests/ui/app.spec.js | 13 +++++++++++ tests/ui/fixtures/linked.xml | 4 ++++ tests/ui/fixtures/nofeed.html | 2 ++ tests/ui/fixtures/site.html | 4 ++++ 8 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 tests/ui/fixtures/linked.xml create mode 100644 tests/ui/fixtures/nofeed.html create mode 100644 tests/ui/fixtures/site.html diff --git a/CHANGELOG.md b/CHANGELOG.md index 273ae2c..46d2251 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Adding an address checks it first: a feed is added, a web page adds the feed it links, and anything else is refused with the reason, instead of being added and failing on every check. - A feed that was failing when its last subscriber left is forgotten, rather than kept with its error for good. One that worked, or has files on disk, is kept as before. - A feed that keeps failing is checked less and less often, waiting as long as it has been failing, up to once a day; it goes back to its schedule as soon as it works. Refreshing it still checks it at once. - A scan's trace shows the time a feed spends on its artwork and in the database after the fetch. diff --git a/src/feed.rs b/src/feed.rs index 77a6dc4..84e4257 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -394,20 +394,38 @@ fn alternate_feed_link(bytes: &[u8]) -> Option { None } -/// The feed a web page links as its own, for someone who pasted a site's address instead of -/// its feed's. Anything else, or a page that cannot be read, comes back as given, and adding -/// it goes on as before. -pub async fn feed_behind_page(client: &reqwest::Client, url: &str) -> String { - let found = async { - let resp = client.get(url).timeout(std::time::Duration::from_secs(20)).send().await.ok()?; +/// What someone asked to add, as a feed: the address itself when it is a feed or an OPML list, +/// otherwise the feed its web page links as its own, otherwise an error and nothing is added. +/// A page that linked no feed used to be added as it was and failed on every check, called a +/// feed that moved (#102); cnn.com is one. +pub async fn find_feed(client: &reqwest::Client, url: &str) -> Result { + let read = |u: String| async move { + 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(); - let page = resp.bytes().await.ok()?; - if !looks_like_html(&page) { - return None; - } - base.join(&alternate_feed_link(&page)?).ok() + anyhow::Ok((base, resp.bytes().await.context("reading")?)) }; - found.await.map(String::from).unwrap_or_else(|| url.to_owned()) + 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}"))?; + if is_feed(&body) { + return Ok(url.to_owned()); + } + if !looks_like_html(&body) { + let why = parse(&body).err().map(|e| format!("{e:#}")).unwrap_or_default(); + anyhow::bail!("{url} is not a feed: {why}"); + } + let Some(href) = alternate_feed_link(&body) else { + anyhow::bail!("{url} is a web page that links no feed, so there is nothing to subscribe to"); + }; + let linked = base.join(&href).with_context(|| format!("{url} links {href} as its feed, which is not an address"))?; + let (_, feed) = read(linked.to_string()).await.with_context(|| format!("{url} links {linked} as its feed, but"))?; + anyhow::ensure!(is_feed(&feed), "{url} links {linked} as its feed, but that is not a feed either"); + Ok(linked.into()) } /// Artwork for a feed that has none: the icon its site's page names, or else the site's diff --git a/src/main.rs b/src/main.rs index b6dfa3f..2a58f02 100644 --- a/src/main.rs +++ b/src/main.rs @@ -661,7 +661,7 @@ async fn add( keywords: Vec, ) -> Result<()> { let mut cfg = (*ctx.cfg()).clone(); - let url = &feed::feed_behind_page(&ctx.client, &feed::expand_input(url)).await; + let url = &feed::find_feed(&ctx.client, &feed::expand_input(url)).await?; // Includes feeds derived from an OPML, or the same show could be added twice. if let Some(existing) = subscriptions(ctx).await?.iter().find(|s| feed::same_feed(&s.cfg.url, url)) { anyhow::bail!("already subscribed as {:?}", existing.id); @@ -672,8 +672,8 @@ async fn add( Ok(()) } -/// Returns the new feed id. The title needs a fetch, so a feed that cannot be reached is -/// still added -- under a slug derived from its URL -- rather than refused. +/// Returns the new feed id. Adding checks the address is a feed first (`feed::find_feed`); this +/// still names one it cannot read from its URL rather than failing. pub async fn add_one( ctx: &Ctx, cfg: &mut config::Config, diff --git a/src/web.rs b/src/web.rs index 5fbf7a1..616f673 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1282,8 +1282,11 @@ async fn add_feed( Json(body): Json, ) -> Result, ApiError> { let mut cfg = (*state.ctx.cfg()).clone(); - // Before the duplicate check, so a site's page finds the feed someone already has. - let url = crate::feed::feed_behind_page(&state.ctx.client, &crate::feed::expand_input(&body.url)).await; + // Before the duplicate check, so a site's page finds the feed someone already has. What is + // not a feed and links none is refused here, with why, and nothing is added. + let url = crate::feed::find_feed(&state.ctx.client, &crate::feed::expand_input(&body.url)) + .await + .map_err(|e| ApiError::bad_request(format!("{e:#}")))?; // Someone else may already have it. Then adding costs nothing: no second fetch, no // second copy on disk, just another name against the same feed. if let Some(existing) = crate::subscriptions(&state.ctx).await? diff --git a/tests/ui/app.spec.js b/tests/ui/app.spec.js index 4d7375a..d36fbed 100644 --- a/tests/ui/app.spec.js +++ b/tests/ui/app.spec.js @@ -927,6 +927,19 @@ test('adding a feed scans it straight away', async ({ page }) => { await expect(page.locator('.ep', { hasText: 'Fresh Ep' })).toBeVisible({ timeout: 10_000 }); }); +test('adding a page adds the feed it links, and a page with no feed is refused', async ({ page }) => { + const feeds = await page.locator('.feed').count(); + await page.locator('#addFeed').click(); + await page.locator('#nurl').fill('http://127.0.0.1:8792/nofeed.html'); + await page.locator('#nsave').click(); + await expect(page.locator('.toast')).toContainText('links no feed'); + await expect(page.locator('#modal.on')).toBeVisible(); // left open to correct it + await expect(page.locator('.feed')).toHaveCount(feeds); + await page.locator('#nurl').fill('http://127.0.0.1:8792/site.html'); + await page.locator('#nsave').click(); + await expect(page.locator('.feed', { hasText: 'Linked Site' })).toBeVisible({ timeout: 20_000 }); +}); + test('a deleted file looks as if it was never downloaded', async ({ page }) => { // Other people subscribe to Picture Blog by now, so both prompts come; take them. page.on('dialog', d => d.accept()); diff --git a/tests/ui/fixtures/linked.xml b/tests/ui/fixtures/linked.xml new file mode 100644 index 0000000..ad3d0bf --- /dev/null +++ b/tests/ui/fixtures/linked.xml @@ -0,0 +1,4 @@ + +Linked Sitehttp://127.0.0.1:8792/site.html +Linked Postlinked-1 + diff --git a/tests/ui/fixtures/nofeed.html b/tests/ui/fixtures/nofeed.html new file mode 100644 index 0000000..58615d6 --- /dev/null +++ b/tests/ui/fixtures/nofeed.html @@ -0,0 +1,2 @@ + +No Feed HereA site that links no feed. diff --git a/tests/ui/fixtures/site.html b/tests/ui/fixtures/site.html new file mode 100644 index 0000000..4441bcc --- /dev/null +++ b/tests/ui/fixtures/site.html @@ -0,0 +1,4 @@ + +A Site + +A site with a feed.