fix(blob): decode delegation token payload as UTF 8 in presignUrl - #1103
Open
Om-singhaI wants to merge 1 commit into
Open
fix(blob): decode delegation token payload as UTF 8 in presignUrl#1103Om-singhaI wants to merge 1 commit into
Om-singhaI wants to merge 1 commit into
Conversation
presignUrl reads the pathname scope out of the delegation token payload and compares it against the pathname the caller passed in. The payload segment was decoded with atob whenever that global exists, and atob returns one character per byte, so any pathname containing non ASCII characters came back as mojibake. Node 18 and later define atob globally, which means the Buffer branch that would have decoded UTF 8 correctly never ran on the server. As a result uploads such as uploads/Skærmbillede.png failed with a false "Blob path does not match the signed token scope" error even though the two strings were identical. The atob branch now turns the binary string into bytes and decodes them with TextDecoder when that global exists, so browsers, edge and Node all produce the same UTF 8 string. React Native on Hermes ships atob but, before the new Hermes stable release, no TextDecoder; in that case the raw atob string is returned exactly as before, so ASCII payloads keep working there and the client side presigned upload flow is not affected. The same change is applied to the duplicate helper in helpers.ts that reads storeId from a delegation token, so both decoders behave identically. Adds shared presignUrl tests that run under both the Node and jsdom suites: one with ASCII, Danish, Cyrillic, Japanese and emoji pathnames, and one that removes the TextDecoder global to exercise the React Native fallback for both decoders. Also adds a patch changeset for @vercel/blob. Fixes vercel#1101
🦋 Changeset detectedLatest commit: e308443 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Someone is attempting to deploy a commit to the Curated Tests - Permanent E2E Team on Vercel. A member of the Team first needs to authorize it. |
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.
fix(blob): decode delegation token payload as UTF 8 in presignUrl
Fixes #1101
What is broken and why
presignUrlreads thepathnamescope out of the delegation token payload and compares it with thepathnamethe caller passed in. Any pathname holding a non ASCII character (Danish, German, Cyrillic, Japanese, emoji, and so on) is rejected with:The two strings are the same. The token is simply being read back wrong.
Root cause is
base64UrlDecodeToStringinpackages/blob/src/signed-token.ts(line 168 on main). It prefersatobwhenever that global exists:atobreturns one character per byte, so a UTF 8 payload comes back as mojibake. Node 18 and later defineatobglobally, so theBufferbranch that would have decoded UTF 8 correctly never runs on the server, which is exactly wherehandleUploadPresignedandpresignUrlare called.tryDecodePayloadthen parses the mojibake JSON and the scope check inpresign(line 364 on main) throws.The same decoder is duplicated as
base64UrlDecodeDelegationSegmentinpackages/blob/src/helpers.ts(line 182 on main). That copy only readsstoreId, which is ASCII, so it does not misbehave today, but it has the same latent bug.The decoder was introduced in #1056 and has not changed since, so
@vercel/blob2.6.1 through 2.8.0 are all affected.The fix
In both helpers, the
atobbranch now converts the binary string into bytes and decodes those bytes as UTF 8 withTextDecoderwhen that global exists. When it does not, the rawatobstring is returned exactly as before:Why the guard matters: this package supports React Native and already feature detects
TextEncoderfor it (computeBodyLengthinhelpers.ts). Hermes addedatobandTextEncoderin React Native 0.74, but nativeTextDecoderonly arrives with the new Hermes stable release, so apps on React Native 0.74 through 0.83 without a polyfill haveatoband noTextDecoder. On those appsparseStoreIdFromDelegationTokenruns insideresolveBlobAuthfor every presigned upload, and an unguardednew TextDecoder()there would have turned every presigned upload, ASCII pathnames included, intoInvalid delegation token payload.. With the guard, that runtime keeps the behaviour it has today (ASCII payloads decode, non ASCII payloads are still mojibake there untilTextDecoderexists), while browsers, the Edge runtime and Node, which all shipTextDecoder, get the correct UTF 8 decode. No other behaviour changes.Files changed:
packages/blob/src/signed-token.tsatobbranch ofbase64UrlDecodeToString, guarded onTextDecoderpackages/blob/src/helpers.tsbase64UrlDecodeDelegationSegmentpackages/blob/src/signed-token.presignurl.shared-spec.tsaccepts non-ASCII pathnames that match the token scopeandfalls back to the raw atob string when TextDecoder is unavailable.changeset/blob-presign-utf8-pathname.md@vercel/blobTesting
Reproduction on pristine main (31245dc, Node 25.6.1)
Built the package and ran the script from the issue against
dist/index.js:After the fix, rebuilt and reran: all five pathnames are
accepted.New tests
Both are added next to the existing path scope tests in
signed-token.presignurl.shared-spec.ts, so they run under both the Node and the jsdom (browser) suites.accepts non-ASCII pathnames that match the token scope: issues a delegation token scoped to each ofuploads/plain-ascii.png,uploads/Skærmbillede.png,uploads/Снимок.png,uploads/スクリーンショット.pnganduploads/😀.png(the test helper builds the payload withBuffer.from(json, 'utf8'), the same way the API does), then assertspresignaccepts the matching pathname and produces the expected HMAC.falls back to the raw atob string when TextDecoder is unavailable: removes theTextDecoderglobal for the duration of the test (restored infinally), then assertspresignstill accepts an ASCII pathname and thatparseStoreIdFromDelegationTokenstill returns the store id. This covers the fallback branch in both decoders.Commands (from
packages/blob):signed-token.node.test.tssigned-token.browser.test.tsProof the tests catch what they are meant to catch (source files swapped, spec kept, both environments gave the same result):
TextDecoderguard: test 1 passes, test 2 fails, reproducing exactly the React Native regression the guard prevents.Full
@vercel/blobsuite (pnpm run test, which runs node, edge and browser)Lint and types
biome check .inpackages/blob: exit 0. It prints two warnings, both already present on main and untouched here:lint/correctness/noUnusedImportsfor theBlobErrorimport insrc/create-folder.ts, andlint/correctness/noUnusedVariablesfornormalizeStoreIdinsrc/signed-token.ts.tsc --noEmitinpackages/blob: exit 0.lint-stagedpre commit hook ran biome on the staged files and made no changes.Notes for reviewers
TextDecoderglobal under Node and jsdom, which reproduces the condition (atobpresent,TextDecoderabsent) rather than the runtime itself.signed-token.tsand the shared spec, but it does not touch the decoder, and its spec hunk starts after therejects path mismatchtest. The new tests here are placed before that test so the two should not conflict.helpers.tschange is purely for consistency;storeIdis ASCII so that path was not failing in practice.TextDecoderwith its default options substitutes U+FFFD for invalid UTF 8 and drops a leading BOM, whereasBuffer.toString('utf8')keeps the BOM. Neither case can occur for the JSON payloads the API issues, so theatobandBufferbranches agree for real tokens.