From df08d527c2083b852d8456b88b39114f30525236 Mon Sep 17 00:00:00 2001
From: Matteo Collina <hello@matteocollina.com>
Date: Thu, 14 May 2020 20:21:34 +0200
Subject: 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: https://github.com/nodejs-private/node-private/pull/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>
---
 doc/api/errors.md                                  |  5 ++
 doc/api/http.md                                    | 17 ++++
 doc/api/https.md                                   | 10 +++
 lib/_http_common.js                                |  8 ++
 lib/_http_server.js                                | 85 ++++++++++++++++++-
 lib/https.js                                       |  1 +
 lib/internal/errors.js                             |  1 +
 src/node_http_parser.cc                            | 28 +++++--
 ...est-http-server-request-timeout-delayed-body.js | 62 ++++++++++++++
 ...-http-server-request-timeout-delayed-headers.js | 48 +++++++++++
 ...http-server-request-timeout-interrupted-body.js | 63 +++++++++++++++
 ...p-server-request-timeout-interrupted-headers.js | 48 +++++++++++
 .../test-http-server-request-timeout-keepalive.js  | 94 ++++++++++++++++++++++
 .../test-http-server-request-timeout-upgrade.js    | 55 +++++++++++++
 14 files changed, 517 insertions(+), 8 deletions(-)
 create mode 100644 test/parallel/test-http-server-request-timeout-delayed-body.js
 create mode 100644 test/parallel/test-http-server-request-timeout-delayed-headers.js
 create mode 100644 test/parallel/test-http-server-request-timeout-interrupted-body.js
 create mode 100644 test/parallel/test-http-server-request-timeout-interrupted-headers.js
 create mode 100644 test/parallel/test-http-server-request-timeout-keepalive.js
 create mode 100644 test/parallel/test-http-server-request-timeout-upgrade.js

diff --git a/doc/api/errors.md b/doc/api/errors.md
index db56c571e9c..98ca5c17a8f 100644
--- a/doc/api/errors.md
+++ b/doc/api/errors.md
@@ -940,6 +940,11 @@ allowed size for a `Buffer`.
 An invalid symlink type was passed to the [`fs.symlink()`][] or
 [`fs.symlinkSync()`][] methods.
 
+<a id="ERR_HTTP_REQUEST_TIMEOUT"></a>
+### `ERR_HTTP_REQUEST_TIMEOUT`
+
+The client has not sent the entire request within the allowed time.
+
 <a id="ERR_HTTP_HEADERS_SENT"></a>
 ### `ERR_HTTP_HEADERS_SENT`
 
diff --git a/doc/api/http.md b/doc/api/http.md
index e5871ef5a8c..8945ba3fda5 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -1259,6 +1259,23 @@ added: v0.7.0
 
 Limits maximum incoming headers count. If set to 0, no limit will be applied.
 
+### `server.requestTimeout`
+<!-- YAML
+added: REPLACEME
+-->
+
+* {number} **Default:** `0`
+
+Sets the timeout value in milliseconds for receiving the entire request from
+the client.
+
+If the timeout expires, the server responds with status 408 without
+forwarding the request to the request listener and then closes the connection.
+
+It must be set to a non-zero value (e.g. 120 seconds) to proctect against
+potential Denial-of-Service attacks in case the server is deployed without a
+reverse proxy in front.
+
 ### `server.setTimeout([msecs][, callback])`
 <!-- YAML
 added: v0.9.12
diff --git a/doc/api/https.md b/doc/api/https.md
index f94bf7f4e9e..8dbe4d2f224 100644
--- a/doc/api/https.md
+++ b/doc/api/https.md
@@ -113,6 +113,15 @@ This method is identical to [`server.listen()`][] from [`net.Server`][].
 
 See [`http.Server#maxHeadersCount`][].
 
+### `server.requestTimeout`
+<!-- YAML
+added: REPLACEME
+-->
+
+* {number} **Default:** `0`
+
+See [`http.Server#requestTimeout`][].
+
 ### `server.setTimeout([msecs][, callback])`
 <!-- YAML
 added: v0.11.2
