http: refactor headersTimeout and requestTimeout logic

PR-URL: https://github.com/nodejs/node/pull/41263
Fixes: https://github.com/nodejs/node/issues/33440
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>

benchmark/http/chunked.js

@@ -32,10 +32,11 @@ function main({ len, n, c, duration }) {
     send(n);
   });
 
-  server.listen(common.PORT, () => {
+  server.listen(0, () => {
     bench.http({
       connections: c,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/client-request-body.js

@@ -32,7 +32,6 @@ function main({ dur, len, type, method }) {
     headers: { 'Connection': 'keep-alive', 'Transfer-Encoding': 'chunked' },
     agent: new http.Agent({ maxSockets: 1 }),
     host: '127.0.0.1',
-    port: common.PORT,
     path: '/',
     method: 'POST'
   };
@@ -40,16 +39,17 @@ function main({ dur, len, type, method }) {
   const server = http.createServer((req, res) => {
     res.end();
   });
-  server.listen(options.port, options.host, () => {
+  server.listen(0, options.host, () => {
     setTimeout(done, dur * 1000);
     bench.start();
-    pummel();
+    pummel(server.address().port);
   });
 
-  function pummel() {
+  function pummel(port) {
+    options.port = port;
     const req = http.request(options, (res) => {
       nreqs++;
-      pummel();  // Line up next request.
+      pummel(port);  // Line up next request.
       res.resume();
     });
     if (method === 'write') {

benchmark/http/end-vs-write-end.js

@@ -48,10 +48,11 @@ function main({ len, type, method, c, duration }) {
     fn(res);
   });
 
-  server.listen(common.PORT, () => {
+  server.listen(0, () => {
     bench.http({
       connections: c,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/headers.js

@@ -27,11 +27,12 @@ function main({ len, n, duration }) {
     res.writeHead(200, headers);
     res.end();
   });
-  server.listen(common.PORT, () => {
+  server.listen(0, () => {
     bench.http({
       path: '/',
       connections: 10,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/incoming_headers.js

@@ -14,7 +14,7 @@ function main({ connections, headers, w, duration }) {
     res.end();
   });
 
-  server.listen(common.PORT, () => {
+  server.listen(0, () => {
     const headers = {
       'Content-Type': 'text/plain',
       'Accept': 'text/plain',
@@ -34,7 +34,8 @@ function main({ connections, headers, w, duration }) {
       path: '/',
       connections,
       headers,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/set-header.js

@@ -1,6 +1,5 @@
 'use strict';
 const common = require('../common.js');
-const PORT = common.PORT;
 
 const bench = common.createBenchmark(main, {
   res: ['normal', 'setHeader', 'setHeaderWH'],
@@ -17,16 +16,16 @@ const c = 50;
 // setHeader: statusCode = status, setHeader(...) x2
 // setHeaderWH: setHeader(...), writeHead(status, ...)
 function main({ res, duration }) {
-  process.env.PORT = PORT;
   const server = require('../fixtures/simple-http-server.js')
-  .listen(PORT)
+  .listen(0)
   .on('listening', () => {
     const path = `/${type}/${len}/${chunks}/${res}/${chunkedEnc}`;
 
     bench.http({
       path: path,
       connections: c,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/simple.js

@@ -13,14 +13,15 @@ const bench = common.createBenchmark(main, {
 
 function main({ type, len, chunks, c, chunkedEnc, duration }) {
   const server = require('../fixtures/simple-http-server.js')
-  .listen(common.PORT)
+  .listen(0)
   .on('listening', () => {
     const path = `/${type}/${len}/${chunks}/normal/${chunkedEnc}`;
 
     bench.http({
       path,
       connections: c,
-      duration
+      duration,
+      port: server.address().port,
     }, () => {
       server.close();
     });

benchmark/http/upgrade.js

@@ -20,7 +20,7 @@ const resData = 'HTTP/1.1 101 Web Socket Protocol Handshake\r\n' +
 
 function main({ n }) {
   const server = require('../fixtures/simple-http-server.js')
-    .listen(common.PORT)
+    .listen(0)
     .on('listening', () => {
       bench.start();
       doBench(server.address(), n, () => {

doc/api/http.md

@@ -1364,15 +1364,12 @@ added:
 Limit the amount of time the parser will wait to receive the complete HTTP
 headers.
 
-In case of inactivity, the rules defined in [`server.timeout`][] apply. However,
-that inactivity based timeout would still allow the connection to be kept open
-if the headers are being sent very slowly (by default, up to a byte per 2
-minutes). In order to prevent this, whenever header data arrives an additional
-check is made that more than `server.headersTimeout` milliseconds has not
-passed since the connection was established. If the check fails, a `'timeout'`
-event is emitted on the server object, and (by default) the socket is destroyed.
-See [`server.timeout`][] for more information on how timeout behavior can be
-customized.
+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 protect against
+potential Denial-of-Service attacks in case the server is deployed without a
+reverse proxy in front.
 
 ### `server.listen()`
 
@@ -1401,9 +1398,14 @@ Limits maximum incoming headers count. If set to 0, no limit will be applied.
 
 <!-- YAML
 added: v14.11.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/41263
+    description: The default request timeout changed
+                 from no timeout to 300s (5 minutes).
 -->
 
-* {number} **Default:** `0`
+* {number} **Default:** `300000`
 
 Sets the timeout value in milliseconds for receiving the entire request from
 the client.
@@ -2856,6 +2858,10 @@ Found'`.
 <!-- YAML
 added: v0.1.13
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/41263
+    description: The `requestTimeout`, `headersTimeout`, `keepAliveTimeout` and
+                 `connectionsCheckingInterval` are supported now.
   - version: REPLACEME
     pr-url: https://github.com/nodejs/node/pull/42163
     description: The `noDelay` option now defaults to `true`.
@@ -2886,6 +2892,22 @@ changes:
   * `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
     to be used. Useful for extending the original `ServerResponse`. **Default:**
     `ServerResponse`.
+  * `requestTimeout`: Sets the timeout value in milliseconds for receiving
+    the entire request from the client.
+    See [`server.requestTimeout`][] for more information.
+    **Default:** `300000`.
+  * `headersTimeout`: Sets the timeout value in milliseconds for receiving
+    the complete HTTP headers from the client.
+    See [`server.headersTimeout`][] for more information.
+    **Default:** `60000`.
+  * `keepAliveTimeout`: The number of milliseconds of inactivity a server
+    needs to wait for additional incoming data, after it has finished writing
+    the last response, before a socket will be destroyed.
+    See [`server.keepAliveTimeout`][] for more information.
+    **Default:** `5000`.
+  * `connectionsCheckingInterval`: Sets the interval value in milliseconds to
+    check for request and headers timeout in incomplete requests.
+    **Default:** `30000`.
   * `insecureHTTPParser` {boolean} Use an insecure HTTP parser that accepts
     invalid HTTP headers when `true`. Using the insecure parser should be
     avoided. See [`--insecure-http-parser`][] for more information.
@@ -3478,7 +3500,10 @@ try {
 [`response.write(data, encoding)`]: #responsewritechunk-encoding-callback
 [`response.writeContinue()`]: #responsewritecontinue
 [`response.writeHead()`]: #responsewriteheadstatuscode-statusmessage-headers
+[`server.headersTimeout`]: #serverheaderstimeout
+[`server.keepAliveTimeout`]: #serverkeepalivetimeout
 [`server.listen()`]: net.md#serverlisten
+[`server.requestTimeout`]: #serverrequesttimeout
 [`server.timeout`]: #servertimeout
 [`setHeader(name, value)`]: #requestsetheadername-value
 [`socket.connect()`]: net.md#socketconnectoptions-connectlistener

lib/_http_client.js

@@ -735,8 +735,7 @@ function tickOnSocket(req, socket) {
   parser.initialize(HTTPParser.RESPONSE,
                     new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
                     req.maxHeaderSize || 0,
-                    lenient ? kLenientAll : kLenientNone,
-                    0);
+                    lenient ? kLenientAll : kLenientNone);
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;

lib/_http_common.js

@@ -40,12 +40,7 @@ const {
   readStop
 } = incoming;
 
-let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
-  debug = fn;
-});
-
 const kIncomingMessage = Symbol('IncomingMessage');
-const kRequestTimeout = Symbol('RequestTimeout');
 const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 const kOnHeaders = HTTPParser.kOnHeaders | 0;
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
@@ -102,12 +97,6 @@ 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.
@@ -273,7 +262,6 @@ module.exports = {
   methods,
   parsers,
   kIncomingMessage,
-  kRequestTimeout,
   HTTPParser,
   isLenient,
   prepareError,

lib/_http_server.js

@@ -27,6 +27,7 @@ const {
   ObjectKeys,
   ObjectSetPrototypeOf,
   RegExpPrototypeTest,
+  ReflectApply,
   Symbol,
   SymbolFor,
 } = primordials;
@@ -40,12 +41,12 @@ const {
   continueExpression,
   chunkExpression,
   kIncomingMessage,
-  kRequestTimeout,
   HTTPParser,
   isLenient,
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
   prepareError,
 } = require('_http_common');
+const { ConnectionsList } = internalBinding('http_parser');
 const { OutgoingMessage } = require('_http_outgoing');
 const {
   kOutHeaders,
@@ -79,8 +80,7 @@ const {
   DTRACE_HTTP_SERVER_REQUEST,
   DTRACE_HTTP_SERVER_RESPONSE
 } = require('internal/dtrace');
-const { setTimeout, clearTimeout } = require('timers');
-
+const { setInterval, clearInterval } = require('timers');
 let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
   debug = fn;
 });
@@ -162,11 +162,12 @@ 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;
 const kLenientAll = HTTPParser.kLenientAll | 0;
 const kLenientNone = HTTPParser.kLenientNone | 0;
+const kConnections = Symbol('http.server.connections');
+const kConnectionsCheckingInterval = Symbol('http.server.connectionsCheckingInterval');
 
 class HTTPServerAsyncResource {
   constructor(type, socket) {
@@ -367,6 +368,51 @@ function storeHTTPOptions(options) {
 
   if (options.noDelay === undefined)
     options.noDelay = true;
+
+  const requestTimeout = options.requestTimeout;
+  if (requestTimeout !== undefined) {
+    validateInteger(requestTimeout, 'requestTimeout', 0);
+    this.requestTimeout = requestTimeout;
+  } else {
+    this.requestTimeout = 300_000; // 5 minutes
+  }
+
+  const headersTimeout = options.headersTimeout;
+  if (headersTimeout !== undefined) {
+    validateInteger(headersTimeout, 'headersTimeout', 0);
+    this.headersTimeout = headersTimeout;
+  } else {
+    this.headersTimeout = 60_000; // 60 seconds
+  }
+
+  if (this.requestTimeout > 0 && this.headersTimeout > 0 && this.headersTimeout >= this.requestTimeout) {
+    throw new codes.ERR_OUT_OF_RANGE('headersTimeout', '>= requestTimeout', headersTimeout);
+  }
+
+  const keepAliveTimeout = options.keepAliveTimeout;
+  if (keepAliveTimeout !== undefined) {
+    validateInteger(keepAliveTimeout, 'keepAliveTimeout', 0);
+    this.keepAliveTimeout = keepAliveTimeout;
+  } else {
+    this.keepAliveTimeout = 5_000; // 5 seconds;
+  }
+
+  const connectionsCheckingInterval = options.connectionsCheckingInterval;
+  if (connectionsCheckingInterval !== undefined) {
+    validateInteger(connectionsCheckingInterval, 'connectionsCheckingInterval', 0);
+    this.connectionsCheckingInterval = connectionsCheckingInterval;
+  } else {
+    this.connectionsCheckingInterval = 30_000; // 30 seconds
+  }
+}
+
+function setupConnectionsTracking(server) {
+  // Start connection handling
+  server[kConnections] = new ConnectionsList();
+  if (server.headersTimeout > 0 || server.requestTimeout > 0) {
+    server[kConnectionsCheckingInterval] =
+      setInterval(checkConnections.bind(server), server.connectionsCheckingInterval).unref();
+  }
 }
 
 function Server(options, requestListener) {
@@ -400,15 +446,17 @@ function Server(options, requestListener) {
   this.on('connection', connectionListener);
 
   this.timeout = 0;
-  this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.maxRequestsPerSocket = 0;
-  this.headersTimeout = 60 * 1000; // 60 seconds
-  this.requestTimeout = 0;
+  setupConnectionsTracking(this);
 }
 ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
 
+Server.prototype.close = function() {
+  clearInterval(this[kConnectionsCheckingInterval]);
+  ReflectApply(net.Server.prototype.close, this, arguments);
+};
 
 Server.prototype.setTimeout = function setTimeout(msecs, callback) {
   this.timeout = msecs;
@@ -440,6 +488,18 @@ Server.prototype[EE.captureRejectionSymbol] = function(err, event, ...args) {
   }
 };
 
+function checkConnections() {
+  const expired = this[kConnections].expired(this.headersTimeout, this.requestTimeout);
+
+  for (let i = 0; i < expired.length; i++) {
+    const socket = expired[i].socket;
+
+    if (socket) {
+      onRequestTimeout(socket);
+    }
+  }
+}
+
 function connectionListener(socket) {
   defaultTriggerAsyncIdScope(
     getOrSetAsyncId(socket), connectionListenerInternal, this, socket
@@ -473,7 +533,7 @@ function connectionListenerInternal(server, socket) {
     new HTTPServerAsyncResource('HTTPINCOMINGMESSAGE', socket),
     server.maxHeaderSize || 0,
     lenient ? kLenientAll : kLenientNone,
-    server.headersTimeout || 0,
+    server[kConnections],
   );
   parser.socket = socket;
   socket.parser = parser;
@@ -539,17 +599,6 @@ function connectionListenerInternal(server, socket) {
     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;
 }
 
@@ -632,7 +681,6 @@ function socketOnData(server, socket, parser, state, d) {
 }
 
 function onRequestTimeout(socket) {
-  socket[kRequestTimeout] = undefined;
   // socketOnError has additional logic and will call socket.destroy(err).
   socketOnError.call(socket, new ERR_HTTP_REQUEST_TIMEOUT());
 }
@@ -727,9 +775,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
 
       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.
@@ -738,11 +783,6 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
   } else if (parser.incoming && parser.incoming.method === 'PRI') {
     debug('SERVER got PRI request');
     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) {
@@ -765,32 +805,6 @@ 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) {
   if (onResponseFinishChannel.hasSubscribers) {
     onResponseFinishChannel.publish({
@@ -814,14 +828,6 @@ 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 already
-  // 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);
@@ -955,7 +961,6 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
   }
 
   if (!handled) {
-    req.on('end', clearRequestTimeout);
     server.emit('request', req, res);
   }
 
@@ -1027,6 +1032,7 @@ module.exports = {
   STATUS_CODES,
   Server,
   ServerResponse,
+  setupConnectionsTracking,
   storeHTTPOptions,
   _connectionListener: connectionListener,
   kServerResponse

lib/https.js

@@ -40,6 +40,7 @@ const tls = require('tls');
 const { Agent: HttpAgent } = require('_http_agent');
 const {
   Server: HttpServer,
+  setupConnectionsTracking,
   storeHTTPOptions,
   _connectionListener,
 } = require('_http_server');
@@ -80,10 +81,8 @@ function Server(opts, requestListener) {
   });
 
   this.timeout = 0;
-  this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
-  this.headersTimeout = 60 * 1000; // 60 seconds
-  this.requestTimeout = 0;
+  setupConnectionsTracking(this);
 }
 ObjectSetPrototypeOf(Server.prototype, tls.Server.prototype);
 ObjectSetPrototypeOf(Server, tls.Server);

src/node_http_parser.cc

@@ -60,6 +60,7 @@ using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Int32;
 using v8::Integer;
+using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
 using v8::Number;
@@ -187,7 +188,58 @@ struct StringPtr {
   size_t size_;
 };
 
+class Parser;
+
+struct ParserComparator {
+  bool operator()(const Parser* lhs, const Parser* rhs) const;
+};
+
+class ConnectionsList : public BaseObject {
+ public:
+    static void New(const FunctionCallbackInfo<Value>& args);
+
+    static void All(const FunctionCallbackInfo<Value>& args);
+
+    static void Idle(const FunctionCallbackInfo<Value>& args);
+
+    static void Active(const FunctionCallbackInfo<Value>& args);
+
+    static void Expired(const FunctionCallbackInfo<Value>& args);
+
+    void Push(Parser* parser) {
+      all_connections_.insert(parser);
+    }
+
+    void Pop(Parser* parser) {
+      all_connections_.erase(parser);
+    }
+
+    void PushActive(Parser* parser) {
+      active_connections_.insert(parser);
+    }
+
+    void PopActive(Parser* parser) {
+      active_connections_.erase(parser);
+    }
+
+    SET_NO_MEMORY_INFO()
+    SET_MEMORY_INFO_NAME(ConnectionsList)
+    SET_SELF_SIZE(ConnectionsList)
+
+ private:
+    ConnectionsList(Environment* env, Local<Object> object)
+      : BaseObject(env, object) {
+        MakeWeak();
+      }
+
+    std::set<Parser*, ParserComparator> all_connections_;
+    std::set<Parser*, ParserComparator> active_connections_;
+};
+
 class Parser : public AsyncWrap, public StreamListener {
+  friend class ConnectionsList;
+  friend struct ParserComparator;
+
  public:
   Parser(BindingData* binding_data, Local<Object> wrap)
       : AsyncWrap(binding_data->env(), wrap),
@@ -205,10 +257,21 @@ class Parser : public AsyncWrap, public StreamListener {
   SET_SELF_SIZE(Parser)
 
   int on_message_begin() {
+    // Important: Pop from the list BEFORE resetting the last_message_start_
+    // otherwise std::set.erase will fail.
+    if (connectionsList_ != nullptr) {
+      connectionsList_->PopActive(this);
+    }
+
     num_fields_ = num_values_ = 0;
+    headers_completed_ = false;
+    last_message_start_ = uv_hrtime();
     url_.Reset();
     status_message_.Reset();
-    header_parsing_start_time_ = uv_hrtime();
+
+    if (connectionsList_ != nullptr) {
+      connectionsList_->PushActive(this);
+    }
 
     Local<Value> cb = object()->Get(env()->context(), kOnMessageBegin)
                               .ToLocalChecked();
@@ -297,8 +360,8 @@ class Parser : public AsyncWrap, public StreamListener {
 
 
   int on_headers_complete() {
+    headers_completed_ = true;
     header_nread_ = 0;
-    header_parsing_start_time_ = 0;
 
     // Arguments for the on-headers-complete javascript callback. This
     // list needs to be kept in sync with the actual argument list for
@@ -429,6 +492,14 @@ class Parser : public AsyncWrap, public StreamListener {
   int on_message_complete() {
     HandleScope scope(env()->isolate());
 
+    // Important: Pop from the list BEFORE resetting the last_message_start_
+    // otherwise std::set.erase will fail.
+    if (connectionsList_ != nullptr) {
+      connectionsList_->PopActive(this);
+    }
+
+    last_message_start_ = 0;
+
     if (num_fields_)
       Flush();  // Flush trailing HTTP headers.
 
@@ -486,6 +557,11 @@ class Parser : public AsyncWrap, public StreamListener {
     Parser* parser;
     ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
 
+    if (parser->connectionsList_ != nullptr) {
+      parser->connectionsList_->Pop(parser);
+      parser->connectionsList_->PopActive(parser);
+    }
+
     // Since the Parser destructor isn't going to run the destroy() callbacks
     // it needs to be triggered manually.
     parser->EmitTraceEventDestroy();
@@ -506,7 +582,6 @@ class Parser : public AsyncWrap, public StreamListener {
     }
   }
 
-
   // var bytesParsed = parser->execute(buffer);
   static void Execute(const FunctionCallbackInfo<Value>& args) {
     Parser* parser;
@@ -545,8 +620,8 @@ class Parser : public AsyncWrap, public StreamListener {
     Environment* env = Environment::GetCurrent(args);
 
     uint64_t max_http_header_size = 0;
-    uint64_t headers_timeout = 0;
     uint32_t lenient_flags = kLenientNone;
+    ConnectionsList* connectionsList = nullptr;
 
     CHECK(args[0]->IsInt32());
     CHECK(args[1]->IsObject());
@@ -565,9 +640,9 @@ class Parser : public AsyncWrap, public StreamListener {
       lenient_flags = args[3].As<Int32>()->Value();
     }
 
-    if (args.Length() > 4) {
-      CHECK(args[4]->IsInt32());
-      headers_timeout = args[4].As<Int32>()->Value();
+    if (args.Length() > 4 && !args[4]->IsNullOrUndefined()) {
+      CHECK(args[4]->IsObject());
+      ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
     }
 
     llhttp_type_t type =
@@ -586,7 +661,21 @@ class Parser : public AsyncWrap, public StreamListener {
 
     parser->set_provider_type(provider);
     parser->AsyncReset(args[1].As<Object>());
-    parser->Init(type, max_http_header_size, lenient_flags, headers_timeout);
+    parser->Init(type, max_http_header_size, lenient_flags);
+
+    if (connectionsList != nullptr) {
+      parser->connectionsList_ = connectionsList;
+
+      parser->connectionsList_->Push(parser);
+
+      // This protects from a DoS attack where an attacker establishes
+      // the connection without sending any data on applications where
+      // server.timeout is left to the default value of zero.
+      parser->last_message_start_ = uv_hrtime();
+      parser->connectionsList_->PushActive(parser);
+    } else {
+      parser->connectionsList_ = nullptr;
+    }
   }
 
   template <bool should_pause>
@@ -644,6 +733,26 @@ class Parser : public AsyncWrap, public StreamListener {
     args.GetReturnValue().Set(ret);
   }
 
+  static void Duration(const FunctionCallbackInfo<Value>& args) {
+    Parser* parser;
+    ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
+
+    if (parser->last_message_start_ == 0) {
+      args.GetReturnValue().Set(0);
+      return;
+    }
+
+    double duration = (uv_hrtime() - parser->last_message_start_) / 1e6;
+    args.GetReturnValue().Set(duration);
+  }
+
+  static void HeadersCompleted(const FunctionCallbackInfo<Value>& args) {
+    Parser* parser;
+    ASSIGN_OR_RETURN_UNWRAP(&parser, args.Holder());
+
+    args.GetReturnValue().Set(parser->headers_completed_);
+  }
+
  protected:
   static const size_t kAllocBufferSize = 64 * 1024;
 
@@ -690,24 +799,6 @@ class Parser : public AsyncWrap, public StreamListener {
     if (ret.IsEmpty())
       return;
 
-    // check header parsing time
-    if (header_parsing_start_time_ != 0 && headers_timeout_ != 0) {
-      uint64_t now = uv_hrtime();
-      uint64_t parsing_time = (now - header_parsing_start_time_) / 1000000;
-
-      if (parsing_time > headers_timeout_) {
-        Local<Value> cb =
-            object()->Get(env()->context(), kOnTimeout).ToLocalChecked();
-
-        if (!cb->IsFunction())
-          return;
-
-        MakeCallback(cb.As<Function>(), 0, nullptr);
-
-        return;
-      }
-    }
-
     Local<Value> cb =
         object()->Get(env()->context(), kOnExecute).ToLocalChecked();
 
@@ -853,7 +944,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
 
   void Init(llhttp_type_t type, uint64_t max_http_header_size,
-            uint32_t lenient_flags, uint64_t headers_timeout) {
+            uint32_t lenient_flags) {
     llhttp_init(&parser_, type, &settings);
 
     if (lenient_flags & kLenientHeaders) {
@@ -873,9 +964,8 @@ class Parser : public AsyncWrap, public StreamListener {
     num_values_ = 0;
     have_flushed_ = false;
     got_exception_ = false;
+    headers_completed_ = false;
     max_http_header_size_ = max_http_header_size;
-    header_parsing_start_time_ = 0;
-    headers_timeout_ = headers_timeout;
   }
 
 
@@ -923,11 +1013,12 @@ class Parser : public AsyncWrap, public StreamListener {
   size_t current_buffer_len_;
   const char* current_buffer_data_;
   unsigned int execute_depth_ = 0;
+  bool headers_completed_ = false;
   bool pending_pause_ = false;
   uint64_t header_nread_ = 0;
   uint64_t max_http_header_size_;
-  uint64_t headers_timeout_;
-  uint64_t header_parsing_start_time_ = 0;
+  uint64_t last_message_start_;
+  ConnectionsList* connectionsList_;
 
   BaseObjectPtr<BindingData> binding_data_;
 
@@ -952,6 +1043,135 @@ class Parser : public AsyncWrap, public StreamListener {
   static const llhttp_settings_t settings;
 };
 
+bool ParserComparator::operator()(const Parser* lhs, const Parser* rhs) const {
+  if (lhs->last_message_start_ == 0) {
+    return false;
+  } else if (rhs->last_message_start_ == 0) {
+    return true;
+  }
+
+  return lhs->last_message_start_ < rhs->last_message_start_;
+}
+
+void ConnectionsList::New(const FunctionCallbackInfo<Value>& args) {
+  Local<Context> context = args.GetIsolate()->GetCurrentContext();
+  Environment* env = Environment::GetCurrent(context);
+
+  new ConnectionsList(env, args.This());
+}
+
+void ConnectionsList::All(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+
+  Local<Array> all = Array::New(isolate);
+  ConnectionsList* list;
+
+  ASSIGN_OR_RETURN_UNWRAP(&list, args.Holder());
+
+  uint32_t i = 0;
+  for (auto parser : list->all_connections_) {
+    if (all->Set(context, i++, parser->object()).IsNothing()) {
+      return;
+    }
+  }
+
+  return args.GetReturnValue().Set(all);
+}
+
+void ConnectionsList::Idle(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+
+  Local<Array> idle = Array::New(isolate);
+  ConnectionsList* list;
+
+  ASSIGN_OR_RETURN_UNWRAP(&list, args.Holder());
+
+  uint32_t i = 0;
+  for (auto parser : list->all_connections_) {
+    if (parser->last_message_start_ == 0) {
+      if (idle->Set(context, i++, parser->object()).IsNothing()) {
+        return;
+      }
+    }
+  }
+
+  return args.GetReturnValue().Set(idle);
+}
+
+void ConnectionsList::Active(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+
+  Local<Array> active = Array::New(isolate);
+  ConnectionsList* list;
+
+  ASSIGN_OR_RETURN_UNWRAP(&list, args.Holder());
+
+  uint32_t i = 0;
+  for (auto parser : list->active_connections_) {
+    if (active->Set(context, i++, parser->object()).IsNothing()) {
+      return;
+    }
+  }
+
+  return args.GetReturnValue().Set(active);
+}
+
+void ConnectionsList::Expired(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+
+  Local<Array> expired = Array::New(isolate);
+  ConnectionsList* list;
+
+  ASSIGN_OR_RETURN_UNWRAP(&list, args.Holder());
+  CHECK(args[0]->IsNumber());
+  CHECK(args[1]->IsNumber());
+  uint64_t headers_timeout =
+    static_cast<uint64_t>(args[0].As<Uint32>()->Value()) * 1000000;
+  uint64_t request_timeout =
+    static_cast<uint64_t>(args[1].As<Uint32>()->Value()) * 1000000;
+
+  if (headers_timeout == 0 && request_timeout == 0) {
+    return args.GetReturnValue().Set(expired);
+  } else if (request_timeout > 0 && headers_timeout > request_timeout) {
+    std::swap(headers_timeout, request_timeout);
+  }
+
+  const uint64_t now = uv_hrtime();
+  const uint64_t headers_deadline =
+    headers_timeout > 0 ? now - headers_timeout : 0;
+  const uint64_t request_deadline =
+    request_timeout > 0 ? now - request_timeout : 0;
+
+  uint32_t i = 0;
+  auto iter = list->active_connections_.begin();
+  auto end = list->active_connections_.end();
+  while (iter != end) {
+    Parser* parser = *iter;
+    iter++;
+
+    // Check for expiration.
+    if (
+      (!parser->headers_completed_ && headers_deadline > 0 &&
+        parser->last_message_start_ < headers_deadline) ||
+      (
+        request_deadline > 0 &&
+        parser->last_message_start_ < request_deadline)
+    ) {
+      if (expired->Set(context, i++, parser->object()).IsNothing()) {
+        return;
+      }
+
+      list->active_connections_.erase(parser);
+    }
+  }
+
+  return args.GetReturnValue().Set(expired);
+}
+
 const llhttp_settings_t Parser::settings = {
   Proxy<Call, &Parser::on_message_begin>::Raw,
   Proxy<DataCall, &Parser::on_url>::Raw,
@@ -1038,8 +1258,19 @@ void InitializeHttpParser(Local<Object> target,
   env->SetProtoMethod(t, "consume", Parser::Consume);
   env->SetProtoMethod(t, "unconsume", Parser::Unconsume);
   env->SetProtoMethod(t, "getCurrentBuffer", Parser::GetCurrentBuffer);
+  env->SetProtoMethod(t, "duration", Parser::Duration);
+  env->SetProtoMethod(t, "headersCompleted", Parser::HeadersCompleted);
 
   env->SetConstructorFunction(target, "HTTPParser", t);
+
+  Local<FunctionTemplate> c = env->NewFunctionTemplate(ConnectionsList::New);
+  c->InstanceTemplate()
+    ->SetInternalFieldCount(ConnectionsList::kInternalFieldCount);
+  env->SetProtoMethod(c, "all", ConnectionsList::All);
+  env->SetProtoMethod(c, "idle", ConnectionsList::Idle);
+  env->SetProtoMethod(c, "active", ConnectionsList::Active);
+  env->SetProtoMethod(c, "expired", ConnectionsList::Expired);
+  env->SetConstructorFunction(target, "ConnectionsList", c);
 }
 
 }  // anonymous namespace

test/async-hooks/test-graph.http.js

@@ -44,7 +44,7 @@ process.on('exit', () => {
         triggerAsyncId: 'tcp:2' },
       { type: 'Timeout',
         id: 'timeout:1',
-        triggerAsyncId: 'httpincomingmessage:1' },
+        triggerAsyncId: null },
       { type: 'SHUTDOWNWRAP',
         id: 'shutdown:1',
         triggerAsyncId: 'tcp:2' } ]

test/parallel/test-http-parser-timeout-reset.js

@@ -27,7 +27,6 @@ const server = net.createServer((socket) => {
     {},
     0,
     0,
-    1e3
   );
 
   parser[HTTPParser.kOnTimeout] = common.mustNotCall();

test/parallel/test-http-server-headers-timeout-delayed-headers.js

@@ -0,0 +1,61 @@
+'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.headersTimeout if the client
+// pauses before start sending the request.
+
+let sendDelayedRequestHeaders;
+const headersTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout,
+  requestTimeout: 0,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustNotCall());
+server.on('connection', common.mustCall(() => {
+  assert.strictEqual(typeof sendDelayedRequestHeaders, 'function');
+  sendDelayedRequestHeaders();
+}));
+
+assert.strictEqual(server.headersTimeout, headersTimeout);
+
+// Check that timeout event is not triggered
+server.once('timeout', common.mustNotCall((socket) => {
+  socket.destroy();
+}));
+
+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.mustSucceed(function(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();
+
+  sendDelayedRequestHeaders = common.mustCall(() => {
+    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(headersTimeout * 2)).unref();
+  });
+}));

test/parallel/test-http-server-headers-timeout-interrupted-headers.js

@@ -0,0 +1,61 @@
+'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.headersTimeout if the client
+// pauses sending in the middle of a header.
+
+let sendDelayedRequestHeaders;
+const headersTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout,
+  requestTimeout: 0,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustNotCall());
+server.on('connection', common.mustCall(() => {
+  assert.strictEqual(typeof sendDelayedRequestHeaders, 'function');
+  sendDelayedRequestHeaders();
+}));
+
+assert.strictEqual(server.headersTimeout, headersTimeout);
+
+// Check that timeout event is not triggered
+server.once('timeout', common.mustNotCall((socket) => {
+  socket.destroy();
+}));
+
+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.mustSucceed(function(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: ');
+
+  sendDelayedRequestHeaders = common.mustCall(() => {
+    setTimeout(() => {
+      client.write('1234567890\r\n\r\n');
+    }, common.platformTimeout(headersTimeout * 2)).unref();
+  });
+}));

test/parallel/test-http-server-headers-timeout-keepalive.js

@@ -0,0 +1,103 @@
+'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.headersTimeout if the client
+// does not send headers before the timeout, and
+// that keep alive works properly.
+
+function performRequestWithDelay(client, firstDelay, secondDelay, closeAfter) {
+  client.resume();
+  client.write('GET / HTTP/1.1\r\n');
+
+  firstDelay = common.platformTimeout(firstDelay);
+  secondDelay = common.platformTimeout(secondDelay);
+
+  setTimeout(() => {
+    client.write('Connection: ');
+  }, firstDelay).unref();
+
+  // Complete the request
+  setTimeout(() => {
+    client.write(`${closeAfter ? 'close' : 'keep-alive'}\r\n\r\n`);
+  }, firstDelay + secondDelay).unref();
+}
+
+const headersTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout,
+  requestTimeout: 0,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustCallAtLeast((req, res) => {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end();
+}));
+
+assert.strictEqual(server.headersTimeout, headersTimeout);
+
+// Check that timeout event is not triggered
+server.once('timeout', common.mustNotCall((socket) => {
+  socket.destroy();
+}));
+
+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(headersTimeout * 1.5);
+
+      // Wait some time to make sure headersTimeout
+      // does not interfere with keep alive
+      setTimeout(() => {
+        response = '';
+        second = true;
+
+        // Perform a second request expected to finish after headersTimeout
+        performRequestWithDelay(
+          client,
+          headersTimeout / 5,
+          headersTimeout,
+          true
+        );
+      }, defer).unref();
+    }
+  }, 1));
+
+  const errOrEnd = common.mustCall(function(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 headersTimeout
+  performRequestWithDelay(
+    client,
+    headersTimeout / 5,
+    headersTimeout / 5,
+    false
+  );
+}));

test/parallel/test-http-server-headers-timeout-pipelining.js

@@ -0,0 +1,76 @@
+'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 when using pipelining.
+
+const headersTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout,
+  requestTimeout: 0,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustCallAtLeast((req, res) => {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end();
+}));
+
+// 0 seconds is the default
+assert.strictEqual(server.headersTimeout, headersTimeout);
+
+// Make sure requestTimeout and keepAliveTimeout
+// are big enough for the headersTimeout.
+server.requestTimeout = 0;
+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'
+      );
+
+      response = '';
+      second = true;
+    }
+  }, 1));
+
+  const errOrEnd = common.mustCall(function(err) {
+    if (!second) {
+      return;
+    }
+
+    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);
+
+  // Send two requests using pipelining. Delay before finishing the second one
+  client.resume();
+  client.write('GET / HTTP/1.1\r\nConnection: keep-alive\r\n\r\nGET / HTTP/1.1\r\nConnection: ');
+
+  // Complete the request
+  setTimeout(() => {
+    client.write('close\r\n\r\n');
+  }, headersTimeout * 1.5).unref();
+}));

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

@@ -10,7 +10,13 @@ const { connect } = require('net');
 // pauses before start sending the body.
 
 let sendDelayedRequestBody;
-const server = createServer(common.mustCall((req, res) => {
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustCall((req, res) => {
   let body = '';
   req.setEncoding('utf-8');
 
@@ -28,10 +34,6 @@ const server = createServer(common.mustCall((req, res) => {
   sendDelayedRequestBody();
 }));
 
-// 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(() => {
@@ -51,11 +53,10 @@ server.listen(0, common.mustCall(() => {
   sendDelayedRequestBody = common.mustCall(() => {
     setTimeout(() => {
       client.write('12345678901234567890\r\n\r\n');
-    }, common.platformTimeout(2000)).unref();
+    }, common.platformTimeout(requestTimeout * 2)).unref();
   });
 
-  const errOrEnd = common.mustCall(function(err) {
-    console.log(err);
+  const errOrEnd = common.mustSucceed(function(err) {
     assert.strictEqual(
       response,
       'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'

test/parallel/test-http-server-request-timeout-delayed-headers.js

@@ -8,16 +8,20 @@ const { connect } = require('net');
 // This test validates that the server returns 408
 // after server.requestTimeout if the client
 // pauses before start sending the request.
+
 let sendDelayedRequestHeaders;
-const server = createServer(common.mustNotCall());
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustNotCall());
 server.on('connection', common.mustCall(() => {
   assert.strictEqual(typeof sendDelayedRequestHeaders, 'function');
   sendDelayedRequestHeaders();
 }));
-// 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(() => {
@@ -28,8 +32,7 @@ server.listen(0, common.mustCall(() => {
     response += chunk.toString('utf-8');
   }));
 
-  const errOrEnd = common.mustCall(function(err) {
-    console.log(err);
+  const errOrEnd = common.mustSucceed(function(err) {
     assert.strictEqual(
       response,
       'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
@@ -48,6 +51,6 @@ server.listen(0, common.mustCall(() => {
       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();
+    }, common.platformTimeout(requestTimeout * 2)).unref();
   });
 }));

test/parallel/test-http-server-request-timeout-interrupted-body.js

@@ -8,8 +8,15 @@ 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.
+
 let sendDelayedRequestBody;
-const server = createServer(common.mustCall((req, res) => {
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustCall((req, res) => {
   let body = '';
   req.setEncoding('utf-8');
 
@@ -27,10 +34,6 @@ const server = createServer(common.mustCall((req, res) => {
   sendDelayedRequestBody();
 }));
 
-// 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(() => {
@@ -41,8 +44,7 @@ server.listen(0, common.mustCall(() => {
     response += chunk.toString('utf-8');
   }));
 
-  const errOrEnd = common.mustCall(function(err) {
-    console.log(err);
+  const errOrEnd = common.mustSucceed(function(err) {
     assert.strictEqual(
       response,
       'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
@@ -63,6 +65,6 @@ server.listen(0, common.mustCall(() => {
   sendDelayedRequestBody = common.mustCall(() => {
     setTimeout(() => {
       client.write('1234567890\r\n\r\n');
-    }, common.platformTimeout(2000)).unref();
+    }, common.platformTimeout(requestTimeout * 2)).unref();
   });
 }));

test/parallel/test-http-server-request-timeout-interrupted-headers.js

@@ -8,17 +8,20 @@ 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.
+
 let sendDelayedRequestHeaders;
-const server = createServer(common.mustNotCall());
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustNotCall());
 server.on('connection', common.mustCall(() => {
   assert.strictEqual(typeof sendDelayedRequestHeaders, 'function');
   sendDelayedRequestHeaders();
 }));
 
-// 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(() => {
@@ -29,8 +32,7 @@ server.listen(0, common.mustCall(() => {
     response += chunk.toString('utf-8');
   }));
 
-  const errOrEnd = common.mustCall(function(err) {
-    console.log(err);
+  const errOrEnd = common.mustSucceed(function(err) {
     assert.strictEqual(
       response,
       'HTTP/1.1 408 Request Timeout\r\nConnection: close\r\n\r\n'
@@ -49,6 +51,6 @@ server.listen(0, common.mustCall(() => {
   sendDelayedRequestHeaders = common.mustCall(() => {
     setTimeout(() => {
       client.write('1234567890\r\n\r\n');
-    }, common.platformTimeout(2000)).unref();
+    }, common.platformTimeout(requestTimeout * 2)).unref();
   });
 }));

test/parallel/test-http-server-request-timeout-keepalive.js

@@ -8,36 +8,36 @@ 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
+// works properly.
 
-function performRequestWithDelay(client, firstDelay, secondDelay) {
+function performRequestWithDelay(client, firstDelay, secondDelay, closeAfter) {
   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');
+    client.write(`${closeAfter ? 'close' : 'keep-alive'}\r\n\r\n`);
   }, firstDelay + secondDelay).unref();
 }
 
-const server = createServer(common.mustCallAtLeast((req, res) => {
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, 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.
@@ -58,9 +58,7 @@ server.listen(0, common.mustCall(() => {
         'HTTP/1.1 200 OK'
       );
 
-      const defer = common.platformTimeout(server.requestTimeout * 1.5);
-
-      console.log('defer by', defer);
+      const defer = common.platformTimeout(requestTimeout * 1.5);
 
       // Wait some time to make sure requestTimeout
       // does not interfere with keep alive
@@ -69,13 +67,17 @@ server.listen(0, common.mustCall(() => {
         second = true;
 
         // Perform a second request expected to finish after requestTimeout
-        performRequestWithDelay(client, 1000, 3000);
+        performRequestWithDelay(
+          client,
+          requestTimeout / 5,
+          requestTimeout,
+          true
+        );
       }, defer).unref();
     }
   }, 1));
 
   const errOrEnd = common.mustCall(function(err) {
-    console.log(err);
     assert.strictEqual(second, true);
     assert.strictEqual(
       response,
@@ -90,5 +92,10 @@ server.listen(0, common.mustCall(() => {
   client.on('end', errOrEnd);
 
   // Perform a second request expected to finish before requestTimeout
-  performRequestWithDelay(client, 50, 500);
+  performRequestWithDelay(
+    client,
+    requestTimeout / 5,
+    requestTimeout / 5,
+    false
+  );
 }));

test/parallel/test-http-server-request-timeout-pipelining.js

@@ -0,0 +1,70 @@
+'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 when using pipelining.
+
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustCallAtLeast((req, res) => {
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+  res.end();
+}));
+
+assert.strictEqual(server.requestTimeout, requestTimeout);
+
+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'
+      );
+
+      response = '';
+      second = true;
+    }
+  }, 1));
+
+  const errOrEnd = common.mustCall(function(err) {
+    if (!second) {
+      return;
+    }
+
+    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);
+
+  // Send two requests using pipelining. Delay before finishing the second one
+  client.resume();
+  client.write('GET / HTTP/1.1\r\nConnection: keep-alive\r\n\r\nGET / HTTP/1.1\r\nConnection: ');
+
+  // Complete the request
+  setTimeout(() => {
+    client.write('close\r\n\r\n');
+  }, requestTimeout * 1.5).unref();
+}));

test/parallel/test-http-server-request-timeout-upgrade.js

@@ -8,16 +8,18 @@ const { connect } = require('net');
 // This test validates that the requestTimeoout
 // is disabled after the connection is upgraded.
 let sendDelayedRequestHeaders;
-const server = createServer(common.mustNotCall());
+const requestTimeout = common.platformTimeout(1000);
+const server = createServer({
+  headersTimeout: 0,
+  requestTimeout,
+  keepAliveTimeout: 0,
+  connectionsCheckingInterval: common.platformTimeout(250),
+}, common.mustNotCall());
 server.on('connection', common.mustCall(() => {
   assert.strictEqual(typeof sendDelayedRequestHeaders, 'function');
   sendDelayedRequestHeaders();
 }));
 
-// 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) => {

test/parallel/test-http-slow-headers-keepalive-multiple-requests.js

@@ -1,51 +0,0 @@
-'use strict';
-
-const common = require('../common');
-const http = require('http');
-const net = require('net');
-const { finished } = require('stream');
-
-const headers =
-  'GET / HTTP/1.1\r\n' +
-  'Host: localhost\r\n' +
-  'Connection: keep-alive\r\n' +
-  'Agent: node\r\n';
-
-const baseTimeout = 1000;
-
-const server = http.createServer(common.mustCall((req, res) => {
-  req.resume();
-  res.writeHead(200);
-  res.end();
-}, 2));
-
-server.keepAliveTimeout = 10 * baseTimeout;
-server.headersTimeout = baseTimeout;
-
-server.once('timeout', common.mustNotCall((socket) => {
-  socket.destroy();
-}));
-
-server.listen(0, () => {
-  const client = net.connect(server.address().port);
-
-  // first request
-  client.write(headers);
-  client.write('\r\n');
-
-  setTimeout(() => {
-    // second request
-    client.write(headers);
-    // `headersTimeout` doesn't seem to fire if request
-    // is sent altogether.
-    setTimeout(() => {
-      client.write('\r\n');
-      client.end();
-    }, 10);
-  }, baseTimeout + 10);
-
-  client.resume();
-  finished(client, common.mustCall((err) => {
-    server.close();
-  }));
-});

test/parallel/test-http-slow-headers-keepalive.js

@@ -1,51 +0,0 @@
-'use strict';
-
-const common = require('../common');
-const http = require('http');
-const net = require('net');
-const { finished } = require('stream');
-
-const headers =
-  'GET / HTTP/1.1\r\n' +
-  'Host: localhost\r\n' +
-  'Connection: keep-alive\r\n' +
-  'Agent: node\r\n';
-
-let sendCharEvery = 1000;
-
-const server = http.createServer(common.mustCall((req, res) => {
-  res.writeHead(200);
-  res.end();
-}));
-
-// Pass a REAL env variable to shortening up the default
-// value which is 40s otherwise this is useful for manual
-// testing
-if (!process.env.REAL) {
-  sendCharEvery = common.platformTimeout(10);
-  server.headersTimeout = 2 * sendCharEvery;
-}
-
-server.once('timeout', common.mustCall((socket) => {
-  socket.destroy();
-}));
-
-server.listen(0, () => {
-  const client = net.connect(server.address().port);
-  client.write(headers);
-  // Finish the first request
-  client.write('\r\n');
-  // second request
-  client.write(headers);
-  client.write('X-CRASH: ');
-
-  const interval = setInterval(() => {
-    client.write('a');
-  }, sendCharEvery);
-
-  client.resume();
-  finished(client, common.mustCall((err) => {
-    clearInterval(interval);
-    server.close();
-  }));
-});

test/parallel/test-http-slow-headers.js

@@ -1,50 +0,0 @@
-'use strict';
-
-const common = require('../common');
-const assert = require('assert');
-const { createServer } = require('http');
-const { connect } = require('net');
-const { finished } = require('stream');
-
-// This test validates that the 'timeout' event fires
-// after server.headersTimeout.
-
-const headers =
-  'GET / HTTP/1.1\r\n' +
-  'Host: localhost\r\n' +
-  'Agent: node\r\n';
-
-const server = createServer(common.mustNotCall());
-let sendCharEvery = 1000;
-
-// 60 seconds is the default
-assert.strictEqual(server.headersTimeout, 60 * 1000);
-
-// Pass a REAL env variable to shortening up the default
-// value which is 40s otherwise this is useful for manual
-// testing
-if (!process.env.REAL) {
-  sendCharEvery = common.platformTimeout(10);
-  server.headersTimeout = 2 * sendCharEvery;
-}
-
-server.once('timeout', common.mustCall((socket) => {
-  socket.destroy();
-}));
-
-server.listen(0, common.mustCall(() => {
-  const client = connect(server.address().port);
-  client.write(headers);
-  client.write('X-CRASH: ');
-
-  const interval = setInterval(() => {
-    client.write('a');
-  }, sendCharEvery);
-
-  client.resume();
-
-  finished(client, common.mustCall((err) => {
-    clearInterval(interval);
-    server.close();
-  }));
-}));

test/parallel/test-https-server-headers-timeout.js

@@ -0,0 +1,21 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const { createServer } = require('https');
+const fixtures = require('../common/fixtures');
+
+const options = {
+  key: fixtures.readKey('agent1-key.pem'),
+  cert: fixtures.readKey('agent1-cert.pem'),
+};
+
+const server = createServer(options);
+
+// 60000 seconds is the default
+assert.strictEqual(server.headersTimeout, 60000);
+const headersTimeout = common.platformTimeout(1000);
+server.headersTimeout = headersTimeout;
+assert.strictEqual(server.headersTimeout, headersTimeout);

test/parallel/test-https-server-request-timeout.js

@@ -14,8 +14,8 @@ const options = {
 
 const server = createServer(options);
 
-// 0 seconds is the default
-assert.strictEqual(server.requestTimeout, 0);
+// 300 seconds is the default
+assert.strictEqual(server.requestTimeout, 300000);
 const requestTimeout = common.platformTimeout(1000);
 server.requestTimeout = requestTimeout;
 assert.strictEqual(server.requestTimeout, requestTimeout);

test/parallel/test-https-slow-headers.js

@@ -1,63 +0,0 @@
-'use strict';
-
-const common = require('../common');
-const { readKey } = require('../common/fixtures');
-
-if (!common.hasCrypto)
-  common.skip('missing crypto');
-
-const assert = require('assert');
-const { createServer } = require('https');
-const { connect } = require('tls');
-const { finished } = require('stream');
-
-// This test validates that the 'timeout' event fires
-// after server.headersTimeout.
-
-const headers =
-  'GET / HTTP/1.1\r\n' +
-  'Host: localhost\r\n' +
-  'Agent: node\r\n';
-
-const server = createServer({
-  key: readKey('agent1-key.pem'),
-  cert: readKey('agent1-cert.pem'),
-  ca: readKey('ca1-cert.pem'),
-}, common.mustNotCall());
-
-let sendCharEvery = 1000;
-
-// 60 seconds is the default
-assert.strictEqual(server.headersTimeout, 60 * 1000);
-
-// Pass a REAL env variable to shortening up the default
-// value which is 40s otherwise
-// this is useful for manual testing
-if (!process.env.REAL) {
-  sendCharEvery = common.platformTimeout(10);
-  server.headersTimeout = 2 * sendCharEvery;
-}
-
-server.once('timeout', common.mustCall((socket) => {
-  socket.destroy();
-}));
-
-server.listen(0, common.mustCall(() => {
-  const client = connect({
-    port: server.address().port,
-    rejectUnauthorized: false
-  });
-  client.write(headers);
-  client.write('X-CRASH: ');
-
-  const interval = setInterval(() => {
-    client.write('a');
-  }, sendCharEvery);
-
-  client.resume();
-
-  finished(client, common.mustCall((err) => {
-    clearInterval(interval);
-    server.close();
-  }));
-}));