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 <noreply@anthropic.com>

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.

src/feed.rs

@@ -394,20 +394,38 @@ fn alternate_feed_link(bytes: &[u8]) -> Option<String> {
     None
 }
 
-/// The feed a web page links as its own, for someone who pasted a site's address instead of
+/// What someone asked to add, as a feed: the address itself when it is a feed or an OPML list,
-/// its feed's. Anything else, or a page that cannot be read, comes back as given, and adding
+/// otherwise the feed its web page links as its own, otherwise an error and nothing is added.
-/// it goes on as before.
+/// A page that linked no feed used to be added as it was and failed on every check, called a
-pub async fn feed_behind_page(client: &reqwest::Client, url: &str) -> String {
+/// feed that moved (#102); cnn.com is one.
-    let found = async {
+pub async fn find_feed(client: &reqwest::Client, url: &str) -> Result<String> {
-        let resp = client.get(url).timeout(std::time::Duration::from_secs(20)).send().await.ok()?;
+    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()?;
+        anyhow::Ok((base, resp.bytes().await.context("reading")?))
-        if !looks_like_html(&page) {
-            return None;
-        }
-        base.join(&alternate_feed_link(&page)?).ok()
     };
-    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

src/main.rs

@@ -661,7 +661,7 @@ async fn add(
     keywords: Vec<String>,
 ) -> 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
+/// Returns the new feed id. Adding checks the address is a feed first (`feed::find_feed`); this
-/// still added -- under a slug derived from its URL -- rather than refused.
+/// still names one it cannot read from its URL rather than failing.
 pub async fn add_one(
     ctx: &Ctx,
     cfg: &mut config::Config,

src/web.rs

@@ -1282,8 +1282,11 @@ async fn add_feed(
     Json(body): Json<NewFeed>,
 ) -> Result<Json<serde_json::Value>, ApiError> {
     let mut cfg = (*state.ctx.cfg()).clone();
-    // Before the duplicate check, so a site's page finds the feed someone already has.
+    // Before the duplicate check, so a site's page finds the feed someone already has. What is
-    let url = crate::feed::feed_behind_page(&state.ctx.client, &crate::feed::expand_input(&body.url)).await;
+    // 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?

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());

tests/ui/fixtures/linked.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0"?>
+<rss version="2.0"><channel><title>Linked Site</title><link>http://127.0.0.1:8792/site.html</link>
+<item><title>Linked Post</title><guid>linked-1</guid></item>
+</channel></rss>

tests/ui/fixtures/nofeed.html

@@ -0,0 +1,2 @@
+<!doctype html>
+<html><head><title>No Feed Here</title></head><body>A site that links no feed.</body></html>

tests/ui/fixtures/site.html

@@ -0,0 +1,4 @@
+<!doctype html>
+<html><head><title>A Site</title>
+<link rel="alternate" type="application/rss+xml" href="/linked.xml">
+</head><body>A site with a feed.</body></html>