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

@@ -19,10 +19,13 @@ type FetchRequestOptions = RequestInit & {
   basicAuth?: BasicAuthOptions
 }
 
+type FetchHeaders = NonNullable<RequestInit['headers']>
+
 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 ' +

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',