Skip to content

Commit 9eddf78

Browse files
ralyodioclaude
andauthored
A dot is not a hostname (#148)
`normalizeUrl` guarded the host with `hostname.includes('.')`, and `new URL()` is far more permissive about hosts than DNS is: quotes, parens, `=` and `&` are not forbidden host code points. So `version="1.0"`, `text="gg.deals`, `zombies.)` and `z.` all parsed, all contained a dot, and all became feeds. That is how ~3,700 rows like `https://version="1.0"/` and `https://zombies.)/` reached the directory. The bulk-upload scanner splits pasted text on whitespace and offers every token as a URL, so pasting raw OPML instead of a URL list fed the markup itself in -- XML attributes, and sentences whose last word ended in a full stop. Each one is then re-crawled forever on a cadence and fails `blocked-host`; they were most of a 960/hour spike in that error. The check goes in `normalizeUrl` rather than in the scanner because it is the one gate every entry path shares -- web submit, OPML import, the queue drain and discovery -- and because the scanner runs in the browser, where importing `@rssamplifier/feed` fails the build on `node:dns`. It rejects what cannot be a hostname, not what is merely unusual: IDN is already punycode by then, a port and a dotted quad still pass, and private ranges go on being refused later by `isPublicHost`. The second test is the half that would break the directory if this were too strict, so it is spelled out. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a83f1e0 commit 9eddf78

2 files changed

Lines changed: 94 additions & 1 deletion

File tree

‎packages/feed/src/discover.js‎

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,69 @@ export function normalizeUrl(input) {
5252
try {
5353
const u = new URL(raw);
5454
if (u.protocol !== 'http:' && u.protocol !== 'https:') return null;
55-
if (!u.hostname.includes('.')) return null;
55+
if (!plausibleHost(u.hostname)) return null;
5656
u.hash = '';
5757
return u.toString();
5858
} catch {
5959
return null;
6060
}
6161
}
6262

63+
/**
64+
* Is this hostname something a publisher could actually be at?
65+
*
66+
* `new URL()` is far more permissive about hosts than DNS is. Quotes, parens,
67+
* `=` and `&` are not forbidden host code points, so `new URL('https://x')`
68+
* happily parses `version="1.0"`, `zombies.)` and `z.` into a hostname — and
69+
* the check this replaced, `hostname.includes('.')`, waved all three through.
70+
*
71+
* That is not a theoretical gap. The bulk-upload scanner splits pasted text on
72+
* whitespace and offers every token as a URL, so pasting raw OPML instead of a
73+
* URL list turned the markup itself into feeds: `https://version="1.0"/`,
74+
* `https://text="gg.deals/`, and a sentence ending in "zombies." into
75+
* `https://zombies.)/`. Roughly 3,700 such rows reached the directory, and
76+
* every one of them is crawled forever on a cadence, failing `blocked-host`.
77+
*
78+
* Validating here rather than in the scanner is deliberate: `normalizeUrl` is
79+
* the single gate every entry path goes through -- web submit, OPML import,
80+
* the queue drain and discovery -- so one check covers all of them, and the
81+
* scanner cannot import this module anyway (it runs in the browser, and
82+
* pulling `@rssamplifier/feed` into client code fails the build on `node:dns`).
83+
*
84+
* Kept deliberately loose about what a real domain looks like: this rejects
85+
* things that cannot be hostnames, not things that are merely unusual. IDN is
86+
* already punycode by the time it arrives, and a bare IPv4 literal is allowed
87+
* through so that a feed genuinely served from one is not newly rejected --
88+
* private ranges are refused later, by `isPublicHost`.
89+
*
90+
* @param {string} hostname as parsed by `new URL`, so lowercased and punycoded
91+
* @returns {boolean}
92+
*/
93+
function plausibleHost(hostname) {
94+
const host = String(hostname ?? '');
95+
if (!host || host.length > 253) return false;
96+
97+
// A dotted-quad is a legitimate, if unusual, place for a feed to live.
98+
if (/^\d{1,3}(\.\d{1,3}){3}$/.test(host)) {
99+
return host.split('.').every((n) => Number(n) <= 255);
100+
}
101+
102+
// Anything outside the DNS alphabet is markup or prose, not a host.
103+
if (!/^[a-z0-9.-]+$/.test(host)) return false;
104+
105+
const labels = host.split('.');
106+
// Two labels at least, none empty -- which is what rules out `z.` and `you.`,
107+
// whose trailing dot leaves an empty final label.
108+
if (labels.length < 2) return false;
109+
if (labels.some((l) => l === '' || l.length > 63 || l.startsWith('-') || l.endsWith('-'))) {
110+
return false;
111+
}
112+
113+
// A public suffix is letters. This is the part that rejects `1.0` while
114+
// leaving `example.co.uk` and `xn--bcher-kva.de` alone.
115+
return /^[a-z]{2,}$/.test(labels[labels.length - 1]);
116+
}
117+
63118
/**
64119
* Extract feed URLs advertised by a page's <link rel="alternate"> tags.
65120
*

‎packages/feed/test/security.test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,44 @@ test('normalizeUrl adds a scheme but rejects dangerous ones', () => {
4747
assert.equal(normalizeUrl(''), null);
4848
});
4949

50+
test('normalizeUrl rejects markup and prose that a dot alone would let through', () => {
51+
// The bulk-upload scanner splits pasted text on whitespace and offers every
52+
// token as a URL, so pasting raw OPML instead of a URL list turned the markup
53+
// itself into feeds. The old check was `hostname.includes('.')`, and every
54+
// one of these contains a dot: ~3,700 reached production, where they are
55+
// re-crawled forever and fail `blocked-host`.
56+
//
57+
// These are the real stored feed_urls, with the `https://` the old code added
58+
// and the trailing slash `URL.toString()` gave them.
59+
for (const token of [
60+
'version="1.0"', // an XML declaration
61+
'text="gg.deals', // an OPML attribute, torn at the quote
62+
'title="arnika.org',
63+
'zombies.)', // a sentence ending in a word and a bracket
64+
'yourself."',
65+
'z.', // a single letter and a full stop
66+
'you.',
67+
'york.',
68+
]) {
69+
assert.equal(normalizeUrl(token), null, `${token} is not a hostname`);
70+
}
71+
});
72+
73+
test('normalizeUrl still accepts the unusual hosts that are real', () => {
74+
// The gate rejects what cannot be a hostname, not what is merely unusual --
75+
// so this is the half of the change that would break the directory if it
76+
// were wrong.
77+
assert.equal(normalizeUrl('sub.domain.co.uk'), 'https://sub.domain.co.uk/');
78+
assert.equal(normalizeUrl('xn--bcher-kva.de'), 'https://xn--bcher-kva.de/', 'IDN arrives punycoded');
79+
assert.equal(normalizeUrl('EXAMPLE.COM'), 'https://example.com/');
80+
assert.equal(normalizeUrl('news.ycombinator.com/rss'), 'https://news.ycombinator.com/rss');
81+
assert.equal(normalizeUrl('http://example.com:8080/feed'), 'http://example.com:8080/feed', 'a port is fine');
82+
// A feed genuinely served from a dotted quad is odd but not impossible, and
83+
// private ranges are refused later by isPublicHost rather than here.
84+
assert.equal(normalizeUrl('192.168.1.1'), 'https://192.168.1.1/');
85+
assert.equal(normalizeUrl('999.1.1.1'), null, 'but not a quad that cannot be one');
86+
});
87+
5088
test('findFeedLinks reads alternate links regardless of attribute order', () => {
5189
const html = `
5290
<link rel="alternate" type="application/rss+xml" href="/feed.xml">

0 commit comments

Comments
 (0)