From a61f1ff273f996de85b17cd43a119d57543c1093 Mon Sep 17 00:00:00 2001
From: sidwebworks <xodedeveloper@gmail.com>
Date: Wed, 24 Aug 2022 22:19:29 +0530
Subject: [PATCH] http: throw error on content-length mismatch

PR-URL: https://github.com/nodejs/node/pull/44378
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
---
 doc/api/errors.md                             |  6 ++
 doc/api/http.md                               | 15 ++--
 lib/_http_outgoing.js                         | 41 +++++++++-
 lib/internal/errors.js                        |  2 +
 .../test-http-content-length-mismatch.js      | 80 +++++++++++++++++++
 .../parallel/test-http-outgoing-properties.js |  2 +-
 ...test-http-response-multi-content-length.js |  2 +-
 7 files changed, 140 insertions(+), 8 deletions(-)
 create mode 100644 test/parallel/test-http-content-length-mismatch.js

diff --git a/doc/api/errors.md b/doc/api/errors.md
index 4c31a19fed3338..a4f6902be32d63 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -1306,6 +1306,12 @@ When using [`fs.cp()`][], `src` or `dest` pointed to an invalid path.
 
 <a id="ERR_FS_CP_FIFO_PIPE"></a>
 
+### `ERR_HTTP_CONTENT_LENGTH_MISMATCH`
+
+Response body size doesn't match with the specified content-length header value.
+
+<a id="ERR_HTTP_CONTENT_LENGTH_MISMATCH"></a>
+
 ### `ERR_FS_CP_FIFO_PIPE`
 
 <!--
diff --git a/doc/api/http.md b/doc/api/http.md
index cb7b91a72f00e2..14710ee932faed 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -419,8 +419,12 @@ the data is read it will consume memory that can eventually lead to a
 For backward compatibility, `res` will only emit `'error'` if there is an
 `'error'` listener registered.
 
-Node.js does not check whether Content-Length and the length of the
-body which has been transmitted are equal or not.
+Set `Content-Length` header to limit the response body size. Mismatching the
+`Content-Length` header value will result in an \[`Error`]\[] being thrown,
+identified by `code:` [`'ERR_HTTP_CONTENT_LENGTH_MISMATCH'`][].
+
+`Content-Length` value should be in bytes, not characters. Use
+[`Buffer.byteLength()`][] to determine the length of the body in bytes.
 
 ### Event: `'abort'`
 
