Skip to content

Commit

Permalink
http: throw error on content-length mismatch
Browse files Browse the repository at this point in the history
PR-URL: #44378
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
  • Loading branch information
sidwebworks authored and juanarbol committed Oct 4, 2022
1 parent d94c416 commit a61f1ff
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 8 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,12 @@ When using [`fs.cp()`][], `src` or `dest` pointed to an invalid path.

<a id="ERR_FS_CP_FIFO_PIPE"></a>

### `ERR_HTTP_CONTENT_LENGTH_MISMATCH`

Response body size doesn't match with the specified content-length header value.

<a id="ERR_HTTP_CONTENT_LENGTH_MISMATCH"></a>

### `ERR_FS_CP_FIFO_PIPE`

<!--
Expand Down
15 changes: 10 additions & 5 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,12 @@ the data is read it will consume memory that can eventually lead to a
For backward compatibility, `res` will only emit `'error'` if there is an
`'error'` listener registered.

Node.js does not check whether Content-Length and the length of the
body which has been transmitted are equal or not.
Set `Content-Length` header to limit the response body size. Mismatching the
`Content-Length` header value will result in an \[`Error`]\[] being thrown,
identified by `code:` [`'ERR_HTTP_CONTENT_LENGTH_MISMATCH'`][].

`Content-Length` value should be in bytes, not characters. Use
[`Buffer.byteLength()`][] to determine the length of the body in bytes.

### Event: `'abort'`

