Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

http: correctly calculate strict content length #46601

Merged
merged 11 commits into from
Feb 23, 2023
Merged

http: correctly calculate strict content length #46601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0268957
http: correctly calculate strict content length
ronag Feb 10, 2023
23d253a
http: speedup strict content length
ronag Feb 10, 2023
1a3683b
http: make sure to pass encoding to Buffer.byteLength
ronag Feb 10, 2023
db11211
http: count writtenBytes even if socket is not ready
ronag Feb 10, 2023
30a9aeb
fixup! http: speedup strict content length
ronag Feb 10, 2023
0729f47
fixup! http: speedup strict content length
ronag Feb 10, 2023
fe6f80a
fixuP
ronag Feb 15, 2023
ab2c162
fixuP
ronag Feb 16, 2023
a1c9fc7
fixup
ronag Feb 16, 2023
e259ee6
fixup: linting
ronag Feb 16, 2023
20c7f2f
fixup: cleanup
ronag Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 38 additions & 44 deletions lib/_http_outgoing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ const {
Array,
ArrayIsArray,
ArrayPrototypeJoin,
MathAbs,
MathFloor,
NumberPrototypeToString,
ObjectDefineProperty,
Expand Down Expand Up @@ -86,7 +85,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 @@ -129,7 +127,6 @@ function OutgoingMessage() {

this.strictContentLength = false;
this[kBytesWritten] = 0;
this[kEndCalled] = false;
this._contentLength = null;
this._hasBody = true;
this._trailer = '';
Expand Down Expand Up @@ -351,7 +348,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 @@ -373,20 +370,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 @@ -399,25 +387,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 @@ -869,18 +838,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 @@ -901,8 +876,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 @@ -922,12 +913,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;
ronag marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -999,8 +991,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 @@ -1035,6 +1025,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
Open in desktop
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