http: refactor headersTimeout and requestTimeout logic

PR-URL: #41263
Fixes: #33440
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>

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,24 +32,24 @@ 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'
   };
 
   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,