Both add paths, the CLI's and the web's, look behind the URL first: a web page that names its feed with <link rel="alternate"> is swapped for that feed, before the duplicate check so it finds a feed someone already has. Before, the page itself was added and every scan failed on it. alternate_feed_link found tags in a to_lowercase() copy and sliced the original at those offsets; Unicode lowercasing changes some characters' length, so a page with one before its <link> tags lost the href or panicked off a char boundary. ASCII lowercasing keeps offsets aligned. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
### Added
|
### Added
|
||||||
|
|
||||||
- A feed that has no artwork of its own shows its website's icon instead.
|
- A feed that has no artwork of its own shows its website's icon instead.
|
||||||
|
- Adding a website's address subscribes to the feed that site links, instead of failing on every scan.
|
||||||
- `/api/status` gives the number of feeds, items waiting to download and files downloaded, and
|
- `/api/status` gives the number of feeds, items waiting to download and files downloaded, and
|
||||||
the version, for a dashboard such as Homepage.
|
the version, for a dashboard such as Homepage.
|
||||||
|
|
||||||
@@ -29,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
|
- A web page with some non-ASCII characters no longer hides the feed it links, or crashes looking for it.
|
||||||
- Pulling the item list down to check for new items shows a spinner for a couple of seconds, and
|
- Pulling the item list down to check for new items shows a spinner for a couple of seconds, and
|
||||||
a second pull meanwhile does nothing, instead of no sign at all that the check started.
|
a second pull meanwhile does nothing, instead of no sign at all that the check started.
|
||||||
- Settings no longer lists the server's download folder, which only an admin can change, on
|
- Settings no longer lists the server's download folder, which only an admin can change, on
|
||||||
|
|||||||
28
src/feed.rs
28
src/feed.rs
@@ -332,7 +332,9 @@ fn plain_text(bytes: &[u8]) -> Option<String> {
|
|||||||
/// Letters of Note, the Daily Dot, Hell Gate, The Frame Lab and Daily Kos.
|
/// Letters of Note, the Daily Dot, Hell Gate, The Frame Lab and Daily Kos.
|
||||||
fn alternate_feed_link(bytes: &[u8]) -> Option<String> {
|
fn alternate_feed_link(bytes: &[u8]) -> Option<String> {
|
||||||
let text = String::from_utf8_lossy(bytes);
|
let text = String::from_utf8_lossy(bytes);
|
||||||
let lower = text.to_lowercase();
|
// ASCII only: to_lowercase changes some characters' length (U+0130 grows a byte), and the
|
||||||
|
// offsets found in the lowered copy then sliced the original off a char boundary.
|
||||||
|
let lower = text.to_ascii_lowercase();
|
||||||
let mut pos = 0;
|
let mut pos = 0;
|
||||||
while let Some(rel) = lower[pos..].find("<link") {
|
while let Some(rel) = lower[pos..].find("<link") {
|
||||||
let start = pos + rel;
|
let start = pos + rel;
|
||||||
@@ -351,6 +353,22 @@ fn alternate_feed_link(bytes: &[u8]) -> Option<String> {
|
|||||||
None
|
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()?;
|
||||||
|
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()
|
||||||
|
};
|
||||||
|
found.await.map(String::from).unwrap_or_else(|| url.to_owned())
|
||||||
|
}
|
||||||
|
|
||||||
/// Artwork for a feed that has none: the icon its site's page names, or else the site's
|
/// Artwork for a feed that has none: the icon its site's page names, or else the site's
|
||||||
/// `/favicon.ico`. None if neither is there.
|
/// `/favicon.ico`. None if neither is there.
|
||||||
pub async fn site_icon(client: &reqwest::Client, site: &str) -> Option<String> {
|
pub async fn site_icon(client: &reqwest::Client, site: &str) -> Option<String> {
|
||||||
@@ -1216,6 +1234,14 @@ mod tests {
|
|||||||
assert_eq!(f.entries[2].enclosures.len(), 0, "an item may have none");
|
assert_eq!(f.entries[2].enclosures.len(), 0, "an item may have none");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_feed_link_after_non_ascii_text_is_found() {
|
||||||
|
// U+0130 lowercases to three bytes from two, which once shifted every offset after it.
|
||||||
|
let page = "<html><title>\u{130}stanbul \u{130}\u{130}</title>\
|
||||||
|
<link rel=\"alternate\" type=\"application/rss+xml\" href=\"/feed.xml\">";
|
||||||
|
assert_eq!(alternate_feed_link(page.as_bytes()).as_deref(), Some("/feed.xml"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn page_icon_prefers_the_touch_icon() {
|
fn page_icon_prefers_the_touch_icon() {
|
||||||
let page = br#"<head><link rel="stylesheet" href="/a.css">
|
let page = br#"<head><link rel="stylesheet" href="/a.css">
|
||||||
|
|||||||
@@ -606,7 +606,7 @@ async fn add(
|
|||||||
keywords: Vec<String>,
|
keywords: Vec<String>,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
let mut cfg = (*ctx.cfg()).clone();
|
let mut cfg = (*ctx.cfg()).clone();
|
||||||
let url = &feed::expand_input(url);
|
let url = &feed::feed_behind_page(&ctx.client, &feed::expand_input(url)).await;
|
||||||
// Includes feeds derived from an OPML, or the same show could be added twice.
|
// 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)) {
|
if let Some(existing) = subscriptions(ctx).await?.iter().find(|s| feed::same_feed(&s.cfg.url, url)) {
|
||||||
anyhow::bail!("already subscribed as {:?}", existing.id);
|
anyhow::bail!("already subscribed as {:?}", existing.id);
|
||||||
|
|||||||
@@ -1279,7 +1279,8 @@ async fn add_feed(
|
|||||||
Json(body): Json<NewFeed>,
|
Json(body): Json<NewFeed>,
|
||||||
) -> Result<Json<serde_json::Value>, ApiError> {
|
) -> Result<Json<serde_json::Value>, ApiError> {
|
||||||
let mut cfg = (*state.ctx.cfg()).clone();
|
let mut cfg = (*state.ctx.cfg()).clone();
|
||||||
let url = crate::feed::expand_input(&body.url);
|
// 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;
|
||||||
// Someone else may already have it. Then adding costs nothing: no second fetch, no
|
// 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.
|
// second copy on disk, just another name against the same feed.
|
||||||
if let Some(existing) = crate::subscriptions(&state.ctx).await?
|
if let Some(existing) = crate::subscriptions(&state.ctx).await?
|
||||||
|
|||||||
Reference in New Issue
Block a user