From 833e44d14240a1c726ef12578593de85a2e1b890 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Wed, 19 Aug 2026 12:05:54 -0400 Subject: [PATCH] Merge pull request #36733 from overleaf/dk-fetch-utils-unset-headers [fetch-utils] Drop headers with unset values GitOrigin-RevId: 284b8551dfc3b15315a873a9256fbf9f35e4004b --- libraries/fetch-utils/index.ts | 52 ++++++++++++++----- .../fetch-utils/test/unit/FetchUtils.test.js | 46 ++++++++++++++++ 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/libraries/fetch-utils/index.ts b/libraries/fetch-utils/index.ts index 383fb60cd2..fa478670e3 100644 --- a/libraries/fetch-utils/index.ts +++ b/libraries/fetch-utils/index.ts @@ -19,10 +19,13 @@ type FetchRequestOptions = RequestInit & { basicAuth?: BasicAuthOptions } +type FetchHeaders = NonNullable + type ParsedFetchOptions = Omit< FetchRequestOptions, - 'json' | 'signal' | 'basicAuth' + 'json' | 'signal' | 'basicAuth' | 'headers' > & { + headers: Headers signal?: AbortSignal } @@ -45,9 +48,6 @@ async function fetchJsonWithResponse( opts: FetchRequestOptions = {} ) { const { fetchOpts, detachSignal } = parseOpts(opts, url) - if (!(fetchOpts.headers instanceof Headers)) { - fetchOpts.headers = new Headers(fetchOpts.headers) - } if (!fetchOpts.headers.has('Accept')) { fetchOpts.headers.set('Accept', 'application/json') } @@ -205,9 +205,43 @@ function setupDefaultAgent(fetchOpts: ParsedFetchOptions) { url.protocol === 'https:' ? httpsAgent : httpAgent } +/** + * Normalise the headers option into a Headers instance, which is what + * node-fetch builds from it anyway. + * + * Headers with an unset value are dropped: header values are stringified, so + * they would otherwise be sent as the literal string `undefined`. + */ +function parseHeaders(headers: FetchHeaders): Headers { + // A Headers instance can't hold unset values. Pass it through rather than + // iterate it, which would collapse repeated headers into a single + // comma-separated value. + if (headers instanceof Headers) { + return headers + } + // Not in the TS type, but node-fetch accepts any iterable of entries, such as + // a Map or a global Headers instance. Those expose no own enumerable + // properties, so they have to be iterated rather than passed to + // Object.entries(). + const entries: Iterable<[string, unknown]> = + Symbol.iterator in headers + ? (headers as Iterable<[string, unknown]>) + : Object.entries(headers) + const parsedHeaders = new Headers() + for (const [name, value] of entries) { + if (value != null) { + parsedHeaders.append(name, String(value)) + } + } + return parsedHeaders +} + function parseOpts(opts: FetchRequestOptions, url: string | URL) { - const { json, signal, basicAuth, ...rawFetchOpts } = opts - const fetchOpts: ParsedFetchOptions = { ...rawFetchOpts } + const { json, signal, basicAuth, headers, ...rawFetchOpts } = opts + const fetchOpts: ParsedFetchOptions = { + ...rawFetchOpts, + headers: parseHeaders(headers ?? {}), + } if (json) { setupJsonBody(fetchOpts, json) } @@ -253,9 +287,6 @@ function parseOpts(opts: FetchRequestOptions, url: string | URL) { function setupJsonBody(fetchOpts: ParsedFetchOptions, json: unknown) { fetchOpts.body = JSON.stringify(json) - if (!(fetchOpts.headers instanceof Headers)) { - fetchOpts.headers = new Headers(fetchOpts.headers) - } fetchOpts.headers.set('Content-Type', 'application/json') } @@ -263,9 +294,6 @@ function setupBasicAuth( fetchOpts: ParsedFetchOptions, basicAuth: BasicAuthOptions ) { - if (!(fetchOpts.headers instanceof Headers)) { - fetchOpts.headers = new Headers(fetchOpts.headers) - } fetchOpts.headers.set( 'Authorization', 'Basic ' + diff --git a/libraries/fetch-utils/test/unit/FetchUtils.test.js b/libraries/fetch-utils/test/unit/FetchUtils.test.js index d4c112447b..0f8bfbaa6e 100644 --- a/libraries/fetch-utils/test/unit/FetchUtils.test.js +++ b/libraries/fetch-utils/test/unit/FetchUtils.test.js @@ -184,6 +184,52 @@ describe('fetch-utils', function () { expect(server.lastReq.headers['x-foo']).to.equal('bar') }) + it('treats an unset Accept header as absent and applies the default', async function () { + await fetchJson(url('/json/hello'), { + headers: { Accept: undefined }, + }) + expect(server.lastReq.headers.accept).to.equal('application/json') + }) + + it('omits headers with unset values rather than sending "undefined"', async function () { + await fetchJson(url('/json/hello'), { + headers: { 'x-some-value': undefined, 'x-other-value': null }, + }) + expect(server.lastReq.headers).to.not.have.property('x-some-value') + expect(server.lastReq.headers).to.not.have.property('x-other-value') + }) + + it('omits headers with unset values provided as an array of tuples', async function () { + await fetchJson(url('/json/hello'), { + headers: [ + ['X-Foo', 'bar'], + ['X-Unset', undefined], + ], + }) + expect(server.lastReq.headers['x-foo']).to.equal('bar') + expect(server.lastReq.headers).to.not.have.property('x-unset') + }) + + it('omits headers with unset values provided as a Map', async function () { + await fetchJson(url('/json/hello'), { + headers: new Map([ + ['X-Foo', 'bar'], + ['X-Unset', undefined], + ]), + }) + expect(server.lastReq.headers['x-foo']).to.equal('bar') + expect(server.lastReq.headers).to.not.have.property('x-unset') + }) + + it('keeps headers provided as a global Headers instance', async function () { + // Not an instance of node-fetch's Headers: it exposes no own + // enumerable properties to iterate over. + await fetchJson(url('/json/hello'), { + headers: new globalThis.Headers({ 'X-Foo': 'bar' }), + }) + expect(server.lastReq.headers['x-foo']).to.equal('bar') + }) + it('sets a Content-Type header of application/json when sending a JSON body', async function () { await fetchJson(url('/json/add'), { method: 'POST',