@@ -451,6 +460,7 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; p
 [`http.Server#headersTimeout`]: http.html#http_server_headerstimeout
 [`http.Server#keepAliveTimeout`]: http.html#http_server_keepalivetimeout
 [`http.Server#maxHeadersCount`]: http.html#http_server_maxheaderscount
+[`http.Server#requestTimeout`]: http.html#http_server_requesttimeout
 [`http.Server#setTimeout()`]: http.html#http_server_settimeout_msecs_callback
 [`http.Server#timeout`]: http.html#http_server_timeout
 [`http.Server`]: http.html#http_class_http_server
diff --git a/lib/_http_common.js b/lib/_http_common.js
index a667557f9d3..e97670f0aca 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -44,6 +44,7 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
 });
 
 const kIncomingMessage = Symbol('IncomingMessage');
+const kRequestTimeout = Symbol('RequestTimeout');
 const kOnHeaders = HTTPParser.kOnHeaders | 0;
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
 const kOnBody = HTTPParser.kOnBody | 0;
@@ -99,6 +100,12 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
   incoming.url = url;
   incoming.upgrade = upgrade;
 
+  if (socket) {
+    debug('requestTimeout timer moved to req');
+    incoming[kRequestTimeout] = incoming.socket[kRequestTimeout];
+    incoming.socket[kRequestTimeout] = undefined;
+  }
+
   let n = headers.length;
 
   // If parser.maxHeaderPairs <= 0 assume that there's no limit.
@@ -264,6 +271,7 @@ module.exports = {
   methods,
   parsers,
   kIncomingMessage,
+  kRequestTimeout,
   HTTPParser,
   isLenient,
   prepareError,
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 84f2696b307..21eb68d463e 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -40,6 +40,7 @@ const {
   continueExpression,
   chunkExpression,
   kIncomingMessage,
+  kRequestTimeout,
   HTTPParser,
   isLenient,
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
@@ -61,6 +62,7 @@ const {
   codes
 } = require('internal/errors');
 const {
+  ERR_HTTP_REQUEST_TIMEOUT,
   ERR_HTTP_HEADERS_SENT,
   ERR_HTTP_INVALID_STATUS_CODE,
   ERR_HTTP_SOCKET_ENCODING,
@@ -77,6 +79,7 @@ const {
   DTRACE_HTTP_SERVER_RESPONSE
 } = require('internal/dtrace');
 const { observerCounts, constants } = internalBinding('performance');
+const { setTimeout, clearTimeout } = require('timers');
 const { NODE_PERFORMANCE_ENTRY_TYPE_HTTP } = constants;
 
 const kServerResponse = Symbol('ServerResponse');
@@ -148,6 +151,7 @@ const STATUS_CODES = {
   511: 'Network Authentication Required' // RFC 6585 6
 };
 
+const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 const kOnExecute = HTTPParser.kOnExecute | 0;
 const kOnTimeout = HTTPParser.kOnTimeout | 0;
 
@@ -369,6 +373,7 @@ function Server(options, requestListener) {
   this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.headersTimeout = 60 * 1000; // 60 seconds
+  this.requestTimeout = 0; // 120 seconds
 }
 ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
@@ -491,6 +496,16 @@ function connectionListenerInternal(server, socket) {
   parser[kOnTimeout] =
     onParserTimeout.bind(undefined, server, socket);
 
+  // When receiving new requests on the same socket (pipelining or keep alive)
+  // make sure the requestTimeout is active.
+  parser[kOnMessageBegin] =
+    setRequestTimeout.bind(undefined, server, socket);
+
+  // This protects from DOS attack where an attacker establish the connection
+  // without sending any data on applications where server.timeout is left to
+  // the default value of zero.
+  setRequestTimeout(server, socket);
+
   socket._paused = false;
 }
 
@@ -586,6 +601,11 @@ function socketOnData(server, socket, parser, state, d) {
   onParserExecuteCommon(server, socket, parser, state, ret, d);
 }
 
+function onRequestTimeout(socket) {
+  socket[kRequestTimeout] = undefined;
+  socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT());
+}
+
 function onParserExecute(server, socket, parser, state, ret) {
   // When underlying `net.Socket` instance is consumed - no
   // `data` events are emitted, and thus `socket.setTimeout` fires the
@@ -608,6 +628,10 @@ const badRequestResponse = Buffer.from(
   `HTTP/1.1 400 ${STATUS_CODES[400]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
 );
+const requestTimeoutResponse = Buffer.from(
+  `HTTP/1.1 408 ${STATUS_CODES[408]}${CRLF}` +
+  `Connection: close${CRLF}${CRLF}`, 'ascii'
+);
 const requestHeaderFieldsTooLargeResponse = Buffer.from(
   `HTTP/1.1 431 ${STATUS_CODES[431]}${CRLF}` +
   `Connection: close${CRLF}${CRLF}`, 'ascii'
@@ -619,8 +643,20 @@ function socketOnError(e) {
 
   if (!this.server.emit('clientError', e, this)) {
     if (this.writable && this.bytesWritten === 0) {
-      const response = e.code === 'HPE_HEADER_OVERFLOW' ?
-        requestHeaderFieldsTooLargeResponse : badRequestResponse;
+      let response;
+
+      switch (e.code) {
+        case 'HPE_HEADER_OVERFLOW':
+          response = requestHeaderFieldsTooLargeResponse;
+          break;
+        case 'ERR_HTTP_REQUEST_TIMEOUT':
+          response = requestTimeoutResponse;
+          break;
+        default:
+          response = badRequestResponse;
+          break;
+      }
+
       this.write(response);
     }
     this.destroy(e);
@@ -660,11 +696,20 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
       const bodyHead = d.slice(ret, d.length);
 
       socket.readableFlowing = null;
+
+      // Clear the requestTimeout after upgrading the connection.
+      clearRequestTimeout(req);
+
       server.emit(eventName, req, socket, bodyHead);
     } else {
       // Got CONNECT method, but have no handler.
       socket.destroy();
     }
+  } else {
+    // When receiving new requests on the same socket (pipelining or keep alive)
+    // make sure the requestTimeout is active.
+    parser[kOnMessageBegin] =
+      setRequestTimeout.bind(undefined, server, socket);
   }
 
   if (socket._paused && socket.parser) {
@@ -692,6 +737,32 @@ function clearIncoming(req) {
   }
 }
 
+function setRequestTimeout(server, socket) {
+  // Set the request timeout handler.
+  if (
+    !socket[kRequestTimeout] &&
+    server.requestTimeout && server.requestTimeout > 0
+  ) {
+    debug('requestTimeout timer set');
+    socket[kRequestTimeout] =
+      setTimeout(onRequestTimeout, server.requestTimeout, socket).unref();
+  }
+}
+
+function clearRequestTimeout(req) {
+  if (!req) {
+    req = this;
+  }
+
+  if (!req[kRequestTimeout]) {
+    return;
+  }
+
+  debug('requestTimeout timer cleared');
+  clearTimeout(req[kRequestTimeout]);
+  req[kRequestTimeout] = undefined;
+}
+
 function resOnFinish(req, res, socket, state, server) {
   // Usually the first incoming element should be our request.  it may
   // be that in the case abortIncoming() was called that the incoming
@@ -706,6 +777,14 @@ function resOnFinish(req, res, socket, state, server) {
   if (!req._consuming && !req._readableState.resumeScheduled)
     req._dump();
 
+  // Make sure the requestTimeout is cleared before finishing.
+  // This might occur if the application has sent a response
+  // without consuming the request body, which would have alredy
+  // cleared the timer.
+  // clearRequestTimeout can be executed even if the timer is not active,
+  // so this is safe.
+  clearRequestTimeout(req);
+
   res.detachSocket(socket);
   clearIncoming(req);
   process.nextTick(emitCloseNT, res);
@@ -802,6 +881,8 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
       res.end();
     }
   } else {
+    req.on('end', clearRequestTimeout);
+
     server.emit('request', req, res);
   }
   return 0;  // No special treatment.
diff --git a/lib/https.js b/lib/https.js
index 17a89c2f0bc..4b21977fe48 100644
--- a/lib/https.js
+++ b/lib/https.js
@@ -80,6 +80,7 @@ function Server(opts, requestListener) {
   this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.headersTimeout = 60 * 1000; // 60 seconds
+  this.requestTimeout = 120 * 1000; // 120 seconds
 }
 ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype);
 ObjectSetPrototypeOf(Server, tls.Server);
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 787298288b6..ff4cea13b61 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -940,6 +940,7 @@ E('ERR_HTTP_HEADERS_SENT',
 E('ERR_HTTP_INVALID_HEADER_VALUE',
   'Invalid value "%s" for header "%s"', TypeError);
 E('ERR_HTTP_INVALID_STATUS_CODE', 'Invalid status code: %s', RangeError);
+E('ERR_HTTP_REQUEST_TIMEOUT', 'Request timeout', Error);
 E('ERR_HTTP_SOCKET_ENCODING',
   'Changing the socket encoding is not allowed per RFC7230 Section 3.', Error);
 E('ERR_HTTP_TRAILER_INVALID',
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index b409d007307..eada685f2d6 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -69,12 +69,13 @@ using v8::Uint32;
 using v8::Undefined;
 using v8::Value;
 
-const uint32_t kOnHeaders = 0;
-const uint32_t kOnHeadersComplete = 1;
-const uint32_t kOnBody = 2;
-const uint32_t kOnMessageComplete = 3;
-const uint32_t kOnExecute = 4;
-const uint32_t kOnTimeout = 5;
+const uint32_t kOnMessageBegin = 0;
+const uint32_t kOnHeaders = 1;
+const uint32_t kOnHeadersComplete = 2;
+const uint32_t kOnBody = 3;
+const uint32_t kOnMessageComplete = 4;
+const uint32_t kOnExecute = 5;
+const uint32_t kOnTimeout = 6;
 // Any more fields than this will be flushed into JS
 const size_t kMaxHeaderFieldsCount = 32;
 
@@ -204,6 +205,19 @@ class Parser : public AsyncWrap, public StreamListener {
     url_.Reset();
     status_message_.Reset();
     header_parsing_start_time_ = uv_hrtime();
+
+    Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
+                              .ToLocalChecked();
+    if (cb->IsFunction()) {
+      InternalCallbackScope callback_scope(
+        this, InternalCallbackScope::kSkipTaskQueues);
+
+      MaybeLocal<Value> r = cb.As<Function>()->Call(
+        env()->context(), object(), 0, nullptr);
+
+      if (r.IsEmpty()) callback_scope.MarkAsFailed();
+    }
+
     return 0;
   }
 
@@ -939,6 +953,8 @@ void InitializeHttpParser(Local<Object> target,
          Integer::New(env->isolate(), HTTP_REQUEST));
   t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "RESPONSE"),
          Integer::New(env->isolate(), HTTP_RESPONSE));
+  t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnMessageBegin"),
+         Integer::NewFromUnsigned(env->isolate(), kOnMessageBegin));
   t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnHeaders"),
          Integer::NewFromUnsigned(env->isolate(), kOnHeaders));
   t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnHeadersComplete"),
diff --git a/test/parallel/test-http-server-request-timeout-delayed-body.js b/test/parallel/test-http-server-request-timeout-delayed-body.js
new file mode 100644
index 00000000000..c6f26def86b
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-delayed-body.js
@@ -0,0 +1,62 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the server returns 408
+// after server.requestTimeout if the client
+// pauses before start sending the body.
+
+const server = createServer(common.mustCall((req, res) => {
+  let body = '';
+  req.setEncoding('utf-8');
+
+  req.on('data', (chunk) => {
+    body += chunk;
+  });
+
+  req.on('end', () => {
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    res.write(body);
+    res.end();
+  });
+}));
+
+// 0 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  client.resume();
+  client.write('POST / HTTP/1.1\r\n');
+  client.write('Content-Length: 20\r\n');
+  client.write('Connection: close\r\n');
+  client.write('\r\n');
+
+  setTimeout(() => {
+    client.write('12345678901234567890\r\n\r\n');
+  }, common.platformTimeout(2000)).unref();
+
+  const errOrEnd = common.mustCall(function(err) {
+    console.log(err);
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  });
+
+  client.on('end', errOrEnd);
+  client.on('error', errOrEnd);
+}));
diff --git a/test/parallel/test-http-server-request-timeout-delayed-headers.js b/test/parallel/test-http-server-request-timeout-delayed-headers.js
new file mode 100644
index 00000000000..b2c282fd2fa
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-delayed-headers.js
@@ -0,0 +1,48 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the server returns 408
+// after server.requestTimeout if the client
+// pauses before start sending the request.
+
+const server = createServer(common.mustNotCall());
+
+// 0 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  const errOrEnd = common.mustCall(function(err) {
+    console.log(err);
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  });
+
+  client.on('end', errOrEnd);
+  client.on('error', errOrEnd);
+
+  client.resume();
+
+  setTimeout(() => {
+    client.write('POST / HTTP/1.1\r\n');
+    client.write('Content-Length: 20\r\n');
+    client.write('Connection: close\r\n\r\n');
+    client.write('12345678901234567890\r\n\r\n');
+  }, common.platformTimeout(2000)).unref();
+}));
diff --git a/test/parallel/test-http-server-request-timeout-interrupted-body.js b/test/parallel/test-http-server-request-timeout-interrupted-body.js
new file mode 100644
index 00000000000..237e133fef7
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-interrupted-body.js
@@ -0,0 +1,63 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the server returns 408
+// after server.requestTimeout if the client
+// pauses sending in the middle of the body.
+
+const server = createServer(common.mustCall((req, res) => {
+  let body = '';
+  req.setEncoding('utf-8');
+
+  req.on('data', (chunk) => {
+    body += chunk;
+  });
+
+  req.on('end', () => {
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    res.write(body);
+    res.end();
+  });
+}));
+
+// 0 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  const errOrEnd = common.mustCall(function(err) {
+    console.log(err);
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  });
+
+  client.on('error', errOrEnd);
+  client.on('end', errOrEnd);
+
+  client.resume();
+  client.write('POST / HTTP/1.1\r\n');
+  client.write('Content-Length: 20\r\n');
+  client.write('Connection: close\r\n');
+  client.write('\r\n');
+  client.write('1234567890');
+
+  setTimeout(() => {
+    client.write('1234567890\r\n\r\n');
+  }, common.platformTimeout(2000)).unref();
+}));
diff --git a/test/parallel/test-http-server-request-timeout-interrupted-headers.js b/test/parallel/test-http-server-request-timeout-interrupted-headers.js
new file mode 100644
index 00000000000..0b8a2c315ab
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-interrupted-headers.js
@@ -0,0 +1,48 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the server returns 408
+// after server.requestTimeout if the client
+// pauses sending in the middle of a header.
+
+const server = createServer(common.mustNotCall());
+
+// 120 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let response = '';
+
+  client.on('data', common.mustCall((chunk) => {
+    response += chunk.toString('utf-8');
+  }));
+
+  const errOrEnd = common.mustCall(function(err) {
+    console.log(err);
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
+    );
+    server.close();
+  });
+
+  client.on('end', errOrEnd);
+  client.on('error', errOrEnd);
+
+  client.resume();
+  client.write('GET / HTTP/1.1\r\n');
+  client.write('Connection: close\r\n');
+  client.write('X-CRASH: ');
+
+  setTimeout(() => {
+    client.write('1234567890\r\n\r\n');
+  }, common.platformTimeout(2000)).unref();
+}));
diff --git a/test/parallel/test-http-server-request-timeout-keepalive.js b/test/parallel/test-http-server-request-timeout-keepalive.js
new file mode 100644
index 00000000000..77fde867e9b
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-keepalive.js
@@ -0,0 +1,94 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the server returns 408
+// after server.requestTimeout if the client
+// does not complete a request, and that keep alive
+// works properly
+
+function performRequestWithDelay(client, firstDelay, secondDelay) {
+  client.resume();
+  client.write('GET / HTTP/1.1\r\n');
+
+  firstDelay = common.platformTimeout(firstDelay);
+  secondDelay = common.platformTimeout(secondDelay);
+
+  console.log('performRequestWithDelay', firstDelay, secondDelay);
+
+  setTimeout(() => {
+    client.write('Connection: ');
+  }, firstDelay).unref();
+
+  // Complete the request
+  setTimeout(() => {
+    client.write('keep-alive\r\n\r\n');
+  }, firstDelay + secondDelay).unref();
+}
+
+const server = createServer(common.mustCallAtLeast((req, res) => {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end();
+}));
+
+// 0 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+// Make sure keepAliveTimeout is big enough for the requestTimeout.
+server.keepAliveTimeout = 0;
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let second = false;
+  let response = '';
+
+  client.on('data', common.mustCallAtLeast((chunk) => {
+    response += chunk.toString('utf-8');
+
+    // First response has ended
+    if (!second && response.endsWith('\r\n\r\n')) {
+      assert.strictEqual(
+        response.split('\r\n')[0],
+        'HTTP/1.1 200 OK'
+      );
+
+      const defer = common.platformTimeout(server.requestTimeout * 1.5);
+
+      console.log('defer by', defer);
+
+      // Wait some time to make sure requestTimeout
+      // does not interfere with keep alive
+      setTimeout(() => {
+        response = '';
+        second = true;
+
+        // Perform a second request expected to finish after requestTimeout
+        performRequestWithDelay(client, 1000, 3000);
+      }, defer).unref();
+    }
+  }, 1));
+
+  const errOrEnd = common.mustCall(function(err) {
+    console.log(err);
+    assert.strictEqual(second, true);
+    assert.strictEqual(
+      response,
+      // Empty because of https://github.com/nodejs/node/commit/e8d7fedf7cad6e612e4f2e0456e359af57608ac7
+      // 'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
+      ''
+    );
+    server.close();
+  });
+
+  client.on('error', errOrEnd);
+  client.on('end', errOrEnd);
+
+  // Perform a second request expected to finish before requestTimeout
+  performRequestWithDelay(client, 50, 500);
+}));
diff --git a/test/parallel/test-http-server-request-timeout-upgrade.js b/test/parallel/test-http-server-request-timeout-upgrade.js
new file mode 100644
index 00000000000..dd7269621e2
--- /dev/null
+++ b/test/parallel/test-http-server-request-timeout-upgrade.js
@@ -0,0 +1,55 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// This test validates that the requestTimeoout
+// is disabled after the connection is upgraded.
+
+const server = createServer(common.mustNotCall());
+
+// 0 seconds is the default
+assert.strictEqual(server.requestTimeout, 0);
+const requestTimeout = common.platformTimeout(1000);
+server.requestTimeout = requestTimeout;
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+server.on('upgrade', common.mustCall((req, socket, head) => {
+  socket.write('HTTP/1.1 101 Web Socket Protocol Handshake\r\n');
+  socket.write('Upgrade: WebSocket\r\n');
+  socket.write('Connection: Upgrade\r\n\r\n');
+  socket.pipe(socket);
+}));
+
+server.listen(0, common.mustCall(() => {
+  const client = connect(server.address().port);
+  let response = '';
+
+  client.on('data', common.mustCallAtLeast((chunk) => {
+    response += chunk.toString('utf-8');
+  }, 1));
+
+  client.on('end', common.mustCall(() => {
+    assert.strictEqual(
+      response,
+      'HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
+      'Upgrade: WebSocket\r\n' +
+      'Connection: Upgrade\r\n\r\n' +
+      '12345678901234567890'
+    );
+
+    server.close();
+  }));
+
+  client.resume();
+  client.write('GET / HTTP/1.1\r\n');
+  client.write('Upgrade: WebSocket\r\n');
+  client.write('Connection: Upgrade\r\n\r\n');
+
+  setTimeout(() => {
+    client.write('12345678901234567890');
+    client.end();
+  }, common.platformTimeout(2000)).unref();
+}));
-- 
cgit v1.2.3