fix: normalizeHeaders throws on flat header arrays from dns re-dispatch

lib/util/cache.js

@@ -31,13 +31,46 @@ function makeCacheKey (opts) {
 }
 
 /**
- * @param {Record<string, string[] | string>}
- * @returns {Record<string, string[] | string>}
+ * @param {unknown} val
+ * @returns {boolean}
+ */
+function isValidHeaderValue (val) {
+  if (typeof val === 'string') {
+    return true
+  }
+
+  if (!Array.isArray(val)) {
+    return false
+  }
+
+  for (let i = 0; i < val.length; i++) {
+    if (typeof val[i] !== 'string') {
+      return false
+    }
+  }
+
+  return true
+}
+
+/**
+ * @param {import('../../types/dispatcher.d.ts').default.DispatchOptions} opts
+ * @returns {Record<string, string | string[]>}
  */
 function normalizeHeaders (opts) {
   let headers
   if (opts.headers == null) {
     headers = {}
+  } else if (Array.isArray(opts.headers) && (opts.headers.length === 0 || !Array.isArray(opts.headers[0]))) {
+    // Flat alternating header array, e.g. ['key', 'value', ...].
+    headers = {}
+    for (let i = 0; i < opts.headers.length; i += 2) {
+      const key = opts.headers[i]
+      const val = opts.headers[i + 1]
+      if (typeof key !== 'string' || !isValidHeaderValue(val)) {
+        throw new Error('opts.headers is not a valid header map')
+      }
+      headers[key.toLowerCase()] = val
+    }
   } else if (typeof opts.headers === 'object') {
     headers = {}
 
@@ -47,7 +80,7 @@ function normalizeHeaders (opts) {
           throw new Error('opts.headers is not a valid header map')
         }
         const [key, val] = x
-        if (typeof key !== 'string' || typeof val !== 'string') {
+        if (typeof key !== 'string' || !isValidHeaderValue(val)) {
           throw new Error('opts.headers is not a valid header map')
         }
         headers[key.toLowerCase()] = val

test/cache-interceptor/cache-utils.js

@@ -42,3 +42,58 @@ test('normalizeHeaders handles headers from Map', (t) => {
 
   strictEqual(headers['x-test'], 'ok')
 })
+
+test('normalizeHeaders handles a flat alternating header array', (t) => {
+  const { strictEqual } = tspl(t, { plan: 2 })
+
+  const headers = normalizeHeaders({
+    headers: ['Host', 'example.com', 'X-Test', 'ok']
+  })
+
+  strictEqual(headers.host, 'example.com')
+  strictEqual(headers['x-test'], 'ok')
+})
+
+test('normalizeHeaders handles an empty flat header array', (t) => {
+  const { deepStrictEqual } = tspl(t, { plan: 1 })
+
+  const headers = normalizeHeaders({ headers: [] })
+
+  deepStrictEqual(headers, {})
+})
+
+test('normalizeHeaders handles a flat header array with an array value (e.g. repeated set-cookie)', (t) => {
+  const { deepStrictEqual } = tspl(t, { plan: 1 })
+
+  const headers = normalizeHeaders({
+    headers: ['Set-Cookie', ['a=1', 'b=2']]
+  })
+
+  deepStrictEqual(headers['set-cookie'], ['a=1', 'b=2'])
+})
+
+test('normalizeHeaders handles a tuple-style array with an array value', (t) => {
+  const { deepStrictEqual } = tspl(t, { plan: 1 })
+
+  const headers = normalizeHeaders({
+    headers: [['Set-Cookie', ['a=1', 'b=2']]]
+  })
+
+  deepStrictEqual(headers['set-cookie'], ['a=1', 'b=2'])
+})
+
+test('normalizeHeaders throws on a flat header array with a non-string, non-array value', (t) => {
+  const { throws } = tspl(t, { plan: 1 })
+
+  throws(() => normalizeHeaders({ headers: ['X-Test', 123] }), {
+    message: 'opts.headers is not a valid header map'
+  })
+})
+
+test('normalizeHeaders throws on a flat header array with an array value containing a non-string', (t) => {
+  const { throws } = tspl(t, { plan: 1 })
+
+  throws(() => normalizeHeaders({ headers: ['Set-Cookie', ['a=1', 2]] }), {
+    message: 'opts.headers is not a valid header map'
+  })
+})

test/interceptors/dns.js

@@ -12,7 +12,7 @@ const { tspl } = require('@matteo.collina/tspl')
 const pem = require('@metcoder95/https-pem')
 
 const { interceptors, Agent, Client, Pool, request } = require('../..')
-const { dns } = interceptors
+const { dns, cache, deduplicate } = interceptors
 
 // Helper to check if IPv6 is available for localhost
 // This is called synchronously at test definition time
@@ -2156,6 +2156,93 @@ test('#4444 - Should preserve iterable headers', async t => {
   t.equal(await response.body.text(), 'hello world!')
 })
 
+test('#5522 - Should not throw when dns re-dispatches into a composed cache interceptor', async t => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer()
+
+  server.on('request', (req, res) => {
+    res.writeHead(200, { 'content-type': 'text/plain' })
+    res.end('hello world!')
+  })
+
+  server.listen(0)
+
+  await once(server, 'listening')
+
+  // dns is the outer interceptor here, so its re-dispatch after resolving
+  // lands back in cache's normalizeHeaders() with a flat header array.
+  const client = new Agent().compose(cache(), dns({
+    lookup: (_origin, _opts, cb) => {
+      cb(null, [
+        {
+          address: '127.0.0.1',
+          family: 4
+        }
+      ])
+    }
+  }))
+
+  after(async () => {
+    await client.close()
+    server.close()
+
+    await once(server, 'close')
+  })
+
+  const response = await request(`http://localhost:${server.address().port}`, {
+    dispatcher: client,
+    headers: []
+  })
+
+  t.equal(response.statusCode, 200)
+  t.equal(await response.body.text(), 'hello world!')
+})
+
+test('#5522 - Should not throw when dns re-dispatches into a composed deduplicate interceptor', async t => {
+  t = tspl(t, { plan: 2 })
+
+  const server = createServer()
+
+  server.on('request', (req, res) => {
+    res.writeHead(200, { 'content-type': 'text/plain' })
+    res.end('hello world!')
+  })
+
+  server.listen(0)
+
+  await once(server, 'listening')
+
+  // Same root cause as the cache-interceptor case above, but through
+  // deduplicate() -- it shares the same normalizeHeaders()/makeCacheKey()
+  // utility from lib/util/cache.js, so it was equally affected.
+  const client = new Agent().compose(deduplicate(), dns({
+    lookup: (_origin, _opts, cb) => {
+      cb(null, [
+        {
+          address: '127.0.0.1',
+          family: 4
+        }
+      ])
+    }
+  }))
+
+  after(async () => {
+    await client.close()
+    server.close()
+
+    await once(server, 'close')
+  })
+
+  const response = await request(`http://localhost:${server.address().port}`, {
+    dispatcher: client,
+    headers: []
+  })
+
+  t.equal(response.statusCode, 200)
+  t.equal(await response.body.text(), 'hello world!')
+})
+
 test('#3951 - Should handle lookup errors correctly', async t => {
   const suite = tspl(t, { plan: 1 })