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>

4 files changed, 72 insertions, 80 deletions

diff --git a/lib/_http_client.js b/lib/_http_client.js
index 42b4683d141..a4f7a255a99 100644
--- a/lib/_http_client.js
+++ b/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;
diff --git a/lib/_http_common.js b/lib/_http_common.js
index 796deeff055..2055ca205b8 100644
--- a/lib/_http_common.js
+++ b/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,
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 00c810e51df..84be32c78c4 100644
--- a/lib/_http_server.js
+++ b/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
diff --git a/lib/https.js b/lib/https.js
index 3834a881775..74ce93baaaf 100644
--- a/lib/https.js
+++ b/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);