Read the whole head, and fix isPrivateIp while we are in there - #128
Merged
Conversation
`num & r.mask` produces a signed 32-bit result. Every prefix in the list from 128.0.0.0 up is written as an unsigned literal, so for those the comparison could never come out true. 172.16/12, 192.168/16 and 169.254/16 all answered false. Only 10/8 and 127/8 ever worked, which is two of the five ranges the list claims to cover. `lan_open` gates on this in socket/handlers/join.ts. So a client on a 192.168 address was not seen as being on the LAN, and had to use an invite anyway. That is the range nearly every home router hands out, so the feature was off for most of the people it exists for. 169.254/16 is where the cloud metadata service answers. The link preview work in the next commit leans on this function to decide whether the server may fetch a URL somebody pasted. The fix is `>>> 0` on the masked value. The tests cover all five ranges, the addresses either side of each boundary, IPv6, the IPv4-mapped form, and junk input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The read stopped at 50 KB. Measured 2026-09-03: MDN closes its head at 5.9 KB and Steam at 8.3 KB, but Modrinth's og:title sits at byte 246,740 and YouTube's at 699,799. Both of those returned nothing, and drew a card with a hostname and an empty grey box. The read now stops at `</head>` or 1 MB. Pages that were already fine stop exactly where they did before, GitHub at 32 KB and MDN at 8 KB, and the two that were broken come back complete. The parsing moves to utils/pageMetadata.ts. It now picks up og:image:width and height, og:type, theme-color, the image alt text, the author, the published time, and any oEmbed endpoint the page advertises. When a page declares its image size, the second request that used to measure it is skipped, and most of the web declares it. theme-color gives a card its accent colour without anybody writing a special case per site: Modrinth's green, Steam's near-black, GitHub's slate. Reading the tags is regex rather than a parse, as before, but written for the variation that actually turns up: either attribute order, single or unquoted values, `rel` as a token list, numeric entities. One case it now gets right is a `<link>` whose rel and href come from two different tags, which is how a stylesheet used to become a favicon. A non-2xx page returns its status and nothing else. A private GitHub repo 404s to anyone not signed in, and GitHub's 404 page carries GitHub's own metadata. Parsing that produced a card titled "Build software better, together" for a link to somebody's repository. With the status, the client can say something honest instead. oEmbed now finds endpoints by asking the page for them, rather than only knowing four hosts by name. Any site that advertises one works: Flickr, CodePen, Giphy, every WordPress install, most Mastodon servers. Reddit and Bluesky are added by name as well, since both want specific parameters. Redirects are followed by hand, one hop at a time, so the check applies to the address actually connected to rather than only to the first one. The oEmbed endpoint read out of a remote page goes through the same check, since it is a URL an attacker controls as surely as the one in the chat box. Both are new because the discovery this commit adds needs them. Still open: DNS is resolved to check it and resolved again to connect, so a name that answers differently the second time is not covered. Closing that means connecting to the address rather than the name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 3, 2026
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
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.
Link previews came back empty for a lot of sites, and the card the client drew
for an empty preview was the "weird card" in the screenshot that started this.
Two changes here: one bug that is not about previews at all, and the preview
work itself.
Look at this first
isPrivateIpanswered false for 172.16/12, 192.168/16 and 169.254/16.num & r.maskgives a signed 32-bit result and those prefixes are unsignedliterals, so the comparison could never be true for them. Only 10/8 and 127/8
ever worked.
Two things follow from that, and neither is about link previews:
lan_opengates on it insocket/handlers/join.ts, so a client on a 192.168address was never seen as being on the LAN. That is the range nearly every
home router hands out, so the feature was off for most of the people it is
for. I have not tested that path end to end — worth a look.
Fix is
>>> 0on the masked value, in its own commit, with tests over all fiveranges and the addresses either side of each boundary. If you would rather this
went in on its own, say so and I will split it out.
The previews
The read stopped at 50 KB. Measured 2026-09-03: MDN closes its head at 5.9 KB
and Steam at 8.3 KB, but Modrinth's
og:titlesits at byte 246,740 andYouTube's at 699,799. Both returned nothing at all. The read now stops at
</head>or 1 MB, so GitHub still stops at 32 KB and MDN at 8 KB, and the twothat were broken come back complete.
Parsing moves to
utils/pageMetadata.tsand picks upog:image:width/height(which lets it skip the request that used to measure the image),
og:type,theme-color, image alt, author, published time, and any oEmbed endpoint thepage advertises.
theme-coloris the one that pays off for sites nobody haswritten a special case for: Modrinth's green, Steam's near-black, GitHub's
slate.
A non-2xx page now returns its status and nothing else. GitHub 404s a private
repository to anyone not signed in and its 404 page carries GitHub's own
metadata, so parsing it produced a card titled "Build software better,
together" for a link to somebody's repo.
oEmbed finds endpoints by reading them off the page rather than only knowing
four hosts, so Flickr, CodePen, Giphy, WordPress and Mastodon work without
being listed. Reddit and Bluesky are added by name since both want particular
parameters.
What changes about outbound requests
This is the part I am least sure about and the reason the diff is bigger than
the feature.
Redirects are now followed one hop at a time with the check applied to each,
because
redirect: "follow"hands the whole chain to undici and the third hopis as attacker-chosen as the first. The oEmbed endpoint discovered from a page
goes through the same check for the same reason.
checkPreviewUrlalsoresolves the hostname, so a public name with an A record pointing at
169.254.169.254 is refused rather than fetched.
Still open, and not closed here: DNS is resolved to check and resolved again to
connect, so a name that answers differently the second time gets through.
Closing it means connecting to the address rather than the name, which needs a
custom agent.
The 1 MB cap is a real bandwidth increase for a page like YouTube's. It is
server-side, cached for an hour, and only for links people actually post, but
it is a number worth disagreeing with if you want to.
Checks
yarn test: 659 pass. The 27 failures are all WindowsEPERMon temp-directorycleanup in
afterhooks — every assertion in those files passes, and they failthe same way on
main.tsc --noEmitclean.Client and mobile read the new fields: