Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 753f3b2

Browse filesBrowse files
mcollinaShogunPandaronag
authored and
richardlau
committed
http: add requestTimeout
This commits introduces a new http.Server option called requestTimeout with a default value in milliseconds of 0. If requestTimeout is set to a positive value, the server will start a new timer set to expire in requestTimeout milliseconds when a new connection is established. The timer is also set again if new requests after the first are received on the socket (this handles pipelining and keep-alive cases). The timer is cancelled when: 1. the request body is completely received by the server. 2. the response is completed. This handles the case where the application responds to the client without consuming the request body. 3. the connection is upgraded, like in the WebSocket case. If the timer expires, then the server responds with status code 408 and closes the connection. CVE-2020-8251 PR-URL: nodejs-private/node-private#208 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Mary Marchini <oss@mmarchini.me> Co-Authored-By: Paolo Insogna <paolo@cowtech.it> Co-Authored-By: Robert Nagy <ronagy@icloud.com>
1 parent dd82837 commit 753f3b2
Copy full SHA for 753f3b2
Expand file treeCollapse file tree

14 files changed

+517
-8
lines changed
Open diff view settings
Collapse file

‎doc/api/errors.md‎

Copy file name to clipboardExpand all lines: doc/api/errors.md
+5Lines changed: 5 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,11 @@ allowed size for a `Buffer`.
940940
An invalid symlink type was passed to the [`fs.symlink()`][] or
941941
[`fs.symlinkSync()`][] methods.
942942

943+
<a id="ERR_HTTP_REQUEST_TIMEOUT"></a>
944+
### `ERR_HTTP_REQUEST_TIMEOUT`
945+
946+
The client has not sent the entire request within the allowed time.
947+
943948
<a id="ERR_HTTP_HEADERS_SENT"></a>
944949
### `ERR_HTTP_HEADERS_SENT`
945950

Collapse file

‎doc/api/http.md‎

Copy file name to clipboardExpand all lines: doc/api/http.md
+17Lines changed: 17 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -1256,6 +1256,23 @@ added: v0.7.0
12561256

12571257
Limits maximum incoming headers count. If set to 0, no limit will be applied.
12581258

1259+
### `server.requestTimeout`
1260+
<!-- YAML
1261+
added: REPLACEME
1262+
-->
1263+
1264+
* {number} **Default:** `0`
1265+
1266+
Sets the timeout value in milliseconds for receiving the entire request from
1267+
the client.
1268+
1269+
If the timeout expires, the server responds with status 408 without
1270+
forwarding the request to the request listener and then closes the connection.
1271+
1272+
It must be set to a non-zero value (e.g. 120 seconds) to proctect against
1273+
potential Denial-of-Service attacks in case the server is deployed without a
1274+
reverse proxy in front.
1275+
12591276
### `server.setTimeout([msecs][, callback])`
12601277
<!-- YAML
12611278
added: v0.9.12
Collapse file

‎doc/api/https.md‎

Copy file name to clipboardExpand all lines: doc/api/https.md
+10Lines changed: 10 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ This method is identical to [`server.listen()`][] from [`net.Server`][].
113113

114114
See [`http.Server#maxHeadersCount`][].
115115

116+
### `server.requestTimeout`
117+
<!-- YAML
118+
added: REPLACEME
119+
-->
120+
121+
* {number} **Default:** `0`
122+
123+
See [`http.Server#requestTimeout`][].
124+
116125
### `server.setTimeout([msecs][, callback])`
117126
<!-- YAML
118127
added: v0.11.2
@@ -449,6 +458,7 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; p
449458
[`http.Server#headersTimeout`]: http.html#http_server_headerstimeout
450459
[`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
451460
[`http.Server#maxHeadersCount`]: http.html#http_server_maxheaderscount
461+
[`http.Server#requestTimeout`]: http.html#http_server_requesttimeout
452462
[`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback
453463
[`http.Server#timeout`]: http.html#http_server_timeout
454464
[`http.Server`]: http.html#http_class_http_server
Collapse file

‎lib/_http_common.js‎

Copy file name to clipboardExpand all lines: lib/_http_common.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
4444
});
4545

4646
const kIncomingMessage = Symbol('IncomingMessage');
47+
const kRequestTimeout = Symbol('RequestTimeout');
4748
const kOnHeaders = HTTPParser.kOnHeaders | 0;
4849
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
4950
const kOnBody = HTTPParser.kOnBody | 0;
@@ -99,6 +100,12 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
99100
incoming.url = url;
100101
incoming.upgrade = upgrade;
101102

103+
if (socket) {
104+
debug('requestTimeout timer moved to req');
105+
incoming[kRequestTimeout] = incoming.socket[kRequestTimeout];
106+
incoming.socket[kRequestTimeout] = undefined;
107+
}
108+
102109
let n = headers.length;
103110

104111
// If parser.maxHeaderPairs <= 0 assume that there's no limit.
@@ -264,6 +271,7 @@ module.exports = {
264271
methods,
265272
parsers,
266273
kIncomingMessage,
274+
kRequestTimeout,
267275
HTTPParser,
268276
isLenient,
269277
prepareError,
Collapse file

‎lib/_http_server.js‎

Copy file name to clipboardExpand all lines: lib/_http_server.js
+83-2Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const {
4040
continueExpression,
4141
chunkExpression,
4242
kIncomingMessage,
43+
kRequestTimeout,
4344
HTTPParser,
4445
isLenient,
4546
_checkInvalidHeaderChar: checkInvalidHeaderChar,
@@ -57,6 +58,7 @@ const {
5758
} = require('internal/async_hooks');
5859
const { IncomingMessage } = require('_http_incoming');
5960
const {
61+
ERR_HTTP_REQUEST_TIMEOUT,
6062
ERR_HTTP_HEADERS_SENT,
6163
ERR_HTTP_INVALID_STATUS_CODE,
6264
ERR_INVALID_ARG_TYPE,
@@ -72,6 +74,7 @@ const {
7274
DTRACE_HTTP_SERVER_RESPONSE
7375
} = require('internal/dtrace');
7476
const { observerCounts, constants } = internalBinding('performance');
77+
const { setTimeout, clearTimeout } = require('timers');
7578
const { NODE_PERFORMANCE_ENTRY_TYPE_HTTP } = constants;
7679

7780
const kServerResponse = Symbol('ServerResponse');
@@ -143,6 +146,7 @@ const STATUS_CODES = {
143146
511: 'Network Authentication Required' // RFC 6585 6
144147
};
145148

149+
const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
146150
const kOnExecute = HTTPParser.kOnExecute | 0;
147151
const kOnTimeout = HTTPParser.kOnTimeout | 0;
148152

@@ -360,6 +364,7 @@ function Server(options, requestListener) {
360364
this.keepAliveTimeout = 5000;
361365
this.maxHeadersCount = null;
362366
this.headersTimeout = 60 * 1000; // 60 seconds
367+
this.requestTimeout = 0; // 120 seconds
363368
}
364369
ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
365370
ObjectSetPrototypeOf(Server, net.Server);
@@ -481,6 +486,16 @@ function connectionListenerInternal(server, socket) {
481486
parser[kOnTimeout] =
482487
onParserTimeout.bind(undefined, server, socket);
483488

489+
// When receiving new requests on the same socket (pipelining or keep alive)
490+
// make sure the requestTimeout is active.
491+
parser[kOnMessageBegin] =
492+
setRequestTimeout.bind(undefined, server, socket);
493+
494+
// This protects from DOS attack where an attacker establish the connection
495+
// without sending any data on applications where server.timeout is left to
496+
// the default value of zero.
497+
setRequestTimeout(server, socket);
498+
484499
socket._paused = false;
485500
}
486501

@@ -567,6 +582,11 @@ function socketOnData(server, socket, parser, state, d) {
567582
onParserExecuteCommon(server, socket, parser, state, ret, d);
568583
}
569584

585+
function onRequestTimeout(socket) {
586+
socket[kRequestTimeout] = undefined;
587+
socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT());
588+
}
589+
570590
function onParserExecute(server, socket, parser, state, ret) {
571591
// When underlying `net.Socket` instance is consumed - no
572592
// `data` events are emitted, and thus `socket.setTimeout` fires the
@@ -589,6 +609,10 @@ const badRequestResponse = Buffer.from(
589609
`HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
590610
`Connection: close${CRLF}${CRLF}`, 'ascii'
591611
);
612+
const requestTimeoutResponse = Buffer.from(
613+
`HTTP/1.1 408 ${STATUS_CODES[408]}${CRLF}` +
614+
`Connection: close${CRLF}${CRLF}`, 'ascii'
615+
);
592616
const requestHeaderFieldsTooLargeResponse = Buffer.from(
593617
`HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
594618
`Connection: close${CRLF}${CRLF}`, 'ascii'
@@ -600,8 +624,20 @@ function socketOnError(e) {
600624

601625
if (!this.server.emit('clientError', e, this)) {
602626
if (this.writable && this.bytesWritten === 0) {
603-
const response = e.code === 'HPE_HEADER_OVERFLOW' ?
604-
requestHeaderFieldsTooLargeResponse : badRequestResponse;
627+
let response;
628+
629+
switch (e.code) {
630+
case 'HPE_HEADER_OVERFLOW':
631+
response = requestHeaderFieldsTooLargeResponse;
632+
break;
633+
case 'ERR_HTTP_REQUEST_TIMEOUT':
634+
response = requestTimeoutResponse;
635+
break;
636+
default:
637+
response = badRequestResponse;
638+
break;
639+
}
640+
605641
this.write(response);
606642
}
607643
this.destroy(e);
@@ -641,11 +677,20 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
641677
const bodyHead = d.slice(ret, d.length);
642678

643679
socket.readableFlowing = null;
680+
681+
// Clear the requestTimeout after upgrading the connection.
682+
clearRequestTimeout(req);
683+
644684
server.emit(eventName, req, socket, bodyHead);
645685
} else {
646686
// Got CONNECT method, but have no handler.
647687
socket.destroy();
648688
}
689+
} else {
690+
// When receiving new requests on the same socket (pipelining or keep alive)
691+
// make sure the requestTimeout is active.
692+
parser[kOnMessageBegin] =
693+
setRequestTimeout.bind(undefined, server, socket);
649694
}
650695

651696
if (socket._paused && socket.parser) {
@@ -671,6 +716,32 @@ function clearIncoming(req) {
671716
}
672717
}
673718

719+
function setRequestTimeout(server, socket) {
720+
// Set the request timeout handler.
721+
if (
722+
!socket[kRequestTimeout] &&
723+
server.requestTimeout && server.requestTimeout > 0
724+
) {
725+
debug('requestTimeout timer set');
726+
socket[kRequestTimeout] =
727+
setTimeout(onRequestTimeout, server.requestTimeout, socket).unref();
728+
}
729+
}
730+
731+
function clearRequestTimeout(req) {
732+
if (!req) {
733+
req = this;
734+
}
735+
736+
if (!req[kRequestTimeout]) {
737+
return;
738+
}
739+
740+
debug('requestTimeout timer cleared');
741+
clearTimeout(req[kRequestTimeout]);
742+
req[kRequestTimeout] = undefined;
743+
}
744+
674745
function resOnFinish(req, res, socket, state, server) {
675746
// Usually the first incoming element should be our request. it may
676747
// be that in the case abortIncoming() was called that the incoming
@@ -685,6 +756,14 @@ function resOnFinish(req, res, socket, state, server) {
685756
if (!req._consuming && !req._readableState.resumeScheduled)
686757
req._dump();
687758

759+
// Make sure the requestTimeout is cleared before finishing.
760+
// This might occur if the application has sent a response
761+
// without consuming the request body, which would have alredy
762+
// cleared the timer.
763+
// clearRequestTimeout can be executed even if the timer is not active,
764+
// so this is safe.
765+
clearRequestTimeout(req);
766+
688767
res.detachSocket(socket);
689768
clearIncoming(req);
690769
process.nextTick(emitCloseNT, res);
@@ -779,6 +858,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
779858
res.end();
780859
}
781860
} else {
861+
req.on('end', clearRequestTimeout);
862+
782863
server.emit('request', req, res);
783864
}
784865
return 0; // No special treatment.
Collapse file

‎lib/https.js‎

Copy file name to clipboardExpand all lines: lib/https.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ function Server(opts, requestListener) {
8080
this.keepAliveTimeout = 5000;
8181
this.maxHeadersCount = null;
8282
this.headersTimeout = 60 * 1000; // 60 seconds
83+
this.requestTimeout = 120 * 1000; // 120 seconds
8384
}
8485
ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype);
8586
ObjectSetPrototypeOf(Server, tls.Server);
Collapse file

‎lib/internal/errors.js‎

Copy file name to clipboardExpand all lines: lib/internal/errors.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,7 @@ E('ERR_HTTP_HEADERS_SENT',
936936
E('ERR_HTTP_INVALID_HEADER_VALUE',
937937
'Invalid value "%s" for header "%s"', TypeError);
938938
E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s', RangeError);
939+
E('ERR_HTTP_REQUEST_TIMEOUT', 'Request timeout', Error);
939940
E('ERR_HTTP_TRAILER_INVALID',
940941
'Trailers are invalid with this transfer encoding', Error);
941942
E('ERR_INCOMPATIBLE_OPTION_PAIR',
Collapse file

‎src/node_http_parser.cc‎

Copy file name to clipboardExpand all lines: src/node_http_parser.cc
+22-6Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,13 @@ using v8::Uint32;
6969
using v8::Undefined;
7070
using v8::Value;
7171

72-
const uint32_t kOnHeaders = 0;
73-
const uint32_t kOnHeadersComplete = 1;
74-
const uint32_t kOnBody = 2;
75-
const uint32_t kOnMessageComplete = 3;
76-
const uint32_t kOnExecute = 4;
77-
const uint32_t kOnTimeout = 5;
72+
const uint32_t kOnMessageBegin = 0;
73+
const uint32_t kOnHeaders = 1;
74+
const uint32_t kOnHeadersComplete = 2;
75+
const uint32_t kOnBody = 3;
76+
const uint32_t kOnMessageComplete = 4;
77+
const uint32_t kOnExecute = 5;
78+
const uint32_t kOnTimeout = 6;
7879
// Any more fields than this will be flushed into JS
7980
const size_t kMaxHeaderFieldsCount = 32;
8081

@@ -204,6 +205,19 @@ class Parser : public AsyncWrap, public StreamListener {
204205
url_.Reset();
205206
status_message_.Reset();
206207
header_parsing_start_time_ = uv_hrtime();
208+
209+
Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
210+
.ToLocalChecked();
211+
if (cb->IsFunction()) {
212+
InternalCallbackScope callback_scope(
213+
this, InternalCallbackScope::kSkipTaskQueues);
214+
215+
MaybeLocal<Value> r = cb.As<Function>()->Call(
216+
env()->context(), object(), 0, nullptr);
217+
218+
if (r.IsEmpty()) callback_scope.MarkAsFailed();
219+
}
220+
207221
return 0;
208222
}
209223

@@ -939,6 +953,8 @@ void InitializeHttpParser(Local<Object> target,
939953
Integer::New(env->isolate(), HTTP_REQUEST));
940954
t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "RESPONSE"),
941955
Integer::New(env->isolate(), HTTP_RESPONSE));
956+
t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnMessageBegin"),
957+
Integer::NewFromUnsigned(env->isolate(), kOnMessageBegin));
942958
t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnHeaders"),
943959
Integer::NewFromUnsigned(env->isolate(), kOnHeaders));
944960
t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnHeadersComplete"),
Collapse file

‎test/parallel/test-http-server-request-timeout-delayed-body.js‎

Copy file name to clipboard
+62Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const { createServer } = require('http');
6+
const { connect } = require('net');
7+
8+
// This test validates that the server returns 408
9+
// after server.requestTimeout if the client
10+
// pauses before start sending the body.
11+
12+
const server = createServer(common.mustCall((req, res) => {
13+
let body = '';
14+
req.setEncoding('utf-8');
15+
16+
req.on('data', (chunk) => {
17+
body += chunk;
18+
});
19+
20+
req.on('end', () => {
21+
res.writeHead(200, { 'Content-Type': 'text/plain' });
22+
res.write(body);
23+
res.end();
24+
});
25+
}));
26+
27+
// 0 seconds is the default
28+
assert.strictEqual(server.requestTimeout, 0);
29+
const requestTimeout = common.platformTimeout(1000);
30+
server.requestTimeout = requestTimeout;
31+
assert.strictEqual(server.requestTimeout, requestTimeout);
32+
33+
server.listen(0, common.mustCall(() => {
34+
const client = connect(server.address().port);
35+
let response = '';
36+
37+
client.on('data', common.mustCall((chunk) => {
38+
response += chunk.toString('utf-8');
39+
}));
40+
41+
client.resume();
42+
client.write('POST / HTTP/1.1\r\n');
43+
client.write('Content-Length: 20\r\n');
44+
client.write('Connection: close\r\n');
45+
client.write('\r\n');
46+
47+
setTimeout(() => {
48+
client.write('12345678901234567890\r\n\r\n');
49+
}, common.platformTimeout(2000)).unref();
50+
51+
const errOrEnd = common.mustCall(function(err) {
52+
console.log(err);
53+
assert.strictEqual(
54+
response,
55+
'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
56+
);
57+
server.close();
58+
});
59+
60+
client.on('end', errOrEnd);
61+
client.on('error', errOrEnd);
62+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.