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

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

/ node Public

Commit aac5c28

Browse files
ronag
authored and
targos
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 e08514e commit aac5c28

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
ObjectDefineProperty,
@@ -86,7 +85,6 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();
8685
const kCorked = Symbol('corked');
8786
const kUniqueHeaders = Symbol('kUniqueHeaders');
8887
const kBytesWritten = Symbol('kBytesWritten');
89-
const kEndCalled = Symbol('kEndCalled');
9088
const kErrored = Symbol('errored');
9189

9290
const nop = () => {};
@@ -133,7 +131,6 @@ function OutgoingMessage() {
133131

134132
this.strictContentLength = false;
135133
this[kBytesWritten] = 0;
136-
this[kEndCalled] = false;
137134
this._contentLength = null;
138135
this._hasBody = true;
139136
this._trailer = '';
@@ -355,7 +352,7 @@ OutgoingMessage.prototype.destroy = function destroy(error) {
355352

356353

357354
// This abstract either writing directly to the socket or buffering it.
358-
OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
355+
OutgoingMessage.prototype._send = function _send(data, encoding, callback, byteLength) {
359356
// This is a shameful hack to get the headers and first body chunk onto
360357
// the same packet. Future versions of Node are going to take care of
361358
// this at a lower level and in a more general way.
@@ -377,20 +374,11 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
377374
}
378375
this._headerSent = true;
379376
}
380-
return this._writeRaw(data, encoding, callback);
377+
return this._writeRaw(data, encoding, callback, byteLength);
381378
};
382379

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

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

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

888-
let len;
889868
if (chunk === null) {
890869
throw new ERR_STREAM_NULL_VALUES();
891-
} else if (typeof chunk === 'string') {
892-
len = Buffer.byteLength(chunk, encoding);
893-
} else if (isUint8Array(chunk)) {
894-
len = chunk.length;
895-
} else {
870+
} else if (typeof chunk !== 'string' && !isUint8Array(chunk)) {
896871
throw new ERR_INVALID_ARG_TYPE(
897872
'chunk', ['string', 'Buffer', 'Uint8Array'], chunk);
898873
}
@@ -913,8 +888,24 @@ function write_(msg, chunk, encoding, callback, fromEnd) {
913888
return false;
914889
}
915890

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

935926
let ret;
936927
if (msg.chunkedEncoding && chunk.length !== 0) {
928+
len ??= typeof chunk === 'string' ? Buffer.byteLength(chunk, encoding) : chunk.byteLength;
937929
msg._send(NumberPrototypeToString(len, 16), 'latin1', null);
938930
msg._send(crlf_buf, null, null);
939-
msg._send(chunk, encoding, null);
931+
msg._send(chunk, encoding, null, len);
940932
ret = msg._send(crlf_buf, null, callback);
941933
} else {
942-
ret = msg._send(chunk, encoding, callback);
934+
ret = msg._send(chunk, encoding, callback, len);
943935
}
944936

945937
debug('write ret = ' + ret);
@@ -1011,8 +1003,6 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
10111003
encoding = null;
10121004
}
10131005

1014-
this[kEndCalled] = true;
1015-
10161006
if (chunk) {
10171007
if (this.finished) {
10181008
onError(this,
@@ -1047,6 +1037,10 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
10471037
if (typeof callback === 'function')
10481038
this.once('finish', callback);
10491039

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

10521046
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