Expand Down Expand Up @@ -2169,13 +2173,13 @@ const server = http.createServer((req, res) => {
});
```

`Content-Length` is given in bytes, not characters. Use
`Content-Length` is read in bytes, not characters. Use
[`Buffer.byteLength()`][] to determine the length of the body in bytes. Node.js
does not check whether `Content-Length` and the length of the body which has
will check whether `Content-Length` and the length of the body which has
been transmitted are equal or not.

Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.
will result in a \[`Error`]\[] being thrown.

### `response.writeProcessing()`

Expand Down Expand Up @@ -3568,6 +3572,7 @@ added: REPLACEME
Set the maximum number of idle HTTP parsers. **Default:** `1000`.

[RFC 8187]: https://www.rfc-editor.org/rfc/rfc8187.txt
[`'ERR_HTTP_CONTENT_LENGTH_MISMATCH'`]: errors.md#err_http_content_length_mismatch
[`'checkContinue'`]: #event-checkcontinue
[`'finish'`]: #event-finish
[`'request'`]: #event-request
Expand Down
41 changes: 40 additions & 1 deletion lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
Array,
ArrayIsArray,
ArrayPrototypeJoin,
MathAbs,
MathFloor,
NumberPrototypeToString,
ObjectCreate,
Expand Down Expand Up @@ -57,6 +58,7 @@ const {
} = require('internal/async_hooks');
const {
codes: {
ERR_HTTP_CONTENT_LENGTH_MISMATCH,
ERR_HTTP_HEADERS_SENT,
ERR_HTTP_INVALID_HEADER_VALUE,
ERR_HTTP_TRAILER_INVALID,
Expand Down Expand Up @@ -84,6 +86,8 @@ const HIGH_WATER_MARK = getDefaultHighWaterMark();

const kCorked = Symbol('corked');
const kUniqueHeaders = Symbol('kUniqueHeaders');
const kBytesWritten = Symbol('kBytesWritten');
const kEndCalled = Symbol('kEndCalled');

const nop = () => {};

Expand Down Expand Up @@ -123,6 +127,9 @@ function OutgoingMessage() {
this._removedContLen = false;
this._removedTE = false;

this.strictContentLength = false;
this[kBytesWritten] = 0;
this[kEndCalled] = false;
this._contentLength = null;
this._hasBody = true;
this._trailer = '';
Expand Down Expand Up @@ -330,7 +337,9 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
// 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.
if (!this._headerSent) {
if (!this._headerSent && this._header !== null) {
// `this._header` can be null if OutgoingMessage is used without a proper Socket
// See: /test/parallel/test-http-outgoing-message-inheritance.js
if (typeof data === 'string' &&
(encoding === 'utf8' || encoding === 'latin1' || !encoding)) {
data = this._header + data;
Expand All @@ -349,6 +358,14 @@ OutgoingMessage.prototype._send = function _send(data, encoding, callback) {
return this._writeRaw(data, encoding, callback);
};

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) {
Expand All @@ -364,6 +381,25 @@ 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 @@ -559,6 +595,7 @@ function matchHeader(self, state, field, value) {
break;
case 'content-length':
state.contLen = true;
self._contentLength = value;
self._removedContLen = false;
break;
case 'date':
Expand Down Expand Up @@ -923,6 +960,8 @@ OutgoingMessage.prototype.end = function end(chunk, encoding, callback) {
encoding = null;
}

this[kEndCalled] = true;

if (chunk) {
if (this.finished) {
onError(this,
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,8 @@ E('ERR_HTTP2_TRAILERS_NOT_READY',
'Trailing headers cannot be sent until after the wantTrailers event is ' +
'emitted', Error);
E('ERR_HTTP2_UNSUPPORTED_PROTOCOL', 'protocol "%s" is unsupported.', Error);
E('ERR_HTTP_CONTENT_LENGTH_MISMATCH',
'Response body\'s content-length of %s byte(s) does not match the content-length of %s byte(s) set in header', Error);
E('ERR_HTTP_HEADERS_SENT',
'Cannot %s headers after they are sent to the client', Error);
E('ERR_HTTP_INVALID_HEADER_VALUE',
Expand Down
80 changes: 80 additions & 0 deletions test/parallel/test-http-content-length-mismatch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const http = require('http');

function shouldThrowOnMoreBytes() {
const server = http.createServer(common.mustCall((req, res) => {
res.strictContentLength = true;
res.setHeader('Content-Length', 5);
res.write('hello');
assert.throws(() => {
res.write('a');
}, {
code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
});
res.statusCode = 200;
res.end();
}));

server.listen(0, () => {
const req = http.get({
port: server.address().port,
}, common.mustCall((res) => {
res.resume();
assert.strictEqual(res.statusCode, 200);
server.close();
}));
req.end();
});
}

function shouldNotThrow() {
const server = http.createServer(common.mustCall((req, res) => {
res.strictContentLength = true;
res.write('helloaa');
res.statusCode = 200;
res.end('ending');
}));

server.listen(0, () => {
http.get({
port: server.address().port,
}, common.mustCall((res) => {
res.resume();
assert.strictEqual(res.statusCode, 200);
server.close();
}));
});
}


function shouldThrowOnFewerBytes() {
const server = http.createServer(common.mustCall((req, res) => {
res.strictContentLength = true;
res.setHeader('Content-Length', 5);
res.write('a');
res.statusCode = 200;
assert.throws(() => {
res.end();
}, {
code: 'ERR_HTTP_CONTENT_LENGTH_MISMATCH'
});
res.end('aaaa');
}));

server.listen(0, () => {
http.get({
port: server.address().port,
}, common.mustCall((res) => {
res.resume();
assert.strictEqual(res.statusCode, 200);
server.close();
}));
});
}

shouldThrowOnMoreBytes();
shouldNotThrow();
shouldThrowOnFewerBytes();
2 changes: 1 addition & 1 deletion test/parallel/test-http-outgoing-properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const OutgoingMessage = http.OutgoingMessage;
msg._implicitHeader = function() {};
assert.strictEqual(msg.writableLength, 0);
msg.write('asd');
assert.strictEqual(msg.writableLength, 7);
assert.strictEqual(msg.writableLength, 3);
}

{
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-response-multi-content-length.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function test(server) {
{
const server = http.createServer((req, res) => {
res.setHeader('content-length', [2, 1]);
res.end('ok');
res.end('k');
});

test(server);
Expand Down

0 comments on commit a61f1ff

Please sign in to comment.