improve spec compliance by properly handling responses without content-length or transfer-encoding

evanderkoogh · evanderkoogh · commit ac2fbb96e612 · 2022-07-12T20:29:20.000+10:00
fixes #1414 #1412 #1490
diff --git a/lib/client.js b/lib/client.js
@@ -419,6 +419,7 @@ class Parser {
     this.headersSize = 0
     this.headersMaxSize = client[kMaxHeadersSize]
     this.shouldKeepAlive = false
+    this.hasTransferEncoding = false
     this.paused = false
     this.resume = this.resume.bind(this)
 
@@ -615,6 +616,8 @@ class Parser {
       this.keepAlive += buf.toString()
     } else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') {
       this.contentLength += buf.toString()
+    } else if (key.length === 16 && key.toString().toLowerCase() === 'transfer-encoding') {
+      this.hasTransferEncoding = true
     }
 
     this.trackHeader(buf.length)
@@ -957,10 +960,16 @@ function onSocketEnd () {
 }
 
 function onSocketClose () {
-  const { [kClient]: client } = this
+  let { [kClient]: client, [kParser]: parser } = this
 
-  this[kParser].destroy()
-  this[kParser] = null
+  // without content-length or transfer-encoding header, the body is defined as everything
+  // the server sends before closing the connection. RFC 7230 3.3.3 point 7.
+  if (!parser.contentLength && !parser.hasTransferEncoding) {
+    parser.onMessageComplete()
+  }
+
+  parser.destroy()
+  parser = null
 
   const err = this[kError] || new SocketError('closed', util.getSocketInfo(this))
 
diff --git a/test/client.js b/test/client.js
@@ -1972,6 +1972,43 @@ test('async iterator yield object error', (t) => {
   })
 })
 
+test('Successfully get a Response when neither a Transfer-Encoding or Content-Length header is present', (t) => {
+  t.plan(2)
+  const server = createServer((req, res) => {
+    req.on('data', (data) => {
+    })
+    req.on('end', () => {
+      res.removeHeader('transfer-encoding')
+      res.writeHead(200, {
+        // Header isn't actually necessary, but tells node to close after response
+        connection: 'close',
+        foo: 'bar'
+      })
+      res.end('a response body')
+    })
+  })
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    t.teardown(client.close.bind(client))
+
+    client.request({ path: '/', method: 'GET' }, (err, { body }) => {
+      t.error(err)
+      const bufs = []
+      body.on('error', () => {
+        t.fail('Closing the connection is valid')
+      })
+      body.on('data', (buf) => {
+        bufs.push(buf)
+      })
+      body.on('end', () => {
+        t.equal('a response body', Buffer.concat(bufs).toString('utf8'))
+      })
+    })
+  })
+})
+
 function buildParams (path) {
   const cleanPath = path.replace('/?', '').replace('/', '').split('&')
   const builtParams = cleanPath.reduce((acc, entry) => {
