feat(node): expose the raw response bytes alongside the decoded text - #1
Open
onamfc wants to merge 1 commit into
Open
feat(node): expose the raw response bytes alongside the decoded text#1onamfc wants to merge 1 commit into
onamfc wants to merge 1 commit into
Conversation
The fetcher read every response through a TextDecoder and returned only the string. That is lossy for anything that is not UTF-8, so an image fetched through it could never be recovered — a PNG's leading 0x89 byte decodes to U+FFFD and the original is gone. That mattered the moment a second caller appeared. LinkForty's generated social cards composite a user-supplied logo or hero image into the card, which means fetching an arbitrary URL server-side: exactly the SSRF position this fetcher exists to make safe. Without bytes, that caller would have had to stand up its own fetcher and reimplement the per-hop address checks — two copies of the control that matters most. readCapped now collects Uint8Array chunks and decodes once at the end, so `body` is unchanged for existing callers and `bodyBytes` carries the original. One read, one code path, both shapes. `bodyBytes` is optional on FetchResult so a custom adapter that only ever handles text stays valid. 72 tests, up from 69. Build, typecheck and lint clean.
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.
The fetcher read every response through a
TextDecoderand returned only the string. That is lossy for anything that is not UTF-8, so an image fetched through it could never be recovered — a PNG's leading0x89decodes toU+FFFDand the original is gone.Why it matters now
LinkForty's generated social cards (SIT-368/369) composite a user-supplied logo or hero image into the card, which means fetching an arbitrary URL server-side — exactly the SSRF position this fetcher exists to make safe.
Without access to bytes, that caller had two options: decode-and-hope (broken), or stand up its own fetcher and reimplement the per-hop address checks. The second is worse than it sounds — it would mean two copies of the control that matters most, and the copy would inevitably drift from this one.
The change
readCappednow collectsUint8Arraychunks and decodes once at the end:One read, one code path, both shapes.
bodyis byte-for-byte unchanged for existing callers —og-core's parser neither knows nor cares.bodyBytesis optional onFetchResultso a custom adapter that only ever handles text remains valid.Verification
body; text bodies still decode correctly including multibyte characters; the byte cap applies to both.pnpm build,pnpm typecheck,pnpm lintclean.The PNG test asserts the point directly:
Versions
@linkforty/og-coreand@linkforty/og-nodeboth to 0.2.0 — minor, sinceFetchResultgains an optional field and no existing behaviour changes.Note
This is a design flaw I introduced when specifying SIT-369: I wrote the ticket saying to fetch card images with
createNodeFetcher, without checking that it returns text. Fixing it here rather than duplicating the fetcher in Cloud is the correct resolution — the guarded fetcher should be usable for every server-side fetch we do, not just HTML.