fix: dispatch receives raw headers

README.md

@@ -24,10 +24,10 @@ Configuration: Node v14.4, HTTP/1.1 without TLS, 100 connections, Linux 5.4.12-1
 
 ```
 http - keepalive x 5,634 ops/sec ±2.53% (274 runs sampled)
-undici - pipeline x 8,609 ops/sec ±2.98% (276 runs sampled)
-undici - request x 12,964 ops/sec ±0.74% (278 runs sampled)
-undici - stream x 14,068 ops/sec ±0.61% (279 runs sampled)
-undici - dispatch x 14,722 ops/sec ±0.63% (276 runs sampled)
+undici - pipeline x 8,642 ops/sec ±3.08% (276 runs sampled)
+undici - request x 12,681 ops/sec ±0.51% (279 runs sampled)
+undici - stream x 14,006 ops/sec ±0.53% (280 runs sampled)
+undici - dispatch x 15,002 ops/sec ±0.39% (278 runs sampled)
 ```
 
 The benchmark is a simple `hello world` [example](benchmarks/index.js).
@@ -438,7 +438,7 @@ The `handler` parameter is defined as follow:
 * `onHeaders(statusCode, headers, resume): Void`, invoked when statusCode and headers have been received. 
   May be invoked multiple times due to 1xx informational headers.
   * `statusCode: Number`
-  * `headers: Object`
+  * `headers: Array`, an array of key-value pairs. Keys are not automatically lowercased.
   * `resume(): Void`, resume `onData` after returning `false`.
 * `onData(chunk): Null|Boolean`, invoked when response payload data is received.
   * `chunk: Buffer`

lib/client-connect.js

@@ -4,6 +4,7 @@ const {
   InvalidArgumentError
 } = require('./errors')
 const { AsyncResource } = require('async_hooks')
