Skip to content

Commit c86cebe

Browse files
ralyodioclaude
andauthored
news: refuse a catalogue-sized import however it is reached (#402)
`searchOnly` guarded the list *name*, so `add smallweb` was refused and `add https://kagi.com/smallweb/opml` was not — it wrote 32,969 outlines into news.opml, and every `/rss` afterwards tried to fetch all of them. That is the crash the flag existed to prevent, through the one door left open. A URL is not a smaller promise than a name, so the limit is on the number of feeds an `add` subscribes to, whatever the source. 2,000 sits well above every published list (the largest is 611) and well below a catalogue. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 73f2eaa commit c86cebe

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

‎src/news.mjs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,15 @@ const FEED_MAX_BYTES = 2 * 1024 * 1024;
124124
*/
125125
const PER_FEED_ITEMS = 200;
126126

127+
/**
128+
* The most feeds one `add` may subscribe to at once.
129+
*
130+
* Whatever the source — a named list, a URL, a local file. The largest list
131+
* published by name holds 611 feeds, so this refuses only the thing that is a
132+
* catalogue rather than a reading list.
133+
*/
134+
const MAX_IMPORT_FEEDS = 2000;
135+
127136
/** How many feeds are in flight at once. Politeness, not throughput. */
128137
const CONCURRENCY = 6;
129138

@@ -1577,6 +1586,17 @@ async function addCommand(request, { out, fail, fetchImpl, env }) {
15771586
// Tagged with the list it came from, so `rm <list>` can take it back out.
15781587
const incoming = bundle ? tagWithList(parsedList, bundle.name) : parsedList;
15791588
if (!incoming.length) { fail(danger(`✗ that ${bundle?.format === "text" ? "list" : "OPML file"} lists no feeds`)); return 1; }
1589+
// A catalogue is not a subscription. `searchOnly` guarded the *name* but
1590+
// never the URL, so `add https://kagi.com/smallweb/opml` wrote 32,969
1591+
// outlines into news.opml and every `/rss` afterwards tried to fetch all of
1592+
// them — the crash this was supposed to prevent, through the one door left
1593+
// open. The limit sits well above every published list (the largest is 611)
1594+
// and well below a catalogue.
1595+
if (incoming.length > MAX_IMPORT_FEEDS) {
1596+
fail(danger(`✗ that list holds ${incoming.length.toLocaleString()} feeds — more than ${MAX_IMPORT_FEEDS.toLocaleString()}, which is more than a reader can fetch`));
1597+
fail(` ${ash("it is a catalogue, not a subscription — search it instead:")} ${bone("/rss search <keyword>")}`);
1598+
return 1;
1599+
}
15801600
let feeds = existing;
15811601
const added = [];
15821602
let skipped = 0;

‎test/news.test.mjs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,6 +1021,25 @@ test("smallweb subscribes as one feed, without downloading it first", async () =
10211021
assert.deepEqual(loadFeeds(env), []);
10221022
});
10231023

1024+
test("a catalogue-sized OPML is refused however it is reached", async () => {
1025+
// The door that was left open: search-only guarded the list name, so
1026+
// `add smallweb` was refused and `add https://kagi.com/smallweb/opml` wrote
1027+
// 32,969 outlines into news.opml — after which every /rss tried to fetch all
1028+
// of them. A URL is not a smaller promise than a name.
1029+
const { env } = sandbox();
1030+
const outlines = Array.from({ length: 2500 }, (_, i) =>
1031+
`<outline text="blog ${i}" xmlUrl="https://b${i}.example/rss"/>`).join("");
1032+
const io = sink();
1033+
const code = await newsCommand(["add", "https://huge.example/list.opml"], {
1034+
...io, env,
1035+
fetchImpl: fakeFetch({ "https://huge.example/list.opml": `<opml><body>${outlines}</body></opml>` }),
1036+
});
1037+
assert.equal(code, 1);
1038+
assert.match(io.errorText(), /2,500 feeds/);
1039+
assert.match(io.errorText(), /catalogue, not a subscription/);
1040+
assert.deepEqual(loadFeeds(env), []);
1041+
});
1042+
10241043
test("a one-feed list is searchable without a catalogue fetch", async () => {
10251044
const list = resolveBundle("smallweb");
10261045
assert.equal(list.format, "feed");

0 commit comments

Comments
 (0)