fix: premature end large body (#805)

* fix: premature end large body * fixup * fixup * fixup * fixuP * fixup * fixup * FIXUP * fixup * fixup * fixup * fixup
nodejs · May 12, 2021 · cd78eb5 · cd78eb5
1 parent f58912b
commit cd78eb5
Show file tree

Hide file tree

Showing 12 changed files with 320 additions and 245 deletions.
diff --git a/deps/llhttp/src/llhttp.c b/deps/llhttp/src/llhttp.c
diff --git a/docs/api/Errors.md b/docs/api/Errors.md
@@ -7,15 +7,16 @@ You can find all the error objects inside the `errors` key.
 const { errors } = require('undici')
 ```
 
-| Error                               | Error Codes                           | Description                                       |
-| ------------------------------------|---------------------------------------|---------------------------------------------------|
-| `InvalidArgumentError`              |  `UND_ERR_INVALID_ARG`                | passed an invalid argument.                       |
-| `InvalidReturnValueError`           |  `UND_ERR_INVALID_RETURN_VALUE`       | returned an invalid value.                        |
-| `RequestAbortedError`               |  `UND_ERR_ABORTED`                    | the request has been aborted by the user          |
-| `ClientDestroyedError`              |  `UND_ERR_DESTROYED`                  | trying to use a destroyed client.                 |
-| `ClientClosedError`                 |  `UND_ERR_CLOSED`                     | trying to use a closed client.                    |
-| `SocketError`                       |  `UND_ERR_SOCKET`                     | there is an error with the socket.                |
-| `NotSupportedError`                 |  `UND_ERR_NOT_SUPPORTED`              | encountered unsupported functionality.            |
-| `RequestContentLengthMismatchError` |  `UND_ERR_REQ_CONTENT_LENGTH_MISMATCH`| request body does not match content-length header |
-| `InformationalError`                |  `UND_ERR_INFO`                       | expected error with reason                        |
-| `TrailerMismatchError`              |  `UND_ERR_TRAILER_MISMATCH`           | trailers did not match specification              |
+| Error                               | Error Codes                           | Description                                        |
+| ------------------------------------|---------------------------------------|----------------------------------------------------|
+| `InvalidArgumentError`              |  `UND_ERR_INVALID_ARG`                | passed an invalid argument.                        |
+| `InvalidReturnValueError`           |  `UND_ERR_INVALID_RETURN_VALUE`       | returned an invalid value.                         |
+| `RequestAbortedError`               |  `UND_ERR_ABORTED`                    | the request has been aborted by the user           |
+| `ClientDestroyedError`              |  `UND_ERR_DESTROYED`                  | trying to use a destroyed client.                  |
+| `ClientClosedError`                 |  `UND_ERR_CLOSED`                     | trying to use a closed client.                     |
+| `SocketError`                       |  `UND_ERR_SOCKET`                     | there is an error with the socket.                 |
+| `NotSupportedError`                 |  `UND_ERR_NOT_SUPPORTED`              | encountered unsupported functionality.             |
+| `RequestContentLengthMismatchError` |  `UND_ERR_REQ_CONTENT_LENGTH_MISMATCH`| request body does not match content-length header  |
+| `RequestContentLengthMismatchError` |  `UND_ERR_RES_CONTENT_LENGTH_MISMATCH`| response body does not match content-length header |
+| `InformationalError`                |  `UND_ERR_INFO`                       | expected error with reason                         |
+| `TrailerMismatchError`              |  `UND_ERR_TRAILER_MISMATCH`           | trailers did not match specification               |
diff --git a/lib/client.js b/lib/client.js
@@ -9,6 +9,7 @@ const Request = require('./core/request')
 const Dispatcher = require('./dispatcher')
 const {
   RequestContentLengthMismatchError,
+  ResponseContentLengthMismatchError,
   TrailerMismatchError,
   InvalidArgumentError,
   RequestAbortedError,
@@ -448,8 +449,11 @@ class Parser {
     this.paused = false
     this.resume = this.resume.bind(this)
 
+    this.bytesRead = 0
+
     this.trailer = ''
     this.keepAlive = ''
+    this.contentLength = ''
   }
 
   setTimeout (value, type) {
@@ -590,6 +594,8 @@ class Parser {
       this.keepAlive += buf.toString()
     } else if (key.length === 7 && key.toString().toLowerCase() === 'trailer') {
       this.trailer += buf.toString()
+    } else if (key.length === 14 && key.toString().toLowerCase() === 'content-length') {
+      this.contentLength += buf.toString()
     }
 
     this.trackHeader(buf.length)
@@ -780,6 +786,8 @@ class Parser {
 
     assert(statusCode >= 200)
 
+    this.bytesRead += buf.length
+
     try {
       if (request.onData(buf) === false) {
         return constants.ERROR.PAUSED
@@ -790,7 +798,7 @@ class Parser {
   }
 
   onMessageComplete () {
-    const { client, socket, statusCode, upgrade, trailer, headers, shouldKeepAlive } = this
+    const { client, socket, statusCode, upgrade, trailer, headers, contentLength, bytesRead, shouldKeepAlive } = this
 
     if (socket.destroyed && (!statusCode || shouldKeepAlive)) {
       return -1
@@ -806,6 +814,8 @@ class Parser {
     assert(statusCode >= 100)
 
     this.statusCode = null
+    this.bytesRead = 0
+    this.contentLength = ''
     this.trailer = ''
     this.keepAlive = ''
 
@@ -834,6 +844,12 @@ class Parser {
       }
     }
 
+    /* istanbul ignore next: should be handled by llhttp? */
+    if (request.method !== 'HEAD' && contentLength && bytesRead !== parseInt(contentLength, 10)) {
+      util.destroy(socket, new ResponseContentLengthMismatchError())
+      return -1
+    }
+
     try {
       request.onComplete(headers)
     } catch (err) {

diff --git a/lib/core/errors.js b/lib/core/errors.js
@@ -94,7 +94,17 @@ class RequestContentLengthMismatchError extends UndiciError {
     Error.captureStackTrace(this, RequestContentLengthMismatchError)
     this.name = 'RequestContentLengthMismatchError'
     this.message = message || 'Request body length does not match content-length header'
-    this.code = 'UND_ERR_CONTENT_LENGTH_MISMATCH'
+    this.code = 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'
+  }
+}
+
+class ResponseContentLengthMismatchError extends UndiciError {
+  constructor (message) {
+    super(message)
+    Error.captureStackTrace(this, ResponseContentLengthMismatchError)
+    this.name = 'ResponseContentLengthMismatchError'
+    this.message = message || 'Response body length does not match content-length header'
+    this.code = 'UND_ERR_RES_CONTENT_LENGTH_MISMATCH'
   }
 }
 
@@ -163,5 +173,6 @@ module.exports = {
   ClientClosedError,
   InformationalError,
   SocketError,
-  NotSupportedError
+  NotSupportedError,
+  ResponseContentLengthMismatchError
 }
diff --git a/lib/core/request.js b/lib/core/request.js
@@ -189,7 +189,7 @@ function processHeader (request, key, val) {
     key.length === 14 &&
     key.toLowerCase() === 'content-length'
   ) {
-    request.contentLength = parseInt(val)
+    request.contentLength = parseInt(val, 10)
     if (!Number.isFinite(request.contentLength)) {
       throw new InvalidArgumentError('invalid content-length header')
     }

diff --git a/lib/core/util.js b/lib/core/util.js
@@ -147,7 +147,7 @@ function destroy (stream, err) {
 const KEEPALIVE_TIMEOUT_EXPR = /timeout=(\d+)/
 function parseKeepAliveTimeout (val) {
   const m = val.toString().match(KEEPALIVE_TIMEOUT_EXPR)
-  return m ? parseInt(m[1]) * 1000 : null
+  return m ? parseInt(m[1], 10) * 1000 : null
 }
 
 function parseHeaders (headers, obj = {}) {

diff --git a/lib/llhttp/llhttp.wasm b/lib/llhttp/llhttp.wasm
diff --git a/lib/llhttp/llhttp_simd.wasm b/lib/llhttp/llhttp_simd.wasm
diff --git a/test/errors.js b/test/errors.js
@@ -21,7 +21,7 @@ const scenarios = [
   createScenario(errors.InvalidReturnValueError, 'Invalid Return Value Error', 'InvalidReturnValueError', 'UND_ERR_INVALID_RETURN_VALUE'),
   createScenario(errors.RequestAbortedError, 'Request aborted', 'RequestAbortedError', 'UND_ERR_ABORTED'),
   createScenario(errors.InformationalError, 'Request information', 'InformationalError', 'UND_ERR_INFO'),
-  createScenario(errors.RequestContentLengthMismatchError, 'Request body length does not match content-length header', 'RequestContentLengthMismatchError', 'UND_ERR_CONTENT_LENGTH_MISMATCH'),
+  createScenario(errors.RequestContentLengthMismatchError, 'Request body length does not match content-length header', 'RequestContentLengthMismatchError', 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'),
   createScenario(errors.TrailerMismatchError, 'Trailers does not match trailer header', 'TrailerMismatchError', 'UND_ERR_TRAILER_MISMATCH'),
   createScenario(errors.ClientDestroyedError, 'The client is destroyed', 'ClientDestroyedError', 'UND_ERR_DESTROYED'),
   createScenario(errors.ClientClosedError, 'The client is closed', 'ClientClosedError', 'UND_ERR_CLOSED'),

diff --git a/test/issue-803.js b/test/issue-803.js
@@ -0,0 +1,47 @@
+'use strict'
+
+const { test } = require('tap')
+const { Client } = require('..')
+const { createServer } = require('http')
+const EE = require('events')
+
+test('https://github.com/nodejs/undici/issues/803', (t) => {
+  t.plan(2)
+
+  const SIZE = 5900373096
+
+  const server = createServer(async (req, res) => {
+    res.setHeader('content-length', SIZE)
+    let pos = 0
+    while (pos < SIZE) {
+      const len = Math.min(SIZE - pos, 65536)
+      if (!res.write(Buffer.allocUnsafe(len))) {
+        await EE.once(res, 'drain')
+      }
+      pos += len
+    }
+
+    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))
+
+    client.request({
+      path: '/',
+      method: 'GET'
+    }, (err, data) => {
+      t.error(err)
+
+      let pos = 0
+      data.body.on('data', (buf) => {
+        pos += buf.length
+      })
+      data.body.on('end', () => {
+        t.equal(pos, SIZE)
+      })
+    })
+  })
+})
diff --git a/test/types/errors.test-d.ts b/test/types/errors.test-d.ts
@@ -36,7 +36,7 @@ expectAssignable<'UND_ERR_INFO'>(new errors.InformationalError().code)
 expectAssignable<errors.UndiciError>(new errors.RequestContentLengthMismatchError())
 expectAssignable<errors.RequestContentLengthMismatchError>(new errors.RequestContentLengthMismatchError())
 expectAssignable<'RequestContentLengthMismatchError'>(new errors.RequestContentLengthMismatchError().name)
-expectAssignable<'UND_ERR_CONTENT_LENGTH_MISMATCH'>(new errors.RequestContentLengthMismatchError().code)
+expectAssignable<'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH'>(new errors.RequestContentLengthMismatchError().code)
 
 expectAssignable<errors.UndiciError>(new errors.ClientDestroyedError())
 expectAssignable<errors.ClientDestroyedError>(new errors.ClientDestroyedError())

diff --git a/types/errors.d.ts b/types/errors.d.ts
@@ -48,7 +48,7 @@ declare namespace Errors {
   /** Body does not match content-length header. */
   export class RequestContentLengthMismatchError extends UndiciError {
     name: 'RequestContentLengthMismatchError';
-    code: 'UND_ERR_CONTENT_LENGTH_MISMATCH';
+    code: 'UND_ERR_REQ_CONTENT_LENGTH_MISMATCH';
   }
 
   /** Trying to use a destroyed client. */