fix: validate EOF for chunked h1 responses

lib/dispatcher/client-h1.js

@@ -360,23 +360,62 @@ class Parser {
           this.paused = true
           socket.unshift(data)
         } else {
-          const ptr = llhttp.llhttp_get_error_reason(this.ptr)
-          let message = ''
-          if (ptr) {
-            const len = new Uint8Array(llhttp.memory.buffer, ptr).indexOf(0)
-            message =
-              'Response does not match the HTTP/1.1 protocol (' +
-              Buffer.from(llhttp.memory.buffer, ptr, len).toString() +
-              ')'
-          }
-          throw new HTTPParserError(message, constants.ERROR[ret], data)
+          throw this.createError(ret, data)
         }
       }
     } catch (err) {
       util.destroy(socket, err)
     }
   }
 
+  finish () {
+    assert(currentParser === null)
+    assert(this.ptr != null)
+    assert(!this.paused)
+
+    const { llhttp } = this
+
+    let ret
+
+    try {
+      currentParser = this
+      ret = llhttp.llhttp_finish(this.ptr)
+    } finally {
+      currentParser = null
+    }
+
+    if (ret === constants.ERROR.OK) {
+      return null
+    }
+
+    if (ret === constants.ERROR.PAUSED || ret === constants.ERROR.PAUSED_UPGRADE) {
+      this.paused = true
+      return null
+    }
+
+    return this.createError(ret, EMPTY_BUF)
+  }
+
+  createError (ret, data) {
+    const { llhttp, contentLength, bytesRead } = this
+
+    if (contentLength !== -1 && bytesRead !== contentLength) {
+      return new ResponseContentLengthMismatchError()
+    }
+
+    const ptr = llhttp.llhttp_get_error_reason(this.ptr)
+    let message = ''
+    if (ptr) {
+      const len = new Uint8Array(llhttp.memory.buffer, ptr).indexOf(0)
+      message =
+        'Response does not match the HTTP/1.1 protocol (' +
+        Buffer.from(llhttp.memory.buffer, ptr, len).toString() +
+        ')'
+    }
+
+    return new HTTPParserError(message, constants.ERROR[ret], data)
+  }
+
   destroy () {
     assert(currentParser === null)
     assert(this.ptr != null)
@@ -888,8 +927,11 @@ function onHttpSocketError (err) {
   // On Mac OS, we get an ECONNRESET even if there is a full body to be forwarded
   // to the user.
   if (err.code === 'ECONNRESET' && parser.statusCode && !parser.shouldKeepAlive) {
-    // We treat all incoming data so for as a valid response.
-    parser.onMessageComplete()
+    const parserErr = parser.finish()
+    if (parserErr) {
+      this[kError] = parserErr
+      this[kClient][kOnError](parserErr)
+    }
     return
   }
 
@@ -906,8 +948,10 @@ function onHttpSocketEnd () {
   const parser = this[kParser]
 
   if (parser.statusCode && !parser.shouldKeepAlive) {
-    // We treat all incoming data so far as a valid response.
-    parser.onMessageComplete()
+    const parserErr = parser.finish()
+    if (parserErr) {
+      util.destroy(this, parserErr)
+    }
     return
   }
 
@@ -919,8 +963,7 @@ function onHttpSocketClose () {
 
   if (parser) {
     if (!this[kError] && parser.statusCode && !parser.shouldKeepAlive) {
-      // We treat all incoming data so far as a valid response.
-      parser.onMessageComplete()
+      this[kError] = parser.finish() || this[kError]
     }
 
     this[kParser].destroy()

test/parser-issues.js

@@ -3,7 +3,16 @@
 const { tspl } = require('@matteo.collina/tspl')
 const { test, after } = require('node:test')
 const net = require('node:net')
-const { Client, errors } = require('..')
+const { Client, errors, fetch } = require('..')
+
+const truncatedChunkedResponse = Buffer.from(
+  'HTTP/1.1 200 OK\r\n' +
+  'Transfer-Encoding: chunked\r\n' +
+  'Connection: close\r\n' +
+  '\r\n' +
+  '3\r\n' +
+  'hel\r\n'
+)
 
 test('https://github.com/mcollina/undici/issues/268', async (t) => {
   t = tspl(t, { plan: 2 })
@@ -195,3 +204,57 @@ test('refreshes wasm input view after reallocating parser buffer', async (t) =>
   t.strictEqual(largeResponse.statusCode, 200)
   t.strictEqual(largeResponse.body.toString(), largeBody.toString())
 })
+
+test('truncated chunked responses terminated by EOF error the response body', async (t) => {
+  t = tspl(t, { plan: 3 })
+
+  const server = net.createServer((socket) => {
+    socket.end(truncatedChunkedResponse)
+  })
+  after(() => server.close())
+
+  await new Promise(resolve => server.listen(0, resolve))
+
+  const client = new Client(`http://localhost:${server.address().port}`)
+  after(() => client.destroy())
+
+  client.request({
+    method: 'GET',
+    path: '/'
+  }, (err, { body } = {}) => {
+    t.ifError(err)
+    body
+      .on('end', () => {
+        t.fail('expected the truncated chunked body to fail')
+      })
+      .on('error', (err) => {
+        t.strictEqual(err.name, 'HTTPParserError')
+        t.strictEqual(err.message, 'Response does not match the HTTP/1.1 protocol (Invalid EOF state)')
+      })
+      .resume()
+  })
+
+  await t.completed
+})
+
+test('fetch rejects truncated chunked responses terminated by EOF', async (t) => {
+  t = tspl(t, { plan: 3 })
+
+  const server = net.createServer((socket) => {
+    socket.end(truncatedChunkedResponse)
+  })
+  after(() => server.close())
+
+  await new Promise(resolve => server.listen(0, resolve))
+
+  const res = await fetch(`http://localhost:${server.address().port}`)
+  t.strictEqual(res.status, 200)
+
+  try {
+    await res.text()
+    t.fail('expected fetch to reject the truncated chunked body')
+  } catch (err) {
+    t.strictEqual(err.name, 'TypeError')
+    t.strictEqual(err.cause?.message, 'Response does not match the HTTP/1.1 protocol (Invalid EOF state)')
+  }
+})