FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http: correctly calculate strict content length · nodejs/node@3538521 · GitHub

/ node Public

Commit 3538521

Browse files
ronag
authored and
danielleadams
committed
http: correctly calculate strict content length
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>
1 parent 9582c8e commit 3538521

2 files changed

Lines changed: 39 additions & 45 deletions

File tree

‎lib/_http_outgoing.js‎

Lines changed: 38 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const {
2525
Array,
2626
ArrayIsArray,
2727
ArrayPrototypeJoin,
28-
MathAbs,
2928
MathFloor,
3029
NumberPrototypeToString,
3130
ObjectCreate,
@@ -87,7 +86,6 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
8786
const kCorked = Symbol('corked');
8887
const kUniqueHeaders = Symbol('kUniqueHeaders');
8988
const kBytesWritten = Symbol('kBytesWritten');
90-
const kEndCalled = Symbol('kEndCalled');
9189
const kErrored = Symbol('errored');
9290

9391
const nop = () => {};
@@ -134,7 +132,6 @@ function OutgoingMessage() {
134132

135133
this.strictContentLength = false;
136134
this[kBytesWritten] = 0;
137-
this[kEndCalled] = false;
138135
this._contentLength = null;
139136
this._hasBody = true;
140137
this._trailer = '';
@@ -356,7 +353,7 @@ OutgoingMessage.prototype.destroy = function destroy(error) {
356353

357354

358355
// This abstract either writing directly to the socket or buffering it.
359-
OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
356+
OutgoingMessage.prototype._send = function _send(data, encoding, callback, byteLength) {
360357
// This is a shameful hack to get the headers and first body chunk onto
361358
// the same packet. Future versions of Node are going to take care of
362359
// this at a lower level and in a more general way.
@@ -378,20 +375,11 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
378375
}
379376
this._headerSent = true;
380377
}
381-
return this._writeRaw(data, encoding, callback);
378+
return this._writeRaw(data, encoding, callback, byteLength);
382379
};
383380

384-
function _getMessageBodySize(chunk, headers, encoding) {
385-
if (Buffer.isBuffer(chunk)) return chunk.length;
386-
const chunkLength = chunk ? Buffer.byteLength(chunk, encoding) : 0;
387-
const headerLength = headers ? headers.length : 0;
388-
if (headerLength === chunkLength) return 0;
389-
if (headerLength < chunkLength) return MathAbs(chunkLength - headerLength);
390-
return chunkLength;
391-
}
392-
393381
OutgoingMessage.prototype._writeRaw = _writeRaw;
394-
function _writeRaw(data, encoding, callback) {
382+
function _writeRaw(data, encoding, callback, size) {
395383
const conn = this.socket;
396384
if (conn && conn.destroyed) {
397385
// The socket was destroyed. If we're still trying to write to it,
@@ -404,25 +392,6 @@ function _writeRaw(data, encoding, callback) {
404392
encoding = null;
405393
}
406394

407-
// TODO(sidwebworks): flip the `strictContentLength` default to `true` in a future PR
408-
if (this.strictContentLength && conn && conn.writable && !this._removedContLen && this._hasBody) {
409-
const skip = conn._httpMessage.statusCode === 304 || (this.hasHeader('transfer-encoding') || this.chunkedEncoding);
410-
411-
if (typeof this._contentLength === 'number' && !skip) {
412-
const size = _getMessageBodySize(data, conn._httpMessage._header, encoding);
413-
414-
if ((size + this[kBytesWritten]) > this._contentLength) {
415-
throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(size + this[kBytesWritten], this._contentLength);
416-
}
417-
418-
if (this[kEndCalled] && (size + this[kBytesWritten]) !== this._contentLength) {
419-
throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(size + this[kBytesWritten], this._contentLength);
420-
}
421-
422-
this[kBytesWritten] += size;
423-
}
424-
}
425-
426395
if (conn && conn._httpMessage === this && conn.writable) {
427396
// There might be pending data in the this.output buffer.
428397
if (this.outputData.length) {
@@ -882,18 +851,24 @@ function emitErrorNt(msg, err, callback) {
882851
}
883852
}
884853

854+
function strictContentLength(msg) {
855+
return (
856+
msg.strictContentLength &&
857+
msg._contentLength != null &&
858+
msg._hasBody &&
859+
!msg._removedContLen &&
860+
!msg.chunkedEncoding &&
861+
!msg.hasHeader('transfer-encoding')
862+
);
863+
}
864+
885865
function write_(msg, chunk, encoding, callback, fromEnd) {
886866
if (typeof callback !== 'function')
887867
callback = nop;
888868

889-
let len;
890869
if (chunk === null) {
891870
throw new ERR_STREAM_NULL_VALUES();
892-
} else if (typeof chunk === 'string') {
893-
len = Buffer.byteLength(chunk, encoding);
894-
} else if (isUint8Array(chunk)) {
895-
len = chunk.length;
896-
} else {
871+
} else if (typeof chunk !== 'string' && !isUint8Array(chunk)) {
897872
throw new ERR_INVALID_ARG_TYPE(
898873
'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
899874
}
@@ -914,8 +889,24 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
914889
return false;
915890
}
916891

892+
let len;
893+
894+
if (msg.strictContentLength) {
895+
len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
896+
897+
if (
898+
strictContentLength(msg) &&
899+
(fromEnd ? msg[kBytesWritten] + len !== msg._contentLength : msg[kBytesWritten] + len > msg._contentLength)
900+
) {
901+
throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(len + msg[kBytesWritten], msg._contentLength);
902+
}
903+
904+
msg[kBytesWritten] += len;
905+
}
906+
917907
if (!msg._header) {
918908
if (fromEnd) {
909+
len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
919910
msg._contentLength = len;
920911
}
921912
msg._implicitHeader();
@@ -935,12 +926,13 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
935926

936927
let ret;
937928
if (msg.chunkedEncoding && chunk.length !== 0) {
929+
len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
938930
msg._send(NumberPrototypeToString(len, 16), 'latin1', null);
939931
msg._send(crlf_buf, null, null);
940-
msg._send(chunk, encoding, null);
932+
msg._send(chunk, encoding, null, len);
941933
ret = msg._send(crlf_buf, null, callback);
942934
} else {
943-
ret = msg._send(chunk, encoding, callback);
935+
ret = msg._send(chunk, encoding, callback, len);
944936
}
945937

946938
debug('write ret = ' + ret);
@@ -1012,8 +1004,6 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
10121004
encoding = null;
10131005
}
10141006

1015-
this[kEndCalled] = true;
1016-
10171007
if (chunk) {
10181008
if (this.finished) {
10191009
onError(this,
@@ -1048,6 +1038,10 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
10481038
if (typeof callback === 'function')
10491039
this.once('finish', callback);
10501040

1041+
if (strictContentLength(this) && this[kBytesWritten] !== this._contentLength) {
1042+
throw new ERR_HTTP_CONTENT_LENGTH_MISMATCH(this[kBytesWritten], this._contentLength);
1043+
}
1044+
10511045
const finish = onFinish.bind(undefined, this);
10521046

10531047
if (this._hasBody && this.chunkedEncoding) {

‎test/parallel/test-http-content-length-mismatch.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function shouldThrowOnFewerBytes() {
5757
res.write('a');
5858
res.statusCode = 200;
5959
assert.throws(() => {
60-
res.end();
60+
res.end('aaa');
6161
}, {
6262
code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
6363
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL