http: correctly calculate strict content length

ronag · danielleadams · commit 3538521bf61a · 2023-04-05T12:52:24.000-04:00
Fixes some logical errors related to strict content length. Also, previously Buffer.byteLength (which is slow) was run regardless of whether or not the len was required. PR-URL: #46601 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
@@ -25,7 +25,6 @@ const {
   Array,
   ArrayIsArray,
   ArrayPrototypeJoin,
-  MathAbs,
   MathFloor,
   NumberPrototypeToString,
   ObjectCreate,
@@ -87,7 +86,6 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
 const kCorked = Symbol('corked');
 const kUniqueHeaders = Symbol('kUniqueHeaders');
 const kBytesWritten = Symbol('kBytesWritten');
-const kEndCalled = Symbol('kEndCalled');
 const kErrored = Symbol('errored');
 
 const nop = () => {};
@@ -134,7 +132,6 @@ function OutgoingMessage() {
 
   this.strictContentLength = false;
   this[kBytesWritten] = 0;
-  this[kEndCalled] = false;
   this._contentLength = null;
   this._hasBody = true;
   this._trailer = '';
@@ -356,7 +353,7 @@ OutgoingMessage.prototype.destroy = function destroy(error) {
 
 
 // This abstract either writing directly to the socket or buffering it.
-OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
+OutgoingMessage.prototype._send = function _send(data, encoding, callback, byteLength) {
   // 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.
@@ -378,20 +375,11 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
     }
     this._headerSent = true;
   }
-  return this._writeRaw(data, encoding, callback);
+  return this._writeRaw(data, encoding, callback, byteLength);
 };
 
-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) {
+function _writeRaw(data, encoding, callback, size) {
   const conn = this.socket;
   if (conn && conn.destroyed) {
     // The socket was destroyed. If we're still trying to write to it,
@@ -404,25 +392,6 @@ 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) {
@@ -882,18 +851,24 @@ function emitErrorNt(msg, err, callback) {
   }
 }
 
+function strictContentLength(msg) {
+  return (
+    msg.strictContentLength &&
+    msg._contentLength != null &&
+    msg._hasBody &&
+    !msg._removedContLen &&
+    !msg.chunkedEncoding &&
+    !msg.hasHeader('transfer-encoding')
+  );
+}
+
 function write_(msg, chunk, encoding, callback, fromEnd) {
   if (typeof callback !== 'function')
     callback = nop;
 
-  let len;
   if (chunk === null) {
     throw new ERR_STREAM_NULL_VALUES();
-  } else if (typeof chunk === 'string') {
-    len = Buffer.byteLength(chunk, encoding);
-  } else if (isUint8Array(chunk)) {
-    len = chunk.length;
-  } else {
+  } else if (typeof chunk !== 'string' && !isUint8Array(chunk)) {
     throw new ERR_INVALID_ARG_TYPE(
       'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
   }
@@ -914,8 +889,24 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
     return false;
   }
 
+  let len;
+
+  if (msg.strictContentLength) {
+    len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
+
+    if (
+      strictContentLength(msg) &&
+      (fromEnd ? msg[kBytesWritten] + len !== msg._contentLength : msg[kBytesWritten] + len > msg._contentLength)
+    ) {
+      throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(len + msg[kBytesWritten], msg._contentLength);
+    }
+
+    msg[kBytesWritten] += len;
+  }
+
   if (!msg._header) {
     if (fromEnd) {
+      len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
       msg._contentLength = len;
     }
     msg._implicitHeader();
@@ -935,12 +926,13 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
 
   let ret;
   if (msg.chunkedEncoding && chunk.length !== 0) {
+    len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
     msg._send(NumberPrototypeToString(len, 16), 'latin1', null);
     msg._send(crlf_buf, null, null);
-    msg._send(chunk, encoding, null);
+    msg._send(chunk, encoding, null, len);
     ret = msg._send(crlf_buf, null, callback);
   } else {
-    ret = msg._send(chunk, encoding, callback);
+    ret = msg._send(chunk, encoding, callback, len);
   }
 
   debug('write ret = ' + ret);
@@ -1012,8 +1004,6 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
     encoding = null;
   }
 
-  this[kEndCalled] = true;
-
   if (chunk) {
     if (this.finished) {
       onError(this,
@@ -1048,6 +1038,10 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
   if (typeof callback === 'function')
     this.once('finish', callback);
 
+  if (strictContentLength(this) && this[kBytesWritten] !== this._contentLength) {
+    throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(this[kBytesWritten], this._contentLength);
+  }
+
   const finish = onFinish.bind(undefined, this);
 
   if (this._hasBody && this.chunkedEncoding) {
diff --git a/test/parallel/test-http-content-length-mismatch.js b/test/parallel/test-http-content-length-mismatch.js
@@ -57,7 +57,7 @@ function shouldThrowOnFewerBytes() {
     res.write('a');
     res.statusCode = 200;
     assert.throws(() => {
-      res.end();
+      res.end('aaa');
     }, {
       code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
     });
