Skip to content

Commit

Permalink
http: correctly calculate strict content length
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@ronag @danielleadams
ronag authored and danielleadams committed Apr 5, 2023
1 parent 9582c8e commit 3538521
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 45 deletions.
82 changes: 38 additions & 44 deletions lib/_http_outgoing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const {
Array,
ArrayIsArray,
ArrayPrototypeJoin,
MathAbs,
MathFloor,
NumberPrototypeToString,
ObjectCreate,
Expand Down Expand Up @@ -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 = () => {};
Expand Down Expand Up @@ -134,7 +132,6 @@ function OutgoingMessage() {

this.strictContentLength = false;
this[kBytesWritten] = 0;
this[kEndCalled] = false;
this._contentLength = null;
this._hasBody = true;
this._trailer = '';
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -1012,8 +1004,6 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
encoding = null;
}

this[kEndCalled] = true;

if (chunk) {
if (this.finished) {
onError(this,
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-content-length-mismatch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
});
Expand Down

0 comments on commit 3538521

Please sign in to comment.