@@ -2169,13 +2173,13 @@ const server = http.createServer((req, res) => {
 });
 ```
 
-`Content-Length` is given in bytes, not characters. Use
+`Content-Length` is read in bytes, not characters. Use
 [`Buffer.byteLength()`][] to determine the length of the body in bytes. Node.js
-does not check whether `Content-Length` and the length of the body which has
+will check whether `Content-Length` and the length of the body which has
 been transmitted are equal or not.
 
 Attempting to set a header field name or value that contains invalid characters
-will result in a [`TypeError`][] being thrown.
+will result in a \[`Error`]\[] being thrown.
 
 ### `response.writeProcessing()`
 
@@ -3568,6 +3572,7 @@ added: REPLACEME
 Set the maximum number of idle HTTP parsers. **Default:** `1000`.
 
 [RFC 8187]: https://www.rfc-editor.org/rfc/rfc8187.txt
+[`'ERR_HTTP_CONTENT_LENGTH_MISMATCH'`]: errors.md#err_http_content_length_mismatch
 [`'checkContinue'`]: #event-checkcontinue
 [`'finish'`]: #event-finish
 [`'request'`]: #event-request
diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 7b1961ad978580..a5b3863abaeebb 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -25,6 +25,7 @@ const {
   Array,
   ArrayIsArray,
   ArrayPrototypeJoin,
+  MathAbs,
   MathFloor,
   NumberPrototypeToString,
   ObjectCreate,
@@ -57,6 +58,7 @@ const {
 } = require('internal/async_hooks');
 const {
   codes: {
+    ERR_HTTP_CONTENT_LENGTH_MISMATCH,
     ERR_HTTP_HEADERS_SENT,
     ERR_HTTP_INVALID_HEADER_VALUE,
     ERR_HTTP_TRAILER_INVALID,
@@ -84,6 +86,8 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
 
 const kCorked = Symbol('corked');
 const kUniqueHeaders = Symbol('kUniqueHeaders');
+const kBytesWritten = Symbol('kBytesWritten');
+const kEndCalled = Symbol('kEndCalled');
 
 const nop = () => {};
 
@@ -123,6 +127,9 @@ function OutgoingMessage() {
   this._removedContLen = false;
   this._removedTE = false;
 
+  this.strictContentLength = false;
+  this[kBytesWritten] = 0;
+  this[kEndCalled] = false;
   this._contentLength = null;
   this._hasBody = true;
   this._trailer = '';
@@ -330,7 +337,9 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
   // This is a shameful hack to get the headers and first body chunk onto
   // the same packet. Future versions of Node are going to take care of
   // this at a lower level and in a more general way.
-  if (!this._headerSent) {
+  if (!this._headerSent && this._header !== null) {
+    // `this._header` can be null if OutgoingMessage is used without a proper Socket
+    // See: /test/parallel/test-http-outgoing-message-inheritance.js
     if (typeof data === 'string' &&
         (encoding === 'utf8' || encoding === 'latin1' || !encoding)) {
       data = this._header + data;
@@ -349,6 +358,14 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
   return this._writeRaw(data, encoding, callback);
 };
 
+function _getMessageBodySize(chunk, headers, encoding) {
+  if (Buffer.isBuffer(chunk)) return chunk.length;
+  const chunkLength = chunk ? Buffer.byteLength(chunk, encoding) : 0;
+  const headerLength = headers ? headers.length : 0;
+  if (headerLength === chunkLength) return 0;
+  if (headerLength < chunkLength) return MathAbs(chunkLength - headerLength);
+  return chunkLength;
+}
 
 OutgoingMessage.prototype._writeRaw = _writeRaw;
 function _writeRaw(data, encoding, callback) {
@@ -364,6 +381,25 @@ function _writeRaw(data, encoding, callback) {
     encoding = null;
   }
 
+  // TODO(sidwebworks): flip the `strictContentLength` default to `true` in a future PR
+  if (this.strictContentLength && conn && conn.writable && !this._removedContLen && this._hasBody) {
+    const skip = conn._httpMessage.statusCode === 304 || (this.hasHeader('transfer-encoding') || this.chunkedEncoding);
+
+    if (typeof this._contentLength === 'number' && !skip) {
+      const size = _getMessageBodySize(data, conn._httpMessage._header, encoding);
+
+      if ((size + this[kBytesWritten]) > this._contentLength) {
+        throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(size + this[kBytesWritten], this._contentLength);
+      }
+
+      if (this[kEndCalled] && (size + this[kBytesWritten]) !== this._contentLength) {
+        throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(size + this[kBytesWritten], this._contentLength);
+      }
+
+      this[kBytesWritten] += size;
+    }
+  }
+
   if (conn && conn._httpMessage === this && conn.writable) {
     // There might be pending data in the this.output buffer.
     if (this.outputData.length) {
@@ -559,6 +595,7 @@ function matchHeader(self, state, field, value) {
       break;
     case 'content-length':
       state.contLen = true;
+      self._contentLength = value;
       self._removedContLen = false;
       break;
     case 'date':
@@ -923,6 +960,8 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
     encoding = null;
   }
 
+  this[kEndCalled] = true;
+
   if (chunk) {
     if (this.finished) {
       onError(this,
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 1ad2a640b4c7b6..a75e7ccd056889 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1134,6 +1134,8 @@ E('ERR_HTTP2_TRAILERS_NOT_READY',
   'Trailing headers cannot be sent until after the wantTrailers event is ' +
   'emitted', Error);
 E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', 'protocol "%s" is unsupported.', Error);
+E('ERR_HTTP_CONTENT_LENGTH_MISMATCH',
+  'Response body\'s content-length of %s byte(s) does not match the content-length of %s byte(s) set in header', Error);
 E('ERR_HTTP_HEADERS_SENT',
   'Cannot %s headers after they are sent to the client', Error);
 E('ERR_HTTP_INVALID_HEADER_VALUE',
diff --git a/test/parallel/test-http-content-length-mismatch.js b/test/parallel/test-http-content-length-mismatch.js
new file mode 100644
index 00000000000000..fc294db16407bc
--- /dev/null
+++ b/test/parallel/test-http-content-length-mismatch.js
@@ -0,0 +1,80 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+function shouldThrowOnMoreBytes() {
+  const server = http.createServer(common.mustCall((req, res) => {
+    res.strictContentLength = true;
+    res.setHeader('Content-Length', 5);
+    res.write('hello');
+    assert.throws(() => {
+      res.write('a');
+    }, {
+      code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
+    });
+    res.statusCode = 200;
+    res.end();
+  }));
+
+  server.listen(0, () => {
+    const req = http.get({
+      port: server.address().port,
+    }, common.mustCall((res) => {
+      res.resume();
+      assert.strictEqual(res.statusCode, 200);
+      server.close();
+    }));
+    req.end();
+  });
+}
+
+function shouldNotThrow() {
+  const server = http.createServer(common.mustCall((req, res) => {
+    res.strictContentLength = true;
+    res.write('helloaa');
+    res.statusCode = 200;
+    res.end('ending');
+  }));
+
+  server.listen(0, () => {
+    http.get({
+      port: server.address().port,
+    }, common.mustCall((res) => {
+      res.resume();
+      assert.strictEqual(res.statusCode, 200);
+      server.close();
+    }));
+  });
+}
+
+
+function shouldThrowOnFewerBytes() {
+  const server = http.createServer(common.mustCall((req, res) => {
+    res.strictContentLength = true;
+    res.setHeader('Content-Length', 5);
+    res.write('a');
+    res.statusCode = 200;
+    assert.throws(() => {
+      res.end();
+    }, {
+      code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
+    });
+    res.end('aaaa');
+  }));
+
+  server.listen(0, () => {
+    http.get({
+      port: server.address().port,
+    }, common.mustCall((res) => {
+      res.resume();
+      assert.strictEqual(res.statusCode, 200);
+      server.close();
+    }));
+  });
+}
+
+shouldThrowOnMoreBytes();
+shouldNotThrow();
+shouldThrowOnFewerBytes();
diff --git a/test/parallel/test-http-outgoing-properties.js b/test/parallel/test-http-outgoing-properties.js
index ec25546c4b66ec..85c5b659a36d64 100644
--- a/test/parallel/test-http-outgoing-properties.js
+++ b/test/parallel/test-http-outgoing-properties.js
@@ -49,7 +49,7 @@ const OutgoingMessage = http.OutgoingMessage;
   msg._implicitHeader = function() {};
   assert.strictEqual(msg.writableLength, 0);
   msg.write('asd');
-  assert.strictEqual(msg.writableLength, 7);
+  assert.strictEqual(msg.writableLength, 3);
 }
 
 {
diff --git a/test/parallel/test-http-response-multi-content-length.js b/test/parallel/test-http-response-multi-content-length.js
index 0ef45a027fb64d..3ae53ffb7ecbfd 100644
--- a/test/parallel/test-http-response-multi-content-length.js
+++ b/test/parallel/test-http-response-multi-content-length.js
@@ -24,7 +24,7 @@ function test(server) {
 {
   const server = http.createServer((req, res) => {
     res.setHeader('content-length', [2, 1]);
-    res.end('ok');
+    res.end('k');
   });
 
   test(server);