fix: some cleanup (#792)

* fix: some cleanup

- Avoid extra loop for getting keep-alive and trailer headers.
- headers in dispatch callbacks is always an array.

* fixup

* fixup

* fixup

* fixup

docs/api/Dispatcher.md

@@ -203,10 +203,10 @@ Returns: `void`
 
 * **onConnect** `(abort: () => void, context: object) => void` - Invoked before request is dispatched on socket. May be invoked multiple times when a request is retried when the request at the head of the pipeline fails.
 * **onError** `(error: Error) => void` - Invoked when an error has occurred.
-* **onUpgrade** `(statusCode: number, headers: Buffer[] | null, socket: Duplex) => void` (optional) - Invoked when request is upgraded. Required if `DispatchOptions.upgrade` is defined or `DispatchOptions.method === 'CONNECT'`.
-* **onHeaders** `(statusCode: number, headers: Buffer[] | null, resume: () => void) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
+* **onUpgrade** `(statusCode: number, headers: Buffer[], socket: Duplex) => void` (optional) - Invoked when request is upgraded. Required if `DispatchOptions.upgrade` is defined or `DispatchOptions.method === 'CONNECT'`.
+* **onHeaders** `(statusCode: number, headers: Buffer[], resume: () => void) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
 * **onData** `(chunk: Buffer) => boolean` - Invoked when response payload data is received. Not required for `upgrade` requests.
-* **onComplete** `(trailers: Buffer[] | null) => void` - Invoked when response payload and trailers have been received and the request has completed. Not required for `upgrade` requests.
+* **onComplete** `(trailers: Buffer[]) => void` - Invoked when response payload and trailers have been received and the request has completed. Not required for `upgrade` requests.
 
 #### Example 1 - Dispatch GET request
 

lib/client.js

@@ -447,6 +447,9 @@ class Parser {
     this.shouldKeepAlive = false
     this.paused = false
     this.resume = this.resume.bind(this)
+
+    this.trailer = ''
+    this.keepAlive = ''
   }
 
   setTimeout (value, type) {
@@ -573,14 +576,22 @@ class Parser {
   }
 
   onHeaderValue (buf) {
-    const len = this.headers.length
+    let len = this.headers.length
 
     if ((len & 1) === 1) {
       this.headers.push(buf)
+      len += 1
     } else {
       this.headers[len - 1] = Buffer.concat([this.headers[len - 1], buf])
     }
 
+    const key = this.headers[len - 2]
+    if (key.length === 10 && key.toString().toLowerCase() === 'keep-alive') {
+      this.keepAlive += buf.toString()
+    } else if (key.length === 7 && key.toString().toLowerCase() === 'trailer') {
+      this.trailer += buf.toString()
+    }
+
     this.trackHeader(buf.length)
   }
 
@@ -649,7 +660,7 @@ class Parser {
   }
 
   onHeadersComplete (statusCode, upgrade, shouldKeepAlive) {
-    const { client, socket, headers: rawHeaders } = this
+    const { client, socket, headers } = this
 
     /* istanbul ignore next: difficult to make a test case for */
     if (socket.destroyed) {
@@ -710,27 +721,8 @@ class Parser {
     this.headers = []
     this.headersSize = 0
 
-    let keepAlive
-    let trailers
-
-    let looking = true
-    for (let n = 0; n < rawHeaders.length && looking; n += 2) {
-      const key = rawHeaders[n]
-      const val = rawHeaders[n + 1]
-
-      if (!keepAlive && key.length === 10 && key.toString().toLowerCase() === 'keep-alive') {
-        keepAlive = val
-        looking = !trailers
-      } else if (!trailers && key.length === 7 && key.toString().toLowerCase() === 'trailer') {
-        trailers = val
-        looking = !keepAlive
-      }
-    }
-
-    this.trailers = trailers ? trailers.toString().toLowerCase().split(/,\s*/) : []
-
     if (shouldKeepAlive && client[kPipelining]) {
-      const keepAliveTimeout = keepAlive ? util.parseKeepAliveTimeout(keepAlive) : null
+      const keepAliveTimeout = this.keepAlive ? util.parseKeepAliveTimeout(this.keepAlive) : null
 
       if (keepAliveTimeout != null) {
         const timeout = Math.min(
@@ -751,7 +743,7 @@ class Parser {
     }
 
     try {
-      if (request.onHeaders(statusCode, rawHeaders, this.resume) === false) {
+      if (request.onHeaders(statusCode, headers, this.resume) === false) {
         return constants.ERROR.PAUSED
       }
     } catch (err) {
@@ -798,7 +790,7 @@ class Parser {
   }
 
   onMessageComplete () {
-    const { client, socket, statusCode, upgrade, trailers, headers: rawTrailers, shouldKeepAlive } = this
+    const { client, socket, statusCode, upgrade, trailer, headers, shouldKeepAlive } = this
 
     if (socket.destroyed && (!statusCode || shouldKeepAlive)) {
       return -1
@@ -814,7 +806,8 @@ class Parser {
     assert(statusCode >= 100)
 
     this.statusCode = null
-    this.trailers = null
+    this.trailer = ''
+    this.keepAlive = ''
 
     assert(this.headers.length % 2 === 0)
     this.headers = []
@@ -824,11 +817,12 @@ class Parser {
       return
     }
 
+    const trailers = trailer ? trailer.split(/,\s*/) : []
     for (let i = 0; i < trailers.length; i++) {
       const trailer = trailers[i]
       let found = false
-      for (let n = 0; n < rawTrailers.length; n += 2) {
-        const key = rawTrailers[n]
+      for (let n = 0; n < headers.length; n += 2) {
+        const key = headers[n]
         if (key.length === trailer.length && key.toString().toLowerCase() === trailer.toLowerCase()) {
           found = true
           break
@@ -841,7 +835,7 @@ class Parser {
     }
 
     try {
-      request.onComplete(rawTrailers.length ? rawTrailers : null)
+      request.onComplete(headers)
     } catch (err) {
       request.onError(err)
       assert(request.aborted)

package.json

@@ -33,7 +33,7 @@
     "build:wasm": "node build/wasm.js --docker",
     "lint": "standard | snazzy",
     "lint:fix": "standard --fix | snazzy",
-    "test": "tap test/*.js --no-coverage && jest test/jest/test",
+    "test": "tap test/*.js --no-coverage --reporter classic && jest test/jest/test",
     "test:tdd": "tap test/*.js -w --no-coverage-report",
     "test:typescript": "tsd",
     "coverage": "standard | snazzy && tap test/*.js",
@@ -48,22 +48,22 @@
   },
   "devDependencies": {
     "@sinonjs/fake-timers": "^7.0.5",
-    "@types/node": "^14.14.39",
+    "@types/node": "^15.0.2",
     "abort-controller": "^3.0.0",
-    "concurrently": "^6.0.2",
+    "concurrently": "^6.1.0",
     "cronometro": "^0.8.0",
-    "docsify-cli": "^4.4.2",
+    "docsify-cli": "^4.4.3",
     "https-pem": "^2.0.0",
     "husky": "^6.0.0",
-    "jest": "^26.4.0",
+    "jest": "^26.6.3",
     "pre-commit": "^1.2.2",
     "proxy": "^1.0.2",
-    "proxyquire": "^2.0.1",
+    "proxyquire": "^2.1.3",
     "semver": "^7.3.5",
     "sinon": "^10.0.0",
     "snazzy": "^9.0.0",
     "standard": "^16.0.3",
-    "tap": "^15.0.0",
+    "tap": "^15.0.9",
     "tsd": "^0.14.0",
     "wait-on": "^5.3.0"
   },

test/client-dispatch.js

@@ -114,7 +114,7 @@ test('basic dispatch get', (t) => {
         bufs.push(buf)
       },
       onComplete (trailers) {
-        t.equal(trailers, null)
+        t.same(trailers, [])
         t.equal('hello', Buffer.concat(bufs).toString('utf8'))
       },
       onError () {