A dot is not a hostname - #148
Merged
Merged
Conversation
`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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
normalizeUrlguarded the host withhostname.includes('.').new URL()is far more permissive about hosts than DNS is — quotes, parens,=and&are not forbidden host code points — so all of these parsed, all contained a dot, and all became feeds:version="1.0"https://version="1.0"/text="gg.dealshttps://text="gg.deals/zombies.)https://zombies.)/z.https://z./~3,700 such rows are in production. 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 is then re-crawled forever on a cadence and fails
blocked-host; they were most of a 960/hour spike in that error.Why the check goes here
normalizeUrlis the one gate every entry path shares — web submit, OPML import, the queue drain, discovery — so one check covers all of them. The scanner itself can't do it: it runs in the browser, and importing@rssamplifier/feedinto client code fails the build onnode:dns.Scope
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; 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's spelled out explicitly.Full workspace suite green (11 packages, 0 failures).
Note: this stops new junk. The ~3,700 existing rows are a separate cleanup.
🤖 Generated with Claude Code