diff --git a/CLAUDE.md b/CLAUDE.md index ab71c5f..7eea16f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Overview -Feedr is a terminal-based RSS/Atom feed reader built with Rust, using ratatui/crossterm for the TUI. It supports feed management, categorization, filtering, dual themes, OPML import, auto-refresh with per-domain rate limiting, feed auto-discovery from HTML pages, configurable keybindings, mouse support, a help overlay, and newsboat-style external-command hooks (macros and `exec_on_new`). +Feedr is a terminal-based RSS/Atom feed reader built with Rust, using ratatui/crossterm for the TUI. It supports feed management, categorization, filtering, dual themes, OPML import, auto-refresh with per-domain rate limiting, feed auto-discovery from HTML pages, configurable keybindings, mouse support, a help overlay, newsboat-style external-command hooks (macros and `exec_on_new`), and Mozilla-Readability full-text article extraction. ## Build & Development Commands @@ -65,7 +65,8 @@ MSRV: 1.75.0. CI runs tests on stable, beta, and 1.75.0. - **Authenticated feeds**: `feed_headers: HashMap>` in `App` maps feed URLs to custom HTTP headers. Built from `config.default_feeds` entries that have `headers`. Passed to `Feed::fetch_url()` at all fetch call sites. - **Compact mode**: `app.compact` bool is updated each frame by `update_compact_mode(terminal_height)`. Rendering in `ui.rs` branches on `app.compact` for layout, title bar, help bar, and dashboard item format. Controlled by `config.ui.compact_mode` (`Auto`/`Always`/`Never`). Dialog modals use `centered_rect_with_min()` to enforce minimum dimensions regardless of compact mode. - **External-command hooks (macros + `exec_on_new`)**: Commands are run **without a shell**. Templates are tokenized once at config load via `shlex`, then `expand_argv_template` substitutes `%X` placeholders into individual argv tokens (no re-expansion), so feed content cannot break out of an argument. The macro engine queues steps into `app.pending_macro_steps` from `events.rs` and the TUI loop drains them in `tui.rs::drain_macro_steps` — drain lives at the loop level because `pipe-to` needs the terminal handle to suspend the TUI. Chains halt on the first step error (tracked via a `pre_error` guard so a stale `app.error` doesn't spuriously abort). The macro prefix (default `,`) is checked at the top of `handle_key_event` and only when `input_mode == Normal`, so text-input modes are not disturbed; an idle prefix times out via the existing success-message timeout. -- **`exec_on_new` crash semantics**: AT-MOST-ONCE. `flush_exec_on_new` persists the `seen_items` / `feeds_seeded` sets *before* spawning any child, so a kill mid-fire loses a notification rather than re-firing on the next launch. `mark_feed_seen` only flips `feeds_seeded` on a fetch that returned items (transiently-empty first fetches don't seed), and the first observation of a feed seeds the seen-set silently to avoid a firehose. Children are spawned detached with stdio nulled; a reaper thread waits on each so they don't linger as zombies. The seen-set is pruned in `remove_current_feed` to prevent monotonic growth across feed churn. +- **`exec_on_new` crash semantics**: AT-MOST-ONCE. `flush_exec_on_new` persists the `seen_items` / `feeds_seeded` sets *before* spawning any child, so a kill mid-fire loses a notification rather than re-firing on the next launch. `mark_feed_seen` only flips `feeds_seeded` on a fetch that returned items (transiently-empty first fetches don't seed), and the first observation of a feed seeds the seen-set silently to avoid a firehose. Children are spawned detached with stdio nulled; a reaper thread waits on each so they don't linger as zombies. The seen-set is pruned in `remove_current_feed` to prevent monotonic growth across feed churn. **Single-shared mark per feed**: `mark_feed_seen` is hoisted to the feed-drain call site in `tui.rs` (gated on `exec_on_new_template.is_some() || fulltext_feeds.contains(&feed.url)`) so multiple consumers (currently exec_on_new and fulltext) share one mark per feed arrival — calling it twice would double-mark and the second consumer would see an empty `newly_seen` list. +- **Full-text extraction**: `feed::extract_article` fetches an article URL with the existing `reqwest::blocking::Client` and runs Mozilla Readability via the `dom_smoothie` crate. **Sync, no tokio.** Per-feed `Authorization`/auth headers are intentionally NOT forwarded to article URLs (they're third-party hosts — propagating would be a credential leak). The response body is read via `Response::take(FULLTEXT_MAX_BYTES+1).read_to_end(...)` so peak allocation is hard-capped at ~5 MB regardless of what the server sends, and the response charset is honored (`Content-Type charset=` → `` sniff → UTF-8) via `encoding_rs` so non-UTF8 pages don't mojibake. Each extraction runs on a `std::thread::spawn` worker wrapped in `catch_unwind` (so a `dom_smoothie` panic on hostile HTML surfaces as `Failed("…panicked…")` instead of stranding the slot on `Pending` forever). The TUI loop maintains a `Arc` `extract_inflight` budget capped at `EXTRACTION_MAX_INFLIGHT = 4`; queued requests beyond that budget — or whose domain was last fetched less than `refresh_rate_limit_delay` ago — are pushed back onto `pending_extraction_requests` to retry on the next loop tick. State lives only in `App::extracted: HashMap` (`Pending` / `Ready(ExtractedArticle)` / `Failed(String)`) — **in-memory only**, never persisted; LRU-capped at `EXTRACTED_CACHE_CAP = 500` with insertion-order tracking via `extracted_order: VecDeque`. The cap is **hard**: `insert_extraction` always evicts the deque head when full, and `record_extraction_result` rejects results for slots that aren't currently `Pending` (so a late worker for an evicted / removed id is dropped rather than resurrecting dead state). `Shift+F` (`KeyAction::FetchFullText`) toggles between summary and extracted text when `Ready`, queues a new request when absent, and re-queues on `Failed` (so the user can retry). Per-feed `fulltext = true` in config auto-extracts newly-seen items on refresh (same `mark_feed_seen` "newly seen" semantics as `exec_on_new` — first fetch seeds silently, no firehose); the auto path additionally filters via `feed::is_safe_auto_url` (http/https only, rejects RFC1918 / loopback / link-local / CGNAT / multicast / 6to4 / NAT64 / `localhost`-style names) to prevent a hostile feed from probing internal hosts. The auto path's worker also uses `Feed::build_safe_redirect_client`, whose `redirect::Policy::custom` re-runs `is_safe_auto_url` on every hop, so a public-looking `` that 302s into an internal target is rejected mid-chain instead of slipping past the upfront URL check. Each `ExtractionRequest` carries a `safe_redirects` flag (true for auto, false for manual `Shift+F`); the spawn loop picks the matching client per-request. Manual `Shift+F` bypasses both the URL allowlist and the safe-redirect client (it's the user's explicit action, same trust model as opening the article in a browser). The spawn loop also gates each pop on the slot still being `Pending`, so requests whose `extracted` entry was evicted by LRU or pruned by `remove_current_feed` get dropped without spawning a worker, and uses `std::thread::Builder::new().spawn()` so an OS thread-creation failure releases the inflight slot and re-queues the request instead of crashing the TUI. The detail-view lookup uses `current_article_indices()` (same resolver as the action handler) so they stay in lockstep. Extracted entries are pruned alongside `seen_items` in `remove_current_feed` via the `remove_extraction(&id)` helper. ## Commit Conventions diff --git a/Cargo.lock b/Cargo.lock index cc431c1..0f4936f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -168,6 +168,21 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -380,7 +395,20 @@ dependencies = [ "cssparser-macros", "dtoa-short", "itoa", - "phf", + "phf 0.10.1", + "smallvec", +] + +[[package]] +name = "cssparser" +version = "0.36.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dae61cf9c0abb83bd659dab65b7e4e38d8236824c85f0f804f173567bda257d2" +dependencies = [ + "cssparser-macros", + "dtoa-short", + "itoa", + "phf 0.13.1", "smallvec", ] @@ -405,6 +433,27 @@ dependencies = [ "syn 2.0.100", ] +[[package]] +name = "derive_more" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d751e9e49156b02b44f9c1815bcb94b984cdcc4396ecc32521c739452808b134" +dependencies = [ + "derive_more-impl", +] + +[[package]] +name = "derive_more-impl" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "799a97264921d8623a957f6c3b9011f3b5492f557bbb7a5a19b7fa6d06ba8dcb" +dependencies = [ + "proc-macro2", + "quote", + "rustc_version", + "syn 2.0.100", +] + [[package]] name = "dirs" version = "5.0.1" @@ -437,6 +486,40 @@ dependencies = [ "syn 2.0.100", ] +[[package]] +name = "dom_query" +version = "0.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "521e380c0c8afb8d9a1e83a1822ee03556fc3e3e7dbc1fd30be14e37f9cb3f89" +dependencies = [ + "bit-set", + "cssparser 0.36.0", + "foldhash", + "html5ever 0.38.0", + "nom", + "precomputed-hash", + "selectors 0.36.1", + "tendril 0.5.0", +] + +[[package]] +name = "dom_smoothie" +version = "0.17.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f21cad308997c8c518aaee15c8b38bb04dc699bbcf30da4176443c9ef40c1bb5" +dependencies = [ + "dom_query", + "flagset", + "foldhash", + "gjson", + "html-escape", + "once_cell", + "phf 0.13.1", + "tendril 0.5.0", + "thiserror 2.0.18", + "unicode-segmentation", +] + [[package]] name = "dtoa" version = "1.0.11" @@ -521,6 +604,8 @@ dependencies = [ "clap", "crossterm", "dirs", + "dom_smoothie", + "encoding_rs", "feed-rs", "html2text", "open", @@ -537,6 +622,12 @@ dependencies = [ "uuid", ] +[[package]] +name = "flagset" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7ac824320a75a52197e8f2d787f6a38b6718bb6897a35142d749af3c0e8f4fe" + [[package]] name = "flate2" version = "1.1.2" @@ -553,6 +644,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" + [[package]] name = "foreign-types" version = "0.3.2" @@ -682,6 +779,12 @@ version = "0.31.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" +[[package]] +name = "gjson" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43503cc176394dd30a6525f5f36e838339b8b5619be33ed9a7783841580a97b6" + [[package]] name = "h2" version = "0.3.26" @@ -744,15 +847,24 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "html-escape" +version = "0.2.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d1ad449764d627e22bfd7cd5e8868264fc9236e07c752972b4080cd351cb476" +dependencies = [ + "utf8-width", +] + [[package]] name = "html2text" version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74cda84f06c1cc83476f79ae8e2e892b626bdadafcb227baec54c918cadc18a0" dependencies = [ - "html5ever", - "markup5ever", - "tendril", + "html5ever 0.26.0", + "markup5ever 0.11.0", + "tendril 0.4.3", "unicode-width 0.1.14", "xml5ever", ] @@ -765,12 +877,22 @@ checksum = "bea68cab48b8459f17cf1c944c67ddc572d272d9f2b274140f223ecb1da4a3b7" dependencies = [ "log", "mac", - "markup5ever", + "markup5ever 0.11.0", "proc-macro2", "quote", "syn 1.0.109", ] +[[package]] +name = "html5ever" +version = "0.38.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1054432bae2f14e0061e33d23402fbaa67a921d319d56adc6bcf887ddad1cbc2" +dependencies = [ + "log", + "markup5ever 0.38.0", +] + [[package]] name = "http" version = "0.2.12" @@ -1127,11 +1249,22 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a2629bb1404f3d34c2e921f21fd34ba00b206124c81f65c50b43b6aaefeb016" dependencies = [ "log", - "phf", - "phf_codegen", - "string_cache", - "string_cache_codegen", - "tendril", + "phf 0.10.1", + "phf_codegen 0.10.0", + "string_cache 0.8.9", + "string_cache_codegen 0.5.4", + "tendril 0.4.3", +] + +[[package]] +name = "markup5ever" +version = "0.38.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8983d30f2915feeaaab2d6babdd6bc7e9ed1a00b66b5e6d74df19aa9c0e91862" +dependencies = [ + "log", + "tendril 0.5.0", + "web_atoms", ] [[package]] @@ -1210,6 +1343,15 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086" +[[package]] +name = "nom" +version = "8.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df9761775871bdef83bee530e60050f7e54b1105350d6884eb0fb4f46c2f9405" +dependencies = [ + "memchr", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -1302,7 +1444,7 @@ checksum = "df2f96426c857a92676dc29a9e2a181eb39321047ac994491c69eae01619ddf2" dependencies = [ "hard-xml", "serde", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -1358,11 +1500,22 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fabbf1ead8a5bcbc20f5f8b939ee3f5b0f6f281b6ad3468b84656b658b455259" dependencies = [ - "phf_macros", + "phf_macros 0.10.0", "phf_shared 0.10.0", "proc-macro-hack", ] +[[package]] +name = "phf" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1562dc717473dbaa4c1f85a36410e03c047b2e7df7f45ee938fbef64ae7fadf" +dependencies = [ + "phf_macros 0.13.1", + "phf_shared 0.13.1", + "serde", +] + [[package]] name = "phf_codegen" version = "0.10.0" @@ -1373,6 +1526,16 @@ dependencies = [ "phf_shared 0.10.0", ] +[[package]] +name = "phf_codegen" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "49aa7f9d80421bca176ca8dbfebe668cc7a2684708594ec9f3c0db0805d5d6e1" +dependencies = [ + "phf_generator 0.13.1", + "phf_shared 0.13.1", +] + [[package]] name = "phf_generator" version = "0.10.0" @@ -1393,6 +1556,16 @@ dependencies = [ "rand", ] +[[package]] +name = "phf_generator" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "135ace3a761e564ec88c03a77317a7c6b80bb7f7135ef2544dbe054243b89737" +dependencies = [ + "fastrand", + "phf_shared 0.13.1", +] + [[package]] name = "phf_macros" version = "0.10.0" @@ -1407,6 +1580,19 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "phf_macros" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "812f032b54b1e759ccd5f8b6677695d5268c588701effba24601f6932f8269ef" +dependencies = [ + "phf_generator 0.13.1", + "phf_shared 0.13.1", + "proc-macro2", + "quote", + "syn 2.0.100", +] + [[package]] name = "phf_shared" version = "0.10.0" @@ -1425,6 +1611,15 @@ dependencies = [ "siphasher 1.0.1", ] +[[package]] +name = "phf_shared" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e57fef6bc5981e38c2ce2d63bfa546861309f875b8a75f092d1d54ae2d64f266" +dependencies = [ + "siphasher 1.0.1", +] + [[package]] name = "pin-project-lite" version = "0.2.16" @@ -1466,9 +1661,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.94" +version = "1.0.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a31971752e70b8b2686d7e46ec17fb38dad4051d94024c88df49b667caea9c84" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" dependencies = [ "unicode-ident", ] @@ -1562,7 +1757,7 @@ checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" dependencies = [ "getrandom 0.2.15", "libredox", - "thiserror", + "thiserror 1.0.69", ] [[package]] @@ -1642,6 +1837,21 @@ version = "0.1.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" +[[package]] +name = "rustc-hash" +version = "2.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94300abf3f1ae2e2b8ffb7b58043de3d399c73fa6f4b73826402a5c457614dbe" + +[[package]] +name = "rustc_version" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfcb3a22ef46e85b45de6ee7e79d063319ebb6594faafcf1c225ea92ab6e9b92" +dependencies = [ + "semver", +] + [[package]] name = "rustix" version = "1.1.2" @@ -1698,13 +1908,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "585480e3719b311b78a573db1c9d9c4c1f8010c2dee4cc59c2efe58ea4dbc3e1" dependencies = [ "ahash", - "cssparser", + "cssparser 0.31.2", "ego-tree", "getopts", - "html5ever", + "html5ever 0.26.0", "once_cell", - "selectors", - "tendril", + "selectors 0.25.0", + "tendril 0.4.3", ] [[package]] @@ -1737,18 +1947,43 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4eb30575f3638fc8f6815f448d50cb1a2e255b0897985c8c59f4d37b72a07b06" dependencies = [ "bitflags 2.9.0", - "cssparser", - "derive_more", + "cssparser 0.31.2", + "derive_more 0.99.20", "fxhash", "log", "new_debug_unreachable", - "phf", - "phf_codegen", + "phf 0.10.1", + "phf_codegen 0.10.0", + "precomputed-hash", + "servo_arc 0.3.0", + "smallvec", +] + +[[package]] +name = "selectors" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c5d9c0c92a92d33f08817311cf3f2c29a3538a8240e94a6a3c622ce652d7e00c" +dependencies = [ + "bitflags 2.9.0", + "cssparser 0.36.0", + "derive_more 2.1.1", + "log", + "new_debug_unreachable", + "phf 0.13.1", + "phf_codegen 0.13.1", "precomputed-hash", - "servo_arc", + "rustc-hash", + "servo_arc 0.4.3", "smallvec", ] +[[package]] +name = "semver" +version = "1.0.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a7852d02fc848982e0c167ef163aaff9cd91dc640ba85e263cb1ce46fae51cd" + [[package]] name = "serde" version = "1.0.219" @@ -1811,6 +2046,15 @@ dependencies = [ "stable_deref_trait", ] +[[package]] +name = "servo_arc" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "170fb83ab34de17dc69aa7c67482b22218ddb85da56546f9bd6b929e32a05930" +dependencies = [ + "stable_deref_trait", +] + [[package]] name = "shlex" version = "1.3.0" @@ -1903,6 +2147,18 @@ dependencies = [ "serde", ] +[[package]] +name = "string_cache" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a18596f8c785a729f2819c0f6a7eae6ebeebdfffbfe4214ae6b087f690e31901" +dependencies = [ + "new_debug_unreachable", + "parking_lot", + "phf_shared 0.13.1", + "precomputed-hash", +] + [[package]] name = "string_cache_codegen" version = "0.5.4" @@ -1915,6 +2171,18 @@ dependencies = [ "quote", ] +[[package]] +name = "string_cache_codegen" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "585635e46db231059f76c5849798146164652513eb9e8ab2685939dd90f29b69" +dependencies = [ + "phf_generator 0.13.1", + "phf_shared 0.13.1", + "proc-macro2", + "quote", +] + [[package]] name = "strsim" version = "0.11.1" @@ -2027,13 +2295,32 @@ dependencies = [ "utf-8", ] +[[package]] +name = "tendril" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4790fc369d5a530f4b544b094e31388b9b3a37c0f4652ade4505945f5660d24" +dependencies = [ + "new_debug_unreachable", + "utf-8", +] + [[package]] name = "thiserror" version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" dependencies = [ - "thiserror-impl", + "thiserror-impl 1.0.69", +] + +[[package]] +name = "thiserror" +version = "2.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" +dependencies = [ + "thiserror-impl 2.0.18", ] [[package]] @@ -2047,6 +2334,17 @@ dependencies = [ "syn 2.0.100", ] +[[package]] +name = "thiserror-impl" +version = "2.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.100", +] + [[package]] name = "tinystr" version = "0.7.6" @@ -2215,6 +2513,12 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c8232dd3cdaed5356e0f716d285e4b40b932ac434100fe9b7e0e8e935b9e6246" +[[package]] +name = "utf8-width" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1292c0d970b54115d14f2492fe0170adf21d68a1de108eebc51c1df4f346a091" + [[package]] name = "utf8_iter" version = "1.0.4" @@ -2354,6 +2658,18 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "web_atoms" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7cff6eef815df1834fd250e3a2ff436044d82a9f1bc1980ca1dbdf07effc538" +dependencies = [ + "phf 0.13.1", + "phf_codegen 0.13.1", + "string_cache 0.9.0", + "string_cache_codegen 0.6.1", +] + [[package]] name = "winapi" version = "0.3.9" @@ -2768,7 +3084,7 @@ checksum = "4034e1d05af98b51ad7214527730626f019682d797ba38b51689212118d8e650" dependencies = [ "log", "mac", - "markup5ever", + "markup5ever 0.11.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 8ee8fab..8f08ddc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,15 @@ toml = "0.8" scraper = "0.18" url = "2" shlex = "1.3" +# dom_smoothie is pre-1.0 and is the security-critical HTML parsing surface +# for the full-text extraction path. Cargo.lock is committed (so the actual +# version pin is enforced there), and the default `"0.17"` (=caret) already +# blocks 0.18+ on pre-1.0 crates. The explicit `~0.17` here is intent +# signaling — it makes "patch-only updates allowed; major-version bumps +# require manual review" obvious at the manifest level rather than only in +# the lockfile. +dom_smoothie = "~0.17" +encoding_rs = "0.8" [profile.release] codegen-units = 1 diff --git a/README.md b/README.md index c29c125..384adb6 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ Feedr is a feature-rich terminal-based RSS feed reader written in Rust. It provi - **Mark All Read**: Quickly mark all visible items as read with `m` - **Article Preview**: Toggle an inline preview pane in the dashboard view - **Link Extraction**: Extract and browse all links from an article with `l` +- **Full-Text Extraction**: Strip away summaries and read the actual article content inline via Mozilla Readability — manual on `Shift+F`, or auto-extract on refresh per feed with `fulltext = true` - **Help Overlay**: Press `?` for a scrollable keybinding reference overlay - **OPML Import**: Bulk import feeds from OPML files via `feedr --import ` - **Browser Integration**: Open articles in your default browser @@ -175,6 +176,7 @@ All keybindings below show their defaults. You can remap any action via the `[ke | `s` | Toggle starred | | `o` | Open item in browser | | `l` | Extract and show all links | +| `Shift+F` | Toggle/fetch full-text (Readability) | #### Starred View | Key | Action | @@ -314,6 +316,23 @@ Cookie = "session=abc123" ``` Headers are sent with every request for that feed, including refreshes. +#### Full-Text Extraction +Most RSS feeds ship only short summaries. Feedr can fetch the linked article URL and run [Mozilla Readability](https://github.com/mozilla/readability) (via the `dom_smoothie` crate) to extract the actual article body and render it inline. + +- **Manual**: in the article detail view, press `Shift+F` to extract the focused article. Press `Shift+F` again to toggle back to the original summary, or after a failure to retry. +- **Auto on refresh**: set `fulltext = true` on a feed and Feedr will auto-extract newly-seen items on each refresh (same "no firehose" rule as `exec_on_new` — the first observation of a feed seeds silently). + +```toml +[[default_feeds]] +url = "https://example.com/summary-only-feed.xml" +fulltext = true +``` + +Notes: +- Extracted content is **in-memory only** — it is not persisted to disk. A restart re-extracts on demand. +- Per-feed auth headers are **not** sent to the article URL (article URLs are typically third-party hosts; forwarding `Authorization` would leak credentials). +- Pages with very short extracted bodies (likely JS-rendered or behind a wall) fail gracefully and fall back to showing the original summary. + ### External-Command Hooks Feedr supports newsboat-style external commands for two workflows: **macros** (key-triggered chains that act on the focused article) and **`exec_on_new`** (a notification hook fired per newly-seen item after each refresh). @@ -417,6 +436,7 @@ toggle_theme = "F5" # Function keys | `open_category_management` | `Ctrl+c` | Category management | | `assign_category` | `c` | Assign category to feed | | `extract_links` | `l` | Extract links from article | +| `fetch_full_text` | `Shift+F` | Toggle/fetch full-text (Readability) | | `scroll_preview_up` | `Shift+K`, `Shift+Up` | Scroll preview up | | `scroll_preview_down` | `Shift+J`, `Shift+Down` | Scroll preview down | | `toggle_expand` | `Space` | Expand/collapse in tree view | diff --git a/src/app.rs b/src/app.rs index f862b29..d9dd304 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,5 +1,5 @@ use crate::config::{CompactMode, Config}; -use crate::feed::{Feed, FeedCategory, FeedItem}; +use crate::feed::{ExtractedArticle, Feed, FeedCategory, FeedItem}; use crate::ui::ColorScheme; use anyhow::Result; use chrono::{DateTime, Utc}; @@ -143,6 +143,38 @@ pub struct App { /// disabled or the template failed to tokenize at startup (in which case /// a startup warning is surfaced). pub exec_on_new_template: Option>, + /// Per-item full-text extraction cache. Keyed by the same item_id scheme + /// as `read_items` / `starred_items`. In-memory only; not persisted. + pub extracted: HashMap, + /// Insertion order for `extracted`, used to drop the oldest entry when + /// the cache hits `EXTRACTED_CACHE_CAP`. + pub extracted_order: VecDeque, + /// Feed URLs that have `fulltext = true` in config and should trigger + /// auto-extraction for newly-seen items on each refresh. Same derivation + /// pattern as `feed_headers` / `feed_refresh_intervals`. + pub fulltext_feeds: HashSet, + /// When the focused article has a `Ready` extraction, this flag controls + /// whether the detail view renders the extracted text or the original + /// summary. Single global toggle (not per-item) — simplest UX and the + /// detail view only ever shows one article at a time. + pub show_extracted: bool, + /// `(feed_idx, item_idx)` of the article the detail view rendered last + /// frame. When the focus moves to a different article, `show_extracted` + /// is reset to `true` so the user doesn't carry over a "view summary" + /// toggle from one article to another (and end up staring at a summary + /// while the title bar labels the body `Full-text`). Only the detail + /// view writes to / consults this — other views resolve focus through + /// `current_article_indices` directly. + pub last_detail_focus: Option<(usize, usize)>, + /// Queue of extraction requests for the TUI loop to spawn worker threads + /// for. Decouples request-time (event handler) from spawn-time (loop + /// drain) so events.rs doesn't need the thread-handle / channel. + pub pending_extraction_requests: VecDeque, + /// Monotonic generation counter for extraction requests. Each new + /// `Pending` slot gets a fresh value so a worker for a previously + /// evicted-and-recreated slot can't accidentally satisfy the new + /// request — `record_extraction_result` matches the generation. + pub next_extraction_generation: u64, } /// Snapshot of the focused article passed to template-expansion / pipe-payload helpers. @@ -191,6 +223,75 @@ pub enum AddFeedResult { }, } +/// State of a per-item full-text extraction. Lives only in-memory: a +/// restart re-fetches on demand or via the next refresh for fulltext-flagged +/// feeds. No on-disk persistence by design — saves us from cache-invalidation +/// logic and bounds storage growth. +#[derive(Clone, Debug)] +pub enum ExtractionState { + /// In-flight. `generation` matches the generation stamped on the + /// `ExtractionRequest` that produced this slot; `started_at` drives the + /// stale-Pending watchdog. The generation closes a TOCTOU window where + /// a Pending slot is evicted from the LRU and re-requested for the same + /// id — without the tag, the old worker's eventual result would land on + /// the new slot. + /// + /// `spawned` flips to true only after the spawn loop has both bumped + /// the inflight counter AND successfully called `thread::Builder::spawn`. + /// A request that ages out while still queued (rate-limited behind a + /// backlog longer than the watchdog window) is `spawned == false`, and + /// the watchdog MUST NOT release an inflight slot for it — there was no + /// permit to release, and a spurious release would shrink the effective + /// pool below `EXTRACTION_MAX_INFLIGHT`. + Pending { + generation: u64, + started_at: Instant, + spawned: bool, + }, + Ready(ExtractedArticle), + Failed(String), +} + +/// A queued request for the TUI loop to spawn a worker thread for. Filled +/// in `request_extraction_for_current` and `enqueue_auto_extractions`, +/// drained in `tui.rs` each tick (same shape as `pending_macro_steps`). +#[derive(Clone, Debug)] +pub struct ExtractionRequest { + pub id: String, + pub url: String, + /// When true, the worker must use a client whose redirect policy re-runs + /// `is_safe_auto_url` on every hop. Set for the auto-fulltext path + /// (URLs come from feed XML, untrusted) and cleared for manual `Shift+F` + /// (user already chose the article — same trust model as opening it in + /// a browser). + pub safe_redirects: bool, + /// Generation tag matching the `Pending` slot this request was created + /// for. Worker echoes it back through the result channel so the receiver + /// can drop late results from stale (evicted-and-recreated) slots. + pub generation: u64, +} + +/// Cap on the number of cached extracted articles. Auto-fetch over a big +/// feed could otherwise accumulate state monotonically across a long +/// session. Insertion-order LRU eviction (oldest cache entries are dropped +/// first) — `extracted_order` tracks the insertion sequence. +/// +/// Soft cap, not hard: `insert_extraction` refuses to evict +/// `Pending { spawned: true }` slots — evicting one would hide it from the +/// stale-Pending watchdog (which only looks at `extracted`) and leak the +/// worker's inflight permit if the worker is dead. The number of such slots +/// at any moment is bounded by the worker-pool size (see +/// `EXTRACTION_MAX_INFLIGHT` in `tui.rs`), so the effective cap grows by at +/// most that constant. +pub const EXTRACTED_CACHE_CAP: usize = 500; + +/// Stale-`Pending` watchdog timeout, as a multiple of `http_timeout`. A worker +/// whose Pending slot is older than `http_timeout * PENDING_WATCHDOG_MULTIPLIER` +/// is assumed dead (OS kill / SIGSEGV in a native dep) and flipped to +/// `Failed("timed out")` so the user can retry. Multiplier covers slow DNS, +/// TLS handshakes, and Readability CPU time on top of the network timeout. +pub const PENDING_WATCHDOG_MULTIPLIER: u32 = 3; + #[derive(Serialize, Deserialize)] struct SavedData { bookmarks: Vec, @@ -256,6 +357,14 @@ impl App { .filter_map(|f| f.refresh_interval.map(|interval| (f.url.clone(), interval))) .collect(); + // Build the set of URLs that opted into full-text auto-extraction + let fulltext_feeds: HashSet = config + .default_feeds + .iter() + .filter(|f| f.fulltext.unwrap_or(false)) + .map(|f| f.url.clone()) + .collect(); + // Parse last session time from saved data let last_session_time = saved_data .last_session_time @@ -351,6 +460,13 @@ impl App { feeds_seeded: saved_data.feeds_seeded, pending_macro_steps: VecDeque::new(), exec_on_new_template, + extracted: HashMap::new(), + extracted_order: VecDeque::new(), + fulltext_feeds, + show_extracted: true, + last_detail_focus: None, + pending_extraction_requests: VecDeque::new(), + next_extraction_generation: 0, }; app.update_dashboard(); @@ -908,10 +1024,19 @@ impl App { // feeds_seeded and its item IDs would stay in seen_items // forever, growing the persisted JSON file monotonically. self.feeds_seeded.remove(&url); + let mut removed_ids: Vec = Vec::new(); for item in &self.feeds[idx].items { let id = make_item_id(&self.feeds[idx], item); self.seen_items.remove(&id); + removed_ids.push(id); + } + // Drop in-memory extracted full-text for the removed items + // too — keeping them would never expire (no persistence) + // but they're keyed on item ids that no longer resolve. + for id in &removed_ids { + self.remove_extraction(id); } + self.fulltext_feeds.remove(&url); // Remove from feeds self.feeds.remove(idx); @@ -1439,7 +1564,7 @@ impl App { } /// Extract domain from URL (e.g., "reddit.com" from "https://www.reddit.com/r/rust/.rss") - fn extract_domain_from_url(url: &str) -> String { + pub(crate) fn extract_domain_from_url(url: &str) -> String { // Simple domain extraction if let Some(domain_start) = url.find("://") { let after_protocol = &url[domain_start + 3..]; @@ -1695,6 +1820,302 @@ impl App { ) -> Option<&crate::keybindings::MacroBinding> { self.macros.iter().find(|m| m.trigger.matches(key)) } + + /// Look up extraction state for an item by its `(feed_idx, item_idx)`. + /// Returns `None` if the indices don't resolve or no extraction has been + /// requested for that item. + pub fn extraction_state_for( + &self, + feed_idx: usize, + item_idx: usize, + ) -> Option<&ExtractionState> { + let id = self.get_item_id(feed_idx, item_idx); + if id.is_empty() { + None + } else { + self.extracted.get(&id) + } + } + + /// Land a worker result for `id`. Drops the result if the slot has been + /// evicted, removed (feed deleted), already taken to a non-Pending state, + /// or if the request's generation no longer matches the current Pending + /// slot — we never want to resurrect a dead cache entry or let a stale + /// worker satisfy a re-requested slot. + pub fn record_extraction_result( + &mut self, + id: String, + generation: u64, + result: Result, + ) { + if id.is_empty() { + return; + } + // Must be Pending AND the generation must match. The generation + // check closes the TOCTOU window where an LRU-evicted Pending slot + // gets re-requested: the old worker's result would otherwise land + // on the new Pending slot of the same id. + match self.extracted.get(&id) { + Some(ExtractionState::Pending { generation: g, .. }) if *g == generation => {} + _ => return, + } + let state = match result { + // `{:#}` flattens the anyhow cause chain into one line + // ("outer: caused by: inner"), preserving context that the + // default `{}` formatter drops. The user only ever sees the + // top message in the detail view, but a chained message helps + // when the failure is logged or reported. + Ok(article) => ExtractionState::Ready(article), + Err(e) => ExtractionState::Failed(format!("{:#}", e)), + }; + self.insert_extraction(id, state); + } + + /// Mark an item as `Pending` and append the request to the spawn queue. + /// No-op if the item already has any state (Pending / Ready / Failed) — + /// callers should clear or re-request explicitly. `safe_redirects` selects + /// whether the worker uses the safe-redirect client (auto path) or the + /// permissive one (manual path); see `ExtractionRequest::safe_redirects`. + /// `priority_front=true` jumps the queue: an explicit user click should + /// not wait behind a refresh's auto-extractions to the same host. + pub fn request_extraction( + &mut self, + id: String, + url: String, + safe_redirects: bool, + priority_front: bool, + ) { + if id.is_empty() || url.is_empty() { + return; + } + if self.extracted.contains_key(&id) { + return; + } + let generation = self.next_extraction_generation; + self.next_extraction_generation = self.next_extraction_generation.wrapping_add(1); + self.insert_extraction( + id.clone(), + ExtractionState::Pending { + generation, + started_at: Instant::now(), + spawned: false, + }, + ); + let req = ExtractionRequest { + id, + url, + safe_redirects, + generation, + }; + if priority_front { + self.pending_extraction_requests.push_front(req); + } else { + self.pending_extraction_requests.push_back(req); + } + } + + /// Sweep Pending slots that have been in-flight longer than + /// `http_timeout * PENDING_WATCHDOG_MULTIPLIER`, flip them to + /// `Failed("timed out")`, and return the count of pruned slots that + /// were actually *spawned* — so the caller can release exactly that + /// many inflight budget slots. + /// + /// A normally-completing worker (success, error, even a Rust panic caught + /// by `catch_unwind`) always sends a result back through the channel. A + /// worker that silently disappears — OS kill, SIGSEGV in a native dep, + /// parent thread crash — leaves its slot stuck on Pending forever, which + /// blocks `toggle_or_request_fulltext`'s `contains_key` guard. The + /// returned count is what `run_app` passes to a saturating decrement of + /// the shared `extract_inflight` counter so the pool doesn't leak budget + /// slots across a session. + /// + /// Critically, we only count pruned slots whose `spawned` flag is true: + /// a request that aged out while still queued never claimed an inflight + /// permit, so releasing one for it would silently widen the effective + /// worker cap. Both the watchdog path and the worker-exit path use + /// saturating decrement, so a slow worker that completes AFTER the + /// watchdog already released its slot can't underflow the counter. + pub fn prune_stale_pending_extractions(&mut self, http_timeout: std::time::Duration) -> usize { + let watchdog = http_timeout.saturating_mul(PENDING_WATCHDOG_MULTIPLIER); + if watchdog.is_zero() { + return 0; + } + let now = Instant::now(); + let mut to_fail: Vec<(String, bool)> = Vec::new(); + for (id, state) in self.extracted.iter() { + if let ExtractionState::Pending { + started_at, + spawned, + .. + } = state + { + if now.duration_since(*started_at) > watchdog { + to_fail.push((id.clone(), *spawned)); + } + } + } + let mut spawned_pruned = 0usize; + for (id, spawned) in to_fail { + if spawned { + spawned_pruned += 1; + } + // Mutate in place — must NOT route through `insert_extraction`, + // which promotes the entry to MRU. A watchdog-failed slot is + // strictly less useful than newer Ready entries, so it should + // keep its original LRU position and be evicted ahead of them. + if let Some(state) = self.extracted.get_mut(&id) { + *state = ExtractionState::Failed("timed out".to_string()); + } + } + spawned_pruned + } + + /// Mark a Pending slot as spawned (the spawn loop has bumped the inflight + /// counter and successfully called `thread::Builder::spawn`). Idempotent + /// — a no-op for any other state, so race conditions where the worker + /// already landed a result before this is called are harmless. + pub(crate) fn mark_extraction_spawned(&mut self, id: &str, generation: u64) { + if let Some(ExtractionState::Pending { + generation: g, + spawned, + .. + }) = self.extracted.get_mut(id) + { + if *g == generation { + *spawned = true; + } + } + } + + /// Forget a single extraction slot (both the state and its LRU + /// position). The deque is kept in sync with the map. + pub(crate) fn remove_extraction(&mut self, id: &str) { + self.extracted.remove(id); + if let Some(pos) = self.extracted_order.iter().position(|x| x.as_str() == id) { + self.extracted_order.remove(pos); + } + } + + fn insert_extraction(&mut self, id: String, state: ExtractionState) { + // If id is already present, move its LRU position to most-recent and + // overwrite the state. + if let Some(existing) = self.extracted.get_mut(&id) { + *existing = state; + if let Some(pos) = self.extracted_order.iter().position(|x| x == &id) { + self.extracted_order.remove(pos); + } + self.extracted_order.push_back(id); + return; + } + // LRU eviction with a soft-protect for `Pending { spawned: true }` + // slots. Evicting a spawned Pending would hide the slot from the + // stale-Pending watchdog (it iterates `extracted`), leaving a dead + // worker's inflight permit leaked for the rest of the session. + // Skip those slots and evict the next-oldest evictable entry. + // + // The protected-slot count is bounded by the worker pool (see + // `EXTRACTION_MAX_INFLIGHT` in `tui.rs`), so the cache size is + // bounded at `EXTRACTED_CACHE_CAP + EXTRACTION_MAX_INFLIGHT` — a + // soft cap, not hard, but still strictly bounded. If every entry + // happens to be a spawned Pending (would mean the entire pool has + // been stuck for a long time), the cache transiently grows past + // CAP; the watchdog will eventually flip those slots to Failed, + // restoring evictability on the next insert. + while self.extracted_order.len() >= EXTRACTED_CACHE_CAP { + let evict_idx = self.extracted_order.iter().position(|id| { + !matches!( + self.extracted.get(id), + Some(ExtractionState::Pending { spawned: true, .. }) + ) + }); + match evict_idx { + Some(idx) => { + // `VecDeque::remove` is O(N) for arbitrary indices, + // but N ≤ ~500 and eviction only fires at the cap. + if let Some(evict) = self.extracted_order.remove(idx) { + self.extracted.remove(&evict); + } + } + None => break, + } + } + self.extracted.insert(id.clone(), state); + self.extracted_order.push_back(id); + } + + /// Handler for the FetchFullText keybinding. Resolves the focused + /// article, then either toggles the view (if extraction is Ready) or + /// queues a new extraction request. Re-queues on `Failed` so the user + /// can retry. No-op on `Pending` (extraction already in flight). + pub fn toggle_or_request_fulltext(&mut self) { + let Some((feed_idx, item_idx)) = self.current_article_indices() else { + self.error = Some("No article in focus".to_string()); + return; + }; + let id = self.get_item_id(feed_idx, item_idx); + if id.is_empty() { + self.error = Some("No article in focus".to_string()); + return; + } + let url = match self + .feeds + .get(feed_idx) + .and_then(|f| f.items.get(item_idx)) + .and_then(|i| i.link.clone()) + { + Some(u) => u, + None => { + self.error = Some("Article has no URL".to_string()); + return; + } + }; + // Track whether this invocation actually queued / unqueued a + // request so we only show the off-view hint when something happened. + let mut queued_or_toggled = false; + match self.extracted.get(&id) { + Some(ExtractionState::Ready(_)) => { + self.show_extracted = !self.show_extracted; + queued_or_toggled = true; + } + Some(ExtractionState::Pending { .. }) => { + // already in flight — leave it + } + Some(ExtractionState::Failed(_)) => { + self.remove_extraction(&id); + self.show_extracted = true; + // Manual retry: jump the queue ahead of any auto-extractions + // a refresh may have enqueued. + self.request_extraction(id, url, false, true); + queued_or_toggled = true; + } + None => { + self.show_extracted = true; + // Manual request: priority over auto-path enqueues. + self.request_extraction(id, url, false, true); + queued_or_toggled = true; + } + } + // The detail view is what renders extracted state; when the action + // fires from anywhere else (macro path on Dashboard / FeedItems / + // Starred) the user gets no visible feedback. Surface a hint so the + // request doesn't feel like a no-op. + if queued_or_toggled && self.view != View::FeedItemDetail { + self.success_message = Some("Extracting full text (open article to view)".to_string()); + self.success_message_time = Some(std::time::Instant::now()); + } + } + + /// Called from the detail renderer each frame. If the focused article + /// changed since the last frame, reset the global `show_extracted` + /// toggle to `true` so the user doesn't carry a "view summary" choice + /// from one article to another. Idempotent on unchanged focus. + pub fn sync_detail_focus_anchor(&mut self) { + let current_focus = self.current_article_indices(); + if self.last_detail_focus != current_focus { + self.show_extracted = true; + self.last_detail_focus = current_focus; + } + } } /// Build an `ArticleContext` directly from a `Feed` + `FeedItem` pair — @@ -2587,4 +3008,674 @@ mod tests { assert!(parsed.seen_items.is_empty()); assert!(parsed.feeds_seeded.is_empty()); } + + fn dummy_extracted(text: &str) -> ExtractedArticle { + ExtractedArticle { + title: "T".to_string(), + plain_text: text.to_string(), + byline: None, + site_name: None, + source_url: "https://ex.com/a".to_string(), + } + } + + /// Look up the generation stamped on a `Pending` slot, so tests can + /// echo it back through `record_extraction_result` the same way a + /// real worker would. + fn pending_generation(app: &App, id: &str) -> u64 { + match app.extracted.get(id) { + Some(ExtractionState::Pending { generation, .. }) => *generation, + other => panic!("expected Pending slot for {}, got {:?}", id, other), + } + } + + #[test] + fn test_request_extraction_creates_pending_and_enqueues() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + assert!(matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { .. }) + )); + assert_eq!(app.pending_extraction_requests.len(), 1); + } + + #[test] + fn test_request_extraction_stores_safe_redirects_flag() { + let mut app = App::new(); + app.request_extraction( + "id_auto".to_string(), + "https://ex.com/a".to_string(), + true, + false, + ); + app.request_extraction( + "id_manual".to_string(), + "https://ex.com/b".to_string(), + false, + false, + ); + let q: Vec<_> = app.pending_extraction_requests.iter().collect(); + let auto = q.iter().find(|r| r.id == "id_auto").unwrap(); + let manual = q.iter().find(|r| r.id == "id_manual").unwrap(); + assert!( + auto.safe_redirects, + "auto-path request must carry safe_redirects=true" + ); + assert!( + !manual.safe_redirects, + "manual-path request must carry safe_redirects=false" + ); + } + + #[test] + fn test_request_extraction_priority_front_jumps_queue() { + // Auto requests go to the back; a manual (`priority_front=true`) + // request must land at the head so an explicit Shift+F isn't + // starved behind a refresh's auto-extraction batch. + let mut app = App::new(); + for i in 0..3 { + app.request_extraction( + format!("auto{}", i), + "https://ex.com/a".to_string(), + true, + false, + ); + } + app.request_extraction( + "manual".to_string(), + "https://ex.com/m".to_string(), + false, + true, + ); + let order: Vec<&str> = app + .pending_extraction_requests + .iter() + .map(|r| r.id.as_str()) + .collect(); + assert_eq!( + order, + vec!["manual", "auto0", "auto1", "auto2"], + "manual request must jump to the front" + ); + } + + #[test] + fn test_request_extraction_idempotent_for_already_pending() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + // No double-queue: second call must be a no-op because state is Pending. + assert_eq!(app.pending_extraction_requests.len(), 1); + } + + #[test] + fn test_record_extraction_result_transitions_to_ready() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "id1"); + app.record_extraction_result("id1".to_string(), gen, Ok(dummy_extracted("body"))); + match app.extracted.get("id1") { + Some(ExtractionState::Ready(a)) => assert_eq!(a.plain_text, "body"), + other => panic!("expected Ready, got {:?}", other), + } + } + + #[test] + fn test_record_extraction_result_transitions_to_failed() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "id1"); + app.record_extraction_result("id1".to_string(), gen, Err(anyhow::anyhow!("boom"))); + match app.extracted.get("id1") { + Some(ExtractionState::Failed(msg)) => assert!(msg.contains("boom")), + other => panic!("expected Failed, got {:?}", other), + } + } + + #[test] + fn test_lru_evicts_oldest_at_cap() { + let mut app = App::new(); + // Insert one more than the cap; the first id must be evicted. Real + // callers always Pending → Ready, and `record_extraction_result` + // refuses results for slots without a Pending predecessor, so do the + // same here. + for i in 0..EXTRACTED_CACHE_CAP + 1 { + let id = format!("id{}", i); + app.request_extraction(id.clone(), "https://ex.com/a".to_string(), false, false); + let gen = pending_generation(&app, &id); + app.record_extraction_result(id, gen, Ok(dummy_extracted("x"))); + } + assert!(!app.extracted.contains_key("id0"), "oldest must be evicted"); + assert_eq!(app.extracted.len(), EXTRACTED_CACHE_CAP); + } + + #[test] + fn test_remove_current_feed_prunes_extracted_cache() { + let mut app = make_test_app(); + // Seed extraction state for items in the first feed via the real + // request → result transition. + let id_old = app.get_item_id(0, 0); + let id_new = app.get_item_id(0, 1); + app.request_extraction(id_old.clone(), "https://ex.com/a".to_string(), false, false); + let gen_old = pending_generation(&app, &id_old); + app.record_extraction_result(id_old.clone(), gen_old, Ok(dummy_extracted("a"))); + app.request_extraction(id_new.clone(), "https://ex.com/b".to_string(), false, false); + let gen_new = pending_generation(&app, &id_new); + app.record_extraction_result(id_new.clone(), gen_new, Ok(dummy_extracted("b"))); + assert_eq!(app.extracted.len(), 2); + + // Remove the first feed. + app.selected_feed = Some(0); + app.remove_current_feed().ok(); + + assert!( + !app.extracted.contains_key(&id_old) && !app.extracted.contains_key(&id_new), + "extracted entries for removed feed must be pruned" + ); + assert!( + app.extracted_order.is_empty(), + "LRU order must be pruned in sync with the map" + ); + } + + #[test] + fn test_record_extraction_result_drops_late_result_for_evicted_id() { + // Simulates the race where a worker thread completes after its slot + // was evicted (or its feed was removed). We must not resurrect the + // slot — otherwise dead state pins memory and confuses the UI. + let mut app = App::new(); + // Any generation — slot doesn't exist, so result is dropped before + // generation is even consulted. + app.record_extraction_result("ghost".to_string(), 0, Ok(dummy_extracted("body"))); + assert!( + !app.extracted.contains_key("ghost"), + "result for an id with no Pending slot must be dropped" + ); + } + + #[test] + fn test_record_extraction_result_drops_late_result_for_ready_slot() { + // If the slot is already `Ready` (e.g. user manually retried and the + // retry landed first), a second late worker must not clobber it. + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "id1"); + app.record_extraction_result("id1".to_string(), gen, Ok(dummy_extracted("first"))); + // Late second worker — slot is Ready, so this must be a no-op. + // Whatever generation it claims is irrelevant; the state check fails. + app.record_extraction_result("id1".to_string(), gen, Ok(dummy_extracted("late"))); + match app.extracted.get("id1") { + Some(ExtractionState::Ready(a)) => { + assert_eq!(a.plain_text, "first", "late result must not clobber Ready") + } + other => panic!("expected Ready, got {:?}", other), + } + } + + #[test] + fn test_record_extraction_result_drops_stale_generation_on_re_requested_slot() { + // The TOCTOU race the generation tag closes: a Pending slot is + // evicted by LRU, then re-requested for the same id. The OLD + // worker (still running) eventually delivers its result. Without + // the generation check, that stale result would land on the new + // Pending slot — silently satisfying a "manual retry" with a + // stale fetch. + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let stale_gen = pending_generation(&app, "id1"); + // Evict + re-request manually (faster than filling 500 entries). + app.remove_extraction("id1"); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let fresh_gen = pending_generation(&app, "id1"); + assert_ne!( + stale_gen, fresh_gen, + "re-requested slot must carry a fresh generation" + ); + // Old worker lands a result tagged with the stale generation. + app.record_extraction_result("id1".to_string(), stale_gen, Ok(dummy_extracted("stale"))); + // Slot must still be Pending — the stale result was discarded. + assert!( + matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { generation: g, .. }) if *g == fresh_gen + ), + "stale-generation result must not satisfy the re-requested slot" + ); + } + + #[test] + fn test_prune_stale_pending_extractions_flips_old_pending_to_failed() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + // Forcibly age the Pending slot's started_at past the watchdog. + // Mark it `spawned=true` so this test exercises the genuine + // stuck-worker case (a worker that bumped inflight and never + // released it) — that's what the watchdog is meant to recover. + let id = "id1".to_string(); + let gen = pending_generation(&app, &id); + let aged = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .expect("Instant::now() - 1h must be representable on a freshly-booted machine"); + app.extracted.insert( + id.clone(), + ExtractionState::Pending { + generation: gen, + started_at: aged, + spawned: true, + }, + ); + // 30 s × 3 = 90 s watchdog; aged 3600 s exceeds it. + app.prune_stale_pending_extractions(std::time::Duration::from_secs(30)); + match app.extracted.get(&id) { + Some(ExtractionState::Failed(msg)) => { + assert!( + msg.contains("timed out"), + "expected timeout reason: {}", + msg + ) + } + other => panic!("expected Failed after watchdog, got {:?}", other), + } + } + + #[test] + fn test_prune_returns_zero_for_aged_but_unspawned_pending() { + // Regression test for the inflight-counter drift: a request that + // aged out while still queued (rate-limited behind a long backlog) + // has `spawned=false`, so the watchdog must NOT count it toward the + // inflight-release total. Counting it would silently widen the + // effective worker cap because no permit was ever claimed. + let mut app = App::new(); + app.request_extraction( + "stranded".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "stranded"); + // Age the slot but leave `spawned=false` — the request never made + // it through the spawn loop. + let aged = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .expect("Instant::now() - 1h must be representable"); + app.extracted.insert( + "stranded".to_string(), + ExtractionState::Pending { + generation: gen, + started_at: aged, + spawned: false, + }, + ); + let pruned = app.prune_stale_pending_extractions(std::time::Duration::from_secs(30)); + assert_eq!( + pruned, 0, + "watchdog must return zero for queue-stranded prunes — no inflight permit was ever claimed" + ); + // The slot still flips to Failed so the user can retry; only the + // inflight-release count is gated on `spawned`. + assert!( + matches!( + app.extracted.get("stranded"), + Some(ExtractionState::Failed(_)) + ), + "queue-stranded slot must still be flipped to Failed so a manual retry works" + ); + } + + #[test] + fn test_prune_counts_only_spawned_slots_when_mixed() { + // Mixed batch: one spawned (real stuck worker) and one queue-stranded. + // Only the spawned one should be counted toward inflight release. + let mut app = App::new(); + app.request_extraction( + "spawned".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + app.request_extraction( + "queued".to_string(), + "https://ex.com/b".to_string(), + false, + false, + ); + let aged = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .expect("Instant::now() - 1h must be representable"); + let g1 = pending_generation(&app, "spawned"); + let g2 = pending_generation(&app, "queued"); + app.extracted.insert( + "spawned".to_string(), + ExtractionState::Pending { + generation: g1, + started_at: aged, + spawned: true, + }, + ); + app.extracted.insert( + "queued".to_string(), + ExtractionState::Pending { + generation: g2, + started_at: aged, + spawned: false, + }, + ); + let pruned = app.prune_stale_pending_extractions(std::time::Duration::from_secs(30)); + assert_eq!(pruned, 1, "only the spawned slot must be counted"); + } + + #[test] + fn test_mark_extraction_spawned_flips_flag_on_matching_generation() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "id1"); + // Before: spawned=false from request_extraction. + assert!(matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { spawned: false, .. }) + )); + app.mark_extraction_spawned("id1", gen); + assert!(matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { spawned: true, .. }) + )); + } + + #[test] + fn test_mark_extraction_spawned_is_noop_on_generation_mismatch() { + // If the slot was evicted and re-requested between spawn() and + // mark_extraction_spawned, the old generation must not flip the + // flag on the fresh slot. The stale worker's record_extraction_result + // would also be dropped by the generation check, so leaving + // spawned=false on the fresh slot is the right semantic. + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let stale_gen = pending_generation(&app, "id1"); + app.remove_extraction("id1"); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let fresh_gen = pending_generation(&app, "id1"); + assert_ne!(stale_gen, fresh_gen); + app.mark_extraction_spawned("id1", stale_gen); + assert!(matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { spawned: false, .. }) + )); + } + + #[test] + fn test_prune_stale_pending_extractions_leaves_fresh_pending_alone() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + // Freshly Pending — well within the watchdog window. + app.prune_stale_pending_extractions(std::time::Duration::from_secs(30)); + assert!(matches!( + app.extracted.get("id1"), + Some(ExtractionState::Pending { .. }) + )); + } + + #[test] + fn test_lru_hard_cap_when_oldest_is_pending() { + // Stress the soft-cap edge case: queue CAP+1 requests as Pending + // (no result returned). The cache size must stay at the cap, not + // grow with the number of in-flight requests. + let mut app = App::new(); + for i in 0..EXTRACTED_CACHE_CAP + 5 { + let id = format!("id{}", i); + app.request_extraction(id, "https://ex.com/a".to_string(), false, false); + } + assert_eq!( + app.extracted.len(), + EXTRACTED_CACHE_CAP, + "Pending entries must not let the cache exceed the hard cap" + ); + assert_eq!(app.extracted_order.len(), EXTRACTED_CACHE_CAP); + } + + #[test] + fn test_remove_extraction_helper_prunes_both_map_and_order() { + let mut app = App::new(); + app.request_extraction( + "id1".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "id1"); + app.record_extraction_result("id1".to_string(), gen, Ok(dummy_extracted("body"))); + assert_eq!(app.extracted.len(), 1); + assert_eq!(app.extracted_order.len(), 1); + app.remove_extraction("id1"); + assert!(app.extracted.is_empty()); + assert!(app.extracted_order.is_empty()); + } + + #[test] + fn test_sync_detail_focus_anchor_resets_show_extracted_on_focus_change() { + // Regression test for the cross-article show_extracted bug: the + // global toggle must reset to `true` whenever the detail view's + // focused article changes, so a "view summary" choice on article A + // doesn't silently carry over to article B. + let mut app = make_test_app(); + app.view = View::FeedItemDetail; + app.selected_feed = Some(0); + app.selected_item = Some(0); + // First sync — establishes the anchor at (0, 0). + app.sync_detail_focus_anchor(); + assert!(app.show_extracted); + assert_eq!(app.last_detail_focus, Some((0, 0))); + + // Simulate the user pressing Shift+F to toggle off on article (0, 0). + app.show_extracted = false; + // Re-sync on the same article — must NOT clobber the user's toggle. + app.sync_detail_focus_anchor(); + assert!( + !app.show_extracted, + "anchor unchanged → user's toggle preserved" + ); + + // User navigates to article (0, 1). Sync must reset the toggle. + app.selected_item = Some(1); + app.sync_detail_focus_anchor(); + assert!( + app.show_extracted, + "focus changed → show_extracted must reset to true" + ); + assert_eq!(app.last_detail_focus, Some((0, 1))); + } + + #[test] + fn test_sync_detail_focus_anchor_is_noop_when_focus_unchanged() { + // Per-frame call must be idempotent — otherwise a toggled-off state + // would be wiped on the very next frame and the toggle would feel + // unresponsive. + let mut app = make_test_app(); + app.view = View::FeedItemDetail; + app.selected_feed = Some(0); + app.selected_item = Some(0); + app.sync_detail_focus_anchor(); + app.show_extracted = false; + for _ in 0..10 { + app.sync_detail_focus_anchor(); + } + assert!( + !app.show_extracted, + "repeated syncs must not wipe the toggle" + ); + } + + /// Regression lock for the LRU-vs-watchdog leak: evicting a Pending slot + /// whose `spawned` flag is true would hide it from + /// `prune_stale_pending_extractions` (which only iterates `extracted`), + /// so a silently-dead worker's inflight permit would leak for the rest + /// of the session. `insert_extraction` must skip such slots during + /// eviction and evict the next-oldest evictable entry. + #[test] + fn test_lru_eviction_skips_spawned_pending_slots() { + let mut app = App::new(); + // Seed one spawned-Pending slot at the head of the LRU. Real callers + // would have inserted via `request_extraction` then flipped `spawned` + // via `mark_extraction_spawned`; we do the same here. + app.request_extraction( + "spawned_head".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + let gen = pending_generation(&app, "spawned_head"); + app.mark_extraction_spawned("spawned_head", gen); + // Fill the rest of the cap with Ready entries (insertion-order LRU + // puts them all behind the spawned head). + for i in 0..EXTRACTED_CACHE_CAP - 1 { + let id = format!("ready{}", i); + app.request_extraction(id.clone(), "https://ex.com/x".to_string(), false, false); + let g = pending_generation(&app, &id); + app.record_extraction_result(id, g, Ok(dummy_extracted("body"))); + } + assert_eq!(app.extracted.len(), EXTRACTED_CACHE_CAP); + + // Insert one more entry — eviction must skip the spawned-Pending + // head and instead evict the next-oldest (ready0). + app.request_extraction( + "newcomer".to_string(), + "https://ex.com/n".to_string(), + false, + false, + ); + let g = pending_generation(&app, "newcomer"); + app.record_extraction_result("newcomer".to_string(), g, Ok(dummy_extracted("body"))); + + assert!( + app.extracted.contains_key("spawned_head"), + "spawned-Pending slot must NOT be evicted — would hide it from the watchdog" + ); + assert!( + !app.extracted.contains_key("ready0"), + "the next-oldest evictable entry should have been evicted instead" + ); + // Size still bounded — cap +/- the one we skipped (no extra growth + // here because we only protected one slot and evicted one entry). + assert_eq!(app.extracted.len(), EXTRACTED_CACHE_CAP); + } + + /// Watchdog must NOT promote a stale Pending → Failed transition to the + /// most-recently-used end of the LRU deque. A timed-out entry should be + /// strictly less useful than newer Ready entries and evicted ahead of + /// them — promoting it would invert that order. + #[test] + fn test_watchdog_failure_does_not_promote_lru_position() { + let mut app = App::new(); + app.request_extraction( + "first".to_string(), + "https://ex.com/a".to_string(), + false, + false, + ); + // Insert another entry so we can observe LRU order before/after. + app.request_extraction( + "second".to_string(), + "https://ex.com/b".to_string(), + false, + false, + ); + // Order should be [first, second] in the deque. + let order_before: Vec = app.extracted_order.iter().cloned().collect(); + assert_eq!( + order_before, + vec!["first".to_string(), "second".to_string()] + ); + + // Age `first` past the watchdog window and run the prune. + let gen = pending_generation(&app, "first"); + let aged = Instant::now() + .checked_sub(std::time::Duration::from_secs(3600)) + .expect("Instant::now() - 1h must be representable"); + app.extracted.insert( + "first".to_string(), + ExtractionState::Pending { + generation: gen, + started_at: aged, + spawned: true, + }, + ); + app.prune_stale_pending_extractions(std::time::Duration::from_secs(30)); + + // State flipped to Failed, but LRU position preserved at the head. + assert!(matches!( + app.extracted.get("first"), + Some(ExtractionState::Failed(_)) + )); + let order_after: Vec = app.extracted_order.iter().cloned().collect(); + assert_eq!( + order_after, + vec!["first".to_string(), "second".to_string()], + "watchdog must NOT promote a Failed entry to MRU — would let it outlive newer Ready slots" + ); + } } diff --git a/src/config.rs b/src/config.rs index 4dd1bf5..de91143 100644 --- a/src/config.rs +++ b/src/config.rs @@ -129,6 +129,10 @@ pub struct DefaultFeed { /// Per-feed refresh interval in seconds; None = use global interval #[serde(default, skip_serializing_if = "Option::is_none")] pub refresh_interval: Option, + /// When true, auto-extract full-text for newly-seen items from this feed + /// on each refresh. Manual extraction via Shift+F always works regardless. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub fulltext: Option, } // Default value functions @@ -406,6 +410,15 @@ impl Config { # [default_feeds.headers]\n\ # Authorization = \"Bearer your_token_here\"\n\ #\n\ + # Full-text extraction (Readability) — auto-extract on refresh:\n\ + # [[default_feeds]]\n\ + # url = \"https://example.com/summary-only-feed.xml\"\n\ + # fulltext = true\n\ + #\n\ + # Press Shift+F in the article detail view to extract on-demand\n\ + # for any feed. Auth headers from this feed are NOT sent to the\n\ + # article URL — they would leak to third-party hosts.\n\ + #\n\ # ── External-command hooks ──────────────────────────────\n\ #\n\ # Macros bind a key (invoked as , default prefix is ',') to\n\ @@ -419,7 +432,7 @@ impl Config { #\n\ # Supported actions inside macros:\n\ # open-in-browser, toggle-star, toggle-read, mark-all-read,\n\ - # refresh, toggle-theme, extract-links, help\n\ + # refresh, toggle-theme, extract-links, fetch-full-text, help\n\ # Other keybinding actions are intentionally not callable from macros.\n\ #\n\ # Variables expanded per argv token:\n\ @@ -474,6 +487,34 @@ mod tests { assert_eq!(config.ui.error_display_timeout, 3000); } + #[test] + fn test_default_feed_without_fulltext_parses() { + // Existing configs that have no `fulltext` key must keep working. + let toml_str = r#" + [[default_feeds]] + url = "https://example.com/feed.xml" + "#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert_eq!(config.default_feeds.len(), 1); + assert!(config.default_feeds[0].fulltext.is_none()); + } + + #[test] + fn test_default_feed_with_fulltext_round_trips() { + let toml_str = r#" + [[default_feeds]] + url = "https://example.com/feed.xml" + fulltext = true + "#; + let config: Config = toml::from_str(toml_str).unwrap(); + assert_eq!(config.default_feeds[0].fulltext, Some(true)); + + // Round-trip: serialize the parsed config and parse the result. + let serialized = toml::to_string(&config).unwrap(); + let reparsed: Config = toml::from_str(&serialized).unwrap(); + assert_eq!(reparsed.default_feeds[0].fulltext, Some(true)); + } + #[test] fn test_default_feed_with_headers() { let toml_str = r#" diff --git a/src/config_tui.rs b/src/config_tui.rs index 92bc6ee..34700f6 100644 --- a/src/config_tui.rs +++ b/src/config_tui.rs @@ -361,6 +361,7 @@ impl ConfigEditor { category, headers: None, refresh_interval: None, + fulltext: None, }); self.dirty = true; self.adding_feed = false; diff --git a/src/events.rs b/src/events.rs index c883cbc..bd2998a 100644 --- a/src/events.rs +++ b/src/events.rs @@ -162,6 +162,7 @@ pub(crate) fn dispatch_action(app: &mut App, action: KeyAction) { Err(e) => app.error = Some(format!("Failed to mark all read: {}", e)), }, KeyAction::ExtractLinks => app.extract_links_from_current_item(), + KeyAction::FetchFullText => app.toggle_or_request_fulltext(), other => { app.error = Some(format!( "macro: action '{}' not supported in macros", @@ -841,6 +842,9 @@ pub(crate) fn handle_key_event(app: &mut App, key: crossterm::event::KeyEvent) - _ if app.key_matches(KeyAction::Help, &key) => { handle_show_help(app); } + _ if app.key_matches(KeyAction::FetchFullText, &key) => { + app.toggle_or_request_fulltext(); + } _ => {} }, View::Starred => match key.code { diff --git a/src/feed.rs b/src/feed.rs index 985a75b..f2cab5a 100644 --- a/src/feed.rs +++ b/src/feed.rs @@ -1,15 +1,34 @@ use anyhow::{Context, Result}; use chrono::{DateTime, Utc}; +use dom_smoothie::{Config as ReadabilityConfig, Readability}; use feed_rs::parser; use scraper::{Html, Selector}; use serde::{Deserialize, Serialize}; use std::collections::{HashMap, HashSet}; use std::fmt; +use std::io::Read; +use std::net::IpAddr; use std::sync::OnceLock; use std::time::Duration; use url::Url; use uuid::Uuid; +/// Cap on response body size for article extraction. Anything larger is +/// almost certainly not a typical article page and would only burn CPU in +/// the Readability DOM walker. +const FULLTEXT_MAX_BYTES: usize = 5 * 1024 * 1024; + +/// Minimum text length the extracted article body must contain to be +/// considered useful. Pages below this are treated as "page appears empty" +/// — usually JS-rendered shells, login walls, or 404 placeholders. +const FULLTEXT_MIN_LENGTH: usize = 200; + +/// Number of leading bytes of the response we sniff for `` +/// when the Content-Type header doesn't carry one. The HTML5 spec mandates +/// any in-document declaration must appear in the first 1024 bytes of the +/// document, so reading more is just wasted work. +const META_SNIFF_BYTES: usize = 1024; + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Feed { pub url: String, @@ -100,6 +119,352 @@ pub struct DiscoveredFeed { pub feed_type: FeedType, } +/// Outcome of a successful Readability extraction against an article URL. +/// Decoupled from `dom_smoothie::Article` so the upstream crate can be +/// swapped without rippling through the rest of the codebase. +#[derive(Clone, Debug)] +pub struct ExtractedArticle { + pub title: String, + pub plain_text: String, + pub byline: Option, + pub site_name: Option, + pub source_url: String, +} + +/// Validates that `url` is safe to fetch on the *auto-fulltext* path (i.e. +/// without a user click). Rejects non-http(s) schemes and hostnames that +/// resolve to private / loopback / link-local addresses, since auto-mode +/// fetches whatever the feed XML put in `` — a hostile feed could +/// otherwise probe the user's internal network. Manual `Shift+F` is the +/// user's explicit action and bypasses this check. +pub fn is_safe_auto_url(url: &str) -> bool { + let parsed = match Url::parse(url) { + Ok(u) => u, + Err(_) => return false, + }; + if !matches!(parsed.scheme(), "http" | "https") { + return false; + } + let host = match parsed.host() { + Some(h) => h, + None => return false, + }; + match host { + url::Host::Ipv4(ip) => is_global_ip(IpAddr::V4(ip)), + url::Host::Ipv6(ip) => is_global_ip(IpAddr::V6(ip)), + url::Host::Domain(name) => { + // Reject obvious local-only names; a hostile feed cannot use + // these to probe RFC1918 space, but we still want to avoid + // hitting "localhost"-style endpoints by accident. + let n = name.to_ascii_lowercase(); + if n == "localhost" || n.ends_with(".localhost") || n.ends_with(".local") { + return false; + } + // DNS resolution itself happens later in reqwest; we don't + // resolve here because that would block. The intent of the + // check is "no obvious internal target encoded directly in + // the URL" — DNS rebinding is out of scope for a feed reader. + true + } + } +} + +fn is_global_ip(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => { + !(v4.is_loopback() + || v4.is_private() + || v4.is_link_local() + || v4.is_broadcast() + || v4.is_documentation() + || v4.is_unspecified() + // Multicast 224.0.0.0/4 — IPv6 multicast (ff00::/8) is + // already rejected below; this closes the v4/v6 asymmetry. + || v4.is_multicast() + || v4.octets()[0] == 0 + // CGNAT 100.64.0.0/10 + || (v4.octets()[0] == 100 && (v4.octets()[1] & 0xC0) == 64) + // IETF protocol-assignments 192.0.0.0/24 + || (v4.octets()[0] == 192 + && v4.octets()[1] == 0 + && v4.octets()[2] == 0)) + } + IpAddr::V6(v6) => { + let seg0 = v6.segments()[0]; + let seg1 = v6.segments()[1]; + !(v6.is_loopback() + || v6.is_unspecified() + // unique-local fc00::/7 + || (seg0 & 0xFE00) == 0xFC00 + // link-local fe80::/10 + || (seg0 & 0xFFC0) == 0xFE80 + // deprecated site-local fec0::/10 + || (seg0 & 0xFFC0) == 0xFEC0 + // multicast ff00::/8 + || (seg0 & 0xFF00) == 0xFF00 + // 6to4 2002::/16 — routes to embedded IPv4 via 6to4 relays, + // so an address like 2002:a9fe:a9fe:: would reach 169.254.169.254 + || seg0 == 0x2002 + // NAT64 well-known prefix 64:ff9b::/96 + || (seg0 == 0x0064 && seg1 == 0xff9b) + // IPv4-mapped — defer to v4 check via the embedded address + || v6.to_ipv4_mapped().map(|v4| !is_global_ip(IpAddr::V4(v4))).unwrap_or(false)) + } + } +} + +/// Fetch `url` with `client` and run Mozilla-Readability extraction over the +/// returned HTML. Feed auth headers are intentionally NOT propagated: the +/// article URL is on a different host and may be third-party, so leaking +/// per-feed `Authorization` headers would be a credential leak. +/// +/// Rejects non-`text/html` responses, oversized bodies (`FULLTEXT_MAX_BYTES`), +/// and pages whose extracted text falls below `FULLTEXT_MIN_LENGTH` (treated +/// as JS-rendered or empty). Body is read with a size-bounded reader so peak +/// allocation cannot exceed the cap, and the response charset is honored so +/// non-UTF8 pages decode correctly. +pub fn extract_article( + url: &str, + client: &reqwest::blocking::Client, + user_agent: Option<&str>, +) -> Result { + // Scheme allowlist — reqwest's blocking client refuses non-http(s) by + // default, but this gives a clearer error and makes the policy explicit. + let parsed = Url::parse(url).context("Article URL is not a valid URL")?; + if !matches!(parsed.scheme(), "http" | "https") { + return Err(anyhow::anyhow!( + "Article URL scheme '{}' is not allowed (only http/https)", + parsed.scheme() + )); + } + + let default_user_agent = + "Mozilla/5.0 (compatible; Feedr/1.0; +https://github.com/bahdotsh/feedr)"; + let ua = user_agent.unwrap_or(default_user_agent); + + let response = client + .get(url) + .header("User-Agent", ua) + .header("Accept", "text/html, application/xhtml+xml, */*;q=0.5") + .header("Accept-Language", "en-US,en;q=0.9") + .header("Accept-Encoding", "gzip, deflate") + .send() + .context("Failed to fetch article")?; + + let status = response.status(); + if !status.is_success() { + return Err(anyhow::anyhow!( + "HTTP error {} fetching article from {}", + status, + url + )); + } + + let content_type_raw = response + .headers() + .get("content-type") + .and_then(|ct| ct.to_str().ok()) + .unwrap_or("") + .to_string(); + let content_type = content_type_raw.to_lowercase(); + if !content_type.is_empty() + && !content_type.contains("text/html") + && !content_type.contains("application/xhtml") + { + return Err(anyhow::anyhow!( + "Article URL did not return HTML (content-type: {})", + content_type + )); + } + + // Reject upfront if Content-Length already exceeds the cap — saves us + // the full transfer for obviously oversized responses. + if let Some(declared_len) = response + .headers() + .get("content-length") + .and_then(|cl| cl.to_str().ok()) + .and_then(|cl| cl.parse::().ok()) + { + if declared_len > FULLTEXT_MAX_BYTES { + return Err(anyhow::anyhow!( + "Article body too large ({} bytes declared, cap {} bytes)", + declared_len, + FULLTEXT_MAX_BYTES + )); + } + } + + let final_url = response.url().to_string(); + + // Preallocate at the hard cap so `read_to_end`'s doubling growth can + // never push capacity past it. Without this, starting from a smaller + // hint (e.g. 64 KiB) grows the backing buffer to ~8 MiB for a 5 MiB + // body — making the body-buffer ceiling ambiguous. With the + // preallocation, the body-buffer ceiling is exactly FULLTEXT_MAX_BYTES; + // the downstream DOM allocation inside dom_smoothie is separate + // working-set and not bounded here. Modern allocators don't commit + // physical pages until written, so a 5 MiB virtual reservation costs + // essentially zero physical memory for the common short-page case. + let mut bytes = Vec::with_capacity(FULLTEXT_MAX_BYTES + 1); + let mut reader = response.take((FULLTEXT_MAX_BYTES as u64) + 1); + reader + .read_to_end(&mut bytes) + .context("Failed to read article body")?; + if bytes.len() > FULLTEXT_MAX_BYTES { + return Err(anyhow::anyhow!( + "Article body too large (exceeded cap {} bytes)", + FULLTEXT_MAX_BYTES + )); + } + + let html = decode_html_bytes(&bytes, &content_type_raw); + extract_from_html(&html, &final_url) +} + +/// Decode `bytes` into a Rust `String` using the encoding declared by the +/// `Content-Type` header, falling back to `` sniffing in the +/// first ~1 KB of the document, and finally to UTF-8. Without this, any +/// non-UTF8 page (Windows-1252, ISO-8859-1, Shift_JIS, GBK, …) produces +/// U+FFFD-laced mojibake that Readability would then misclassify as too +/// short. +pub(crate) fn decode_html_bytes(bytes: &[u8], content_type: &str) -> String { + let mut encoding: Option<&'static encoding_rs::Encoding> = None; + + // 1) Honor charset= in the Content-Type header. + if let Some(label) = charset_from_content_type(content_type) { + encoding = encoding_rs::Encoding::for_label(label.as_bytes()); + } + + // 2) Otherwise sniff the leading bytes for an in-document . + if encoding.is_none() { + let sniff_len = bytes.len().min(META_SNIFF_BYTES); + if let Some(label) = sniff_meta_charset(&bytes[..sniff_len]) { + encoding = encoding_rs::Encoding::for_label(label.as_bytes()); + } + } + + // 3) Default to UTF-8 — same fate `from_utf8_lossy` would have given us, + // but routed through encoding_rs so behavior is uniform. + let encoding = encoding.unwrap_or(encoding_rs::UTF_8); + let (cow, _enc, _had_errors) = encoding.decode(bytes); + cow.into_owned() +} + +fn charset_from_content_type(content_type: &str) -> Option { + for part in content_type.split(';') { + let lower = part.trim().to_ascii_lowercase(); + if let Some(rest) = lower.strip_prefix("charset=") { + let v = rest.trim().trim_matches('"').trim_matches('\''); + if !v.is_empty() { + return Some(v.to_string()); + } + } + } + None +} + +/// Sniff `` or `` from the leading bytes of an HTML document. Only +/// matches `charset=` that appears INSIDE a `` tag — a loose +/// substring match would be fooled by `data-charset="x"` attributes or +/// string literals inside `` in the head must not + // fool the sniffer. + let html = b"\ + \ + \ + x"; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "utf-8", "script literal must be ignored"); + } + + #[test] + fn test_sniff_meta_charset_ignores_metadata_tag() { + // `` must not be matched as ``. + let html = b"\ + charset=bogus\ + \ + "; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "utf-8"); + } + + #[test] + fn test_sniff_meta_charset_handles_http_equiv_meta() { + // The other legal form: + let html = b"\ + \ + "; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "iso-8859-1"); + } + + #[test] + fn test_sniff_meta_charset_skips_attribute_value_named_charset() { + // Pathological: an attribute whose VALUE is literally "charset" + // would have shadowed the real declaration with the old + // first-occurrence-wins logic. The tightened sniffer requires + // `charset` to be preceded by a word boundary AND followed by `=`, + // so the value match is rejected. + let html = b"\ + \ + "; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "utf-8"); + } + + #[test] + fn test_sniff_meta_charset_skips_data_attr_with_charset_value() { + // `data-foo="charset=bogus"` would have been a false positive under + // the old logic (substring + `=` after charset). The boundary check + // — `charset` must be preceded by whitespace/`/`/start-of-tag — + // rejects it because the preceding char is `"`. + let html = b"\ + \ + "; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "utf-8"); + } + + #[test] + fn test_sniff_meta_charset_skips_longer_attribute_name_prefixed_with_charset() { + // Word-boundary check: a hypothetical attribute like + // `data-charset="bogus"` shouldn't trigger — the char before + // `charset` is `-`, not a boundary. + let html = b"\ + \ + "; + let label = sniff_meta_charset(html).expect("should find charset"); + assert_eq!(label, "utf-8"); + } + + #[test] + fn test_is_safe_auto_url_accepts_public_https() { + assert!(is_safe_auto_url("https://example.com/article")); + assert!(is_safe_auto_url("http://example.com/article")); + } + + #[test] + fn test_is_safe_auto_url_rejects_non_http_scheme() { + assert!(!is_safe_auto_url("file:///etc/passwd")); + assert!(!is_safe_auto_url("ftp://example.com/x")); + assert!(!is_safe_auto_url("javascript:alert(1)")); + assert!(!is_safe_auto_url("data:text/html,