fix(functions): merge headers case-insensitively - #2578
Conversation
Header names are case-insensitive (RFC 9110), but an object spread only
overrides on an exact key match. `invoke` merged three unnormalized
sources, so an entry differing only in case survived alongside its
counterpart and `fetch` joined the two into one comma-separated value:
new FunctionsClient(url, { headers: { 'content-type': 'application/json' } })
.invoke('fn', { headers: { 'Content-Type': 'text/plain' }, body: 'x' })
// sent: content-type: application/json, text/plain
`setAuth` had the same problem and a worse consequence: it assigned
`headers.Authorization`, so a client constructed with a lowercase
`authorization` kept sending the stale token too, as
`authorization: Bearer stale, Bearer fresh`.
Normalize header keys to lowercase at both boundaries — the constructor
and each invoke — so the documented precedence (invoke > client >
default) actually holds. Mirrors `normalizeHeaders` in storage-js.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Description
Header names are case-insensitive (RFC 9110), but an object spread only overrides on an exact key match.
invokemerged three unnormalized sources, so an entry differing only in case survived alongside its counterpart — andfetchjoins two same-name headers into one comma-separated value, so the result is a malformed header rather than a wrong-but-valid one.That silently contradicts both the precedence comment above the merge (invoke > client > default) and the
invokedocstring, which tells callers they can override the SDK-chosenContent-Type.setAuthhad the same root cause and a worse consequence. It assignedheaders.Authorization, so a client constructed with a lowercaseauthorizationkept sending the stale token alongside the new one:No server accepts that, so
setAuthbreaks the client for anyone who spelled the header in lowercase.What changed
Header keys are normalized to lowercase at the two boundaries where headers enter the client — the constructor and each
invoke— so the existing spread expresses the documented precedence again. The internally chosenContent-Typedefaults are written lowercase for the same reason.normalizeHeadersmirrors the helper of the same name instorage-js(src/lib/common/headers.ts), including its semantics: last value wins, input not mutated. It is duplicated rather than shared becausefunctions-jshas no dependency onstorage-jsandpackages/sharedcurrently holds onlytracing— happy to extract it intopackages/sharedinstead if you would prefer that.The existing case-insensitive
hasContentTypeHeadercheck is untouched; it was already correct.Testing
New
packages/core/functions-js/test/header-case.test.ts— 3 unit tests using a capturedfetch, asserting throughnew Headers(...)so the assertion sees what the platform actually sends.On
masterall three fail, with exactly the joined values:masterContent-Typeover clientcontent-typeapplication/json, text/plaintext/plainsetAuthover clientauthorizationBearer stale, Bearer freshBearer freshx-customover clientX-Customclient, invokeinvokeFull
nx test functions-js(testcontainers-backed Deno relay):masterBoth fully green; the delta is exactly this suite and its 3 tests.
nx lint functions-jsreports 17 errors both here and onmaster— all pre-existing.nx format:checkandnx build functions-jspass.nx test supabase-jsfails identically on this branch and onmasterwithThe type definition dist/index.d.cts does not exist, which is a local build-order issue unrelated to this change.Related, not fixed here
The same class of bug exists in
auth-js.lib/fetch.tsmerges headers with plain spreads and setsheaders['Authorization']by exact case, so a client created withglobal.headers: { authorization: '…' }(lowercase, a documented option) sends every auth request withauthorization: Bearer <jwt>, Bearer <custom>. Reachable straight fromcreateClient.I have left that out of this PR because
auth-jsis security-critical and has a wider header surface, so it deserves its own review. Happy to open it separately — or to fold it in here if you would rather have one change.Type of Change
Checklist
nx format)nx test functions-js, fully green)nx build functions-js)