perf: allow keep alive for HEAD requests

nodejs · Jan 16, 2023 · 3342cf5 · 3342cf5
1 parent d5af7dd
commit 3342cf5
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 15 deletions.
diff --git a/lib/client.js b/lib/client.js
@@ -439,8 +439,9 @@ class Parser {
 
     this.bytesRead = 0
 
-    this.keepAlive = ''
-    this.contentLength = ''
+    this.keepAliveHeader = ''
+    this.contentLengthHeader = ''
+    this.connectionHeader = ''
     this.maxResponseSize = client[kMaxResponseSize]
   }
 
@@ -615,9 +616,11 @@ class Parser {
 
     const key = this.headers[len - 2]
     if (key.length === 10 && key.toString().toLowerCase() === 'keep-alive') {
-      this.keepAlive += buf.toString()
+      this.keepAliveHeader += buf.toString()
+    } else if (key.length === 10 && key.toString().toLowerCase() === 'connection') {
+      this.connectionHeader += buf.toString()
     } else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') {
-      this.contentLength += buf.toString()
+      this.contentLengthHeader += buf.toString()
     }
 
     this.trackHeader(buf.length)
@@ -645,7 +648,7 @@ class Parser {
 
     this.statusCode = null
     this.statusText = ''
-    this.shouldKeepAlive = null
+    this.shouldKeepAlive = false
 
     assert(this.headers.length % 2 === 0)
     this.headers = []
@@ -709,7 +712,11 @@ class Parser {
     assert.strictEqual(this.timeoutType, TIMEOUT_HEADERS)
 
     this.statusCode = statusCode
-    this.shouldKeepAlive = shouldKeepAlive
+    this.shouldKeepAlive = (
+      shouldKeepAlive ||
+      // Override llhttp value which does not allow keepAlive for HEAD.
+      (request.reset === false && this.connectionHeader.toLowerCase() === 'keep-alive')
+    )
 
     if (this.statusCode >= 200) {
       const bodyTimeout = request.bodyTimeout != null
@@ -739,8 +746,8 @@ class Parser {
     this.headers = []
     this.headersSize = 0
 
-    if (shouldKeepAlive && client[kPipelining]) {
-      const keepAliveTimeout = this.keepAlive ? util.parseKeepAliveTimeout(this.keepAlive) : null
+    if (this.shouldKeepAlive && client[kPipelining]) {
+      const keepAliveTimeout = this.keepAliveHeader ? util.parseKeepAliveTimeout(this.keepAliveHeader) : null
 
       if (keepAliveTimeout != null) {
         const timeout = Math.min(
@@ -769,7 +776,7 @@ class Parser {
     }
 
     if (request.method === 'HEAD') {
-      assert(socket[kReset])
+      assert(request.reset === false || socket[kReset])
       return 1
     }
 
@@ -823,7 +830,7 @@ class Parser {
   }
 
   onMessageComplete () {
-    const { client, socket, statusCode, upgrade, headers, contentLength, bytesRead, shouldKeepAlive } = this
+    const { client, socket, statusCode, upgrade, headers, contentLengthHeader, bytesRead, shouldKeepAlive } = this
 
     if (socket.destroyed && (!statusCode || shouldKeepAlive)) {
       return -1
@@ -841,8 +848,9 @@ class Parser {
     this.statusCode = null
     this.statusText = ''
     this.bytesRead = 0
-    this.contentLength = ''
-    this.keepAlive = ''
+    this.contentLengthHeader = ''
+    this.keepAliveHeader = ''
+    this.connectionHeader = ''
 
     assert(this.headers.length % 2 === 0)
     this.headers = []
@@ -853,7 +861,7 @@ class Parser {
     }
 
     /* istanbul ignore next: should be handled by llhttp? */
-    if (request.method !== 'HEAD' && contentLength && bytesRead !== parseInt(contentLength, 10)) {
+    if (request.method !== 'HEAD' && contentLengthHeader && bytesRead !== parseInt(contentLengthHeader, 10)) {
       util.destroy(socket, new ResponseContentLengthMismatchError())
       return -1
     }
@@ -1376,8 +1384,8 @@ function write (client, request) {
     socket[kReset] = true
   }
 
-  if (reset) {
-    socket[kReset] = true
+  if (reset != null) {
+    socket[kReset] = reset
   }
 
   if (client[kMaxRequests] && socket[kCounter]++ >= client[kMaxRequests]) {

diff --git a/test/client-head-reset-override.js b/test/client-head-reset-override.js
@@ -0,0 +1,62 @@
+'use strict'
+
+const { createServer } = require('http')
+const { test } = require('tap')
+const { Client } = require('..')
+
+test('override HEAD reset', (t) => {
+  const expected = 'testing123'
+  const server = createServer((req, res) => {
+    if (req.method === 'GET') {
+      res.write(expected)
+    }
+    res.end()
+  })
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    t.teardown(client.close.bind(client))
+
+    let done
+    client.on('disconnect', () => {
+      if (!done) {
+        t.fail()
+      }
+    })
+
+    client.request({
+      path: '/',
+      method: 'HEAD',
+      reset: false
+    }, (err, res) => {
+      t.error(err)
+      res.body.resume()
+    })
+
+    client.request({
+      path: '/',
+      method: 'HEAD',
+      reset: false
+    }, (err, res) => {
+      t.error(err)
+      res.body.resume()
+    })
+
+    client.request({
+      path: '/',
+      method: 'GET',
+      reset: false
+    }, (err, res) => {
+      t.error(err)
+      let str = ''
+      res.body.on('data', (data) => {
+        str += data
+      }).on('end', () => {
+        t.same(str, expected)
+        done = true
+        t.end()
+      })
+    })
+  })
+})