+const util = require('./util')
 
 class ConnectHandler extends AsyncResource {
   constructor (opts, callback) {
@@ -23,7 +24,7 @@ class ConnectHandler extends AsyncResource {
     this.callback = null
     this.runInAsyncScope(callback, null, null, {
       statusCode,
-      headers,
+      headers: util.parseHeaders(headers),
       socket,
       opaque
     })

lib/client-pipeline.js

@@ -136,7 +136,7 @@ class PipelineHandler extends AsyncResource {
       this.handler = null
       body = this.runInAsyncScope(handler, null, {
         statusCode,
-        headers,
+        headers: util.parseHeaders(headers),
         opaque,
         body: this.res
       })

lib/client-request.js

@@ -55,7 +55,7 @@ class RequestHandler extends AsyncResource {
 
     this.runInAsyncScope(callback, null, null, {
       statusCode,
-      headers,
+      headers: util.parseHeaders(headers),
       opaque,
       body
     })

lib/client-stream.js

@@ -41,7 +41,7 @@ class StreamHandler extends AsyncResource {
     this.factory = null
     const res = this.runInAsyncScope(factory, null, {
       statusCode,
-      headers,
+      headers: util.parseHeaders(headers),
       opaque
     })
 
@@ -88,7 +88,7 @@ class StreamHandler extends AsyncResource {
       return
     }
 
-    this.trailers = trailers || {}
+    this.trailers = util.parseHeaders(trailers)
 
     res.end()
   }

lib/client-upgrade.js

@@ -4,6 +4,7 @@ const {
   InvalidArgumentError
 } = require('./errors')
 const { AsyncResource } = require('async_hooks')
+const util = require('./util')
 
 class UpgradeHandler extends AsyncResource {
   constructor (opts, callback) {
@@ -22,7 +23,7 @@ class UpgradeHandler extends AsyncResource {
 
     this.callback = null
     this.runInAsyncScope(callback, null, null, {
-      headers,
+      headers: util.parseHeaders(headers),
       socket,
       opaque
     })

lib/client.js

@@ -425,7 +425,11 @@ class Parser extends HTTPParser {
   }
 
   [HTTPParser.kOnHeaders] (rawHeaders) {
-    this.headers = util.parseHeaders(rawHeaders, this.headers)
+    if (this.headers) {
+      Array.prototype.push.apply(this.headers, rawHeaders)
+    } else {
+      this.headers = rawHeaders
+    }
   }
 
   [HTTPParser.kOnExecute] (ret) {
@@ -513,7 +517,12 @@ class Parser extends HTTPParser {
       return 1
     }
 
-    this.headers = util.parseHeaders(rawHeaders, this.headers)
+    if (this.headers) {
+      Array.prototype.push.apply(this.headers, rawHeaders)
+    } else {
+      this.headers = rawHeaders
+    }
+
     this.statusCode = statusCode
     this.shouldKeepAlive = shouldKeepAlive
 
@@ -530,18 +539,18 @@ class Parser extends HTTPParser {
     const { headers } = this
     this.headers = null
 
-    if (headers['keep-alive']) {
-      const m = headers['keep-alive'].match(/timeout=(\d+)/)
-      if (m) {
-        const keepAliveTimeout = Math.min(
-          Number(m[1]) * 1000 - client[kKeepAliveTimeoutThreshold],
-          client[kMaxKeepAliveTimeout]
-        )
-        if (!keepAliveTimeout || keepAliveTimeout < 1e3) {
-          client[kReset] = true
-        } else {
-          client[kKeepAliveTimeout] = keepAliveTimeout
-        }
+    // TODO: Have llhttp parse keep-alive timeout?
+    let keepAliveTimeout = util.parseKeepAliveTimeout(shouldKeepAlive, headers)
+
+    if (Number.isFinite(keepAliveTimeout)) {
+      keepAliveTimeout = Math.min(
+        keepAliveTimeout - client[kKeepAliveTimeoutThreshold],
+        client[kMaxKeepAliveTimeout]
+      )
+      if (!keepAliveTimeout || keepAliveTimeout < 1e3) {
+        client[kReset] = true
+      } else {
+        client[kKeepAliveTimeout] = keepAliveTimeout
       }
     } else {
       client[kKeepAliveTimeout] = client[kIdleTimeout]

lib/request.js

@@ -265,7 +265,7 @@ class Request {
     }
 
     try {
-      this[kHandler].onHeaders(statusCode, headers, resume)
+      this[kHandler].onHeaders(statusCode, headers || [], resume)
     } catch (err) {
       this.onError(err)
     }
@@ -291,7 +291,7 @@ class Request {
     reset.call(this)
 
     try {
-      this[kHandler].onComplete(trailers)
+      this[kHandler].onComplete(trailers || [])
     } catch (err) {
       this.onError(err)
     }

lib/util.js

@@ -47,6 +47,24 @@ function destroy (stream, err) {
   }
 }
 
+function parseKeepAliveTimeout (shouldKeepAlive, headers) {
+  if (!shouldKeepAlive) {
+    return null
+  }
+
+  let keepAliveHeader
+  for (let n = 0; n < headers.length; n += 2) {
+    const key = headers[n + 0]
+    if (key.length === 10 && key.toLowerCase() === 'keep-alive') {
+      keepAliveHeader = headers[n + 1]
+      break
+    }
+  }
+
+  const m = keepAliveHeader && keepAliveHeader.match(/timeout=(\d+)/)
+  return m ? Number(m[1]) * 1000 : null
+}
+
 function parseHeaders (headers, obj) {
   obj = obj || {}
   if (!headers) {
@@ -78,6 +96,7 @@ module.exports = {
   isStream,
   isDestroyed,
   parseHeaders,
+  parseKeepAliveTimeout,
   destroy,
   bodyLength,
   isBuffer

test/client-dispatch.js

@@ -2,6 +2,7 @@
 
 const { test } = require('tap')
 const { Client, errors } = require('..')
+const http = require('http')
 
 test('dispatch invalid opts', (t) => {
   t.plan(1)
@@ -18,3 +19,110 @@ test('dispatch invalid opts', (t) => {
     t.ok(err instanceof errors.InvalidArgumentError)
   }
 })
+
+test('basic dispatch get', (t) => {
+  t.plan(11)
+
+  const server = http.createServer((req, res) => {
+    t.strictEqual('/', req.url)
+    t.strictEqual('GET', req.method)
+    t.strictEqual('localhost', req.headers.host)
+    t.strictEqual(undefined, req.headers.foo)
+    t.strictEqual('bar', req.headers.bar)
+    t.strictEqual(undefined, req.headers['content-length'])
+    res.end('hello')
+  })
+  t.tearDown(server.close.bind(server))
+
+  const reqHeaders = {
+    foo: undefined,
+    bar: 'bar'
+  }
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    t.tearDown(client.close.bind(client))
+
+    const bufs = []
+    client.dispatch({
+      path: '/',
+      method: 'GET',
+      headers: reqHeaders
+    }, {
+      onHeaders (statusCode, headers) {
+        t.strictEqual(headers.length, 6)
+        t.strictEqual(statusCode, 200)
+        t.strictEqual(Array.isArray(headers), true)
+      },
+      onData (buf) {
+        bufs.push(buf)
+      },
+      onComplete (trailers) {
+        t.strictEqual(Array.isArray(trailers), true)
+        t.strictEqual('hello', Buffer.concat(bufs).toString('utf8'))
+      },
+      onError () {
+        t.fail()
+      }
+    })
+  })
+})
+
+test('trailers dispatch get', (t) => {
+  t.plan(13)
+
+  const server = http.createServer((req, res) => {
+    t.strictEqual('/', req.url)
+    t.strictEqual('GET', req.method)
+    t.strictEqual('localhost', req.headers.host)
+    t.strictEqual(undefined, req.headers.foo)
+    t.strictEqual('bar', req.headers.bar)
+    t.strictEqual(undefined, req.headers['content-length'])
+    res.addTrailers({ 'Content-MD5': 'test' })
+    res.setHeader('Content-Type', 'text/plain')
+    res.setHeader('Trailer', 'Content-MD5')
+    res.end('hello')
+  })
+  t.tearDown(server.close.bind(server))
+
+  const reqHeaders = {
+    foo: undefined,
+    bar: 'bar'
+  }
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    t.tearDown(client.close.bind(client))
+
+    const bufs = []
+    client.dispatch({
+      path: '/',
+      method: 'GET',
+      headers: reqHeaders
+    }, {
+      onHeaders (statusCode, headers) {
+        t.strictEqual(headers.length, 10)
+        t.strictEqual(statusCode, 200)
+        t.strictEqual(Array.isArray(headers), true)
+        {
+          const contentTypeIdx = headers.findIndex(x => x === 'Content-Type')
+          t.strictEqual(headers[contentTypeIdx + 1], 'text/plain')
+        }
+      },
+      onData (buf) {
+        bufs.push(buf)
+      },
+      onComplete (trailers) {
+        t.strictEqual(Array.isArray(trailers), true)
+        {
+          const contentMD5Idx = trailers.findIndex(x => x === 'Content-MD5')
+          t.strictEqual(trailers[contentMD5Idx + 1], 'test')
+        }
+        t.strictEqual('hello', Buffer.concat(bufs).toString('utf8'))
+      },
+      onError () {
+        t.fail()
+      }
+    })
+  })
+})