From e4921d4ff39cc4c38d03e1129ec63e3c41f62e11 Mon Sep 17 00:00:00 2001
From: Ali Ijaz Sheikh <ofrobots@google.com>
Date: Tue, 14 May 2019 14:50:37 -0700
Subject: [PATCH] http, http2: flag for overriding server timeout

Make it possible to override the default http server timeout. Ideally
there should be no server timeout - as done on the master branch. This
is a non-breaking way to enable platform providers to override the
value.

PR-URL: https://github.com/nodejs/node/pull/27704
Refs: https://github.com/nodejs/node/pull/27558
Refs: https://github.com/nodejs/node/issues/27556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
---
 doc/api/cli.md                          | 12 ++++++++
 doc/api/http.md                         |  7 +++++
 doc/api/http2.md                        |  7 +++++
 doc/node.1                              |  3 ++
 lib/_http_server.js                     |  5 ++-
 lib/https.js                            |  6 +++-
 lib/internal/http2/core.js              |  8 +++--
 src/node_options.cc                     |  5 +++
 src/node_options.h                      |  1 +
 test/parallel/test-http-timeout-flag.js | 41 +++++++++++++++++++++++++
 10 files changed, 90 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-http-timeout-flag.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 7563515b9f9b21..c90fe4bd5f5015 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -119,6 +119,17 @@ added: v6.0.0
 Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
 (Same requirements as `--enable-fips`.)
 
+### `--http-server-default-timeout=milliseconds`
+<!-- YAML
+added: REPLACEME
+-->
+
+Overrides the default value of `http`, `https` and `http2` server socket
+timeout. Setting the value to 0 disables server socket timeout. Unless
+provided, http server sockets timeout after 120s (2 minutes). Programmatic
+setting of the timeout takes precedence over the value set through this
+flag.
+
 ### `--icu-data-dir=file`
 <!-- YAML
 added: v0.11.15
@@ -590,6 +601,7 @@ Node.js options that are allowed are:
 - `--experimental-vm-modules`
 - `--experimental-worker`
 - `--force-fips`
+- `--http-server-default-timeout`
 - `--icu-data-dir`
 - `--inspect`
 - `--inspect-brk`
diff --git a/doc/api/http.md b/doc/api/http.md
index d1619597532689..0a73400b1b9e0f 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -1004,6 +1004,9 @@ By default, the Server's timeout value is 2 minutes, and sockets are
 destroyed automatically if they time out. However, if a callback is assigned
 to the Server's `'timeout'` event, timeouts must be handled explicitly.
 
+To change the default timeout use the [`--http-server-default-timeout`][]
+flag.
+
 ### server.timeout
 <!-- YAML
 added: v0.9.12
@@ -1019,6 +1022,9 @@ A value of `0` will disable the timeout behavior on incoming connections.
 The socket timeout logic is set up on connection, so changing this
 value only affects new connections to the server, not any existing connections.
 
+To change the default timeout use the [`--http-server-default-timeout`][]
+flag.
+
 ### server.keepAliveTimeout
 <!-- YAML
 added: v8.0.0
@@ -2113,6 +2119,7 @@ will be emitted in the following order:
 Note that setting the `timeout` option or using the `setTimeout()` function will
 not abort the request or do anything besides add a `'timeout'` event.
 
+[`--http-server-default-timeout`]: cli.html#cli_http_server_default_timeout_milliseconds
 [`--max-http-header-size`]: cli.html#cli_max_http_header_size_size
 [`'checkContinue'`]: #http_event_checkcontinue
 [`'request'`]: #http_event_request
diff --git a/doc/api/http2.md b/doc/api/http2.md
index 120cc2715f7981..6e8789561bff32 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -1723,6 +1723,9 @@ The `'timeout'` event is emitted when there is no activity on the Server for
 a given number of milliseconds set using `http2server.setTimeout()`.
 **Default:** 2 minutes.
 
+To change the default timeout use the [`--http-server-default-timeout`][]
+flag.
+
 #### server.close([callback])
 <!-- YAML
 added: v8.4.0
@@ -1753,6 +1756,9 @@ The given callback is registered as a listener on the `'timeout'` event.
 In case of no callback function were assigned, a new `ERR_INVALID_CALLBACK`
 error will be thrown.
 
+To change the default timeout use the [`--http-server-default-timeout`][]
+flag.
+
 ### Class: Http2SecureServer
 <!-- YAML
 added: v8.4.0
@@ -3411,6 +3417,7 @@ following additional properties:
 * `type` {string} Either `'server'` or `'client'` to identify the type of
   `Http2Session`.
 
+[`--http-server-default-timeout`]: cli.html#cli_http_server_default_timeout_milliseconds
 [ALPN Protocol ID]: https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids
 [ALPN negotiation]: #http2_alpn_negotiation
 [Compatibility API]: #http2_compatibility_api
diff --git a/doc/node.1 b/doc/node.1
index 61bd4925d54502..a5557d60a794cd 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -109,6 +109,9 @@ Chooses an HTTP parser library. Available values are
 or
 .Sy legacy .
 .
+.It Fl -http-server-default-timeout Ns = Ns Ar milliseconds
+Overrides the default value for server socket timeout.
+.
 .It Fl -icu-data-dir Ns = Ns Ar file
 Specify ICU data load path.
 Overrides
diff --git a/lib/_http_server.js b/lib/_http_server.js
index f8aacb40452c8a..6d602d98c2a188 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -50,8 +50,11 @@ const {
   ERR_INVALID_CHAR
 } = require('internal/errors').codes;
 const Buffer = require('buffer').Buffer;
+const { getOptionValue } = require('internal/options');
 
 const kServerResponse = Symbol('ServerResponse');
+const kDefaultHttpServerTimeout =
+  getOptionValue('--http-server-default-timeout');
 
 const STATUS_CODES = {
   100: 'Continue',
@@ -300,7 +303,7 @@ function Server(options, requestListener) {
 
   this.on('connection', connectionListener);
 
-  this.timeout = 2 * 60 * 1000;
+  this.timeout = kDefaultHttpServerTimeout;
   this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.headersTimeout = 40 * 1000; // 40 seconds
diff --git a/lib/https.js b/lib/https.js
index 67a4648e3b70a2..38468ed9d464bf 100644
--- a/lib/https.js
+++ b/lib/https.js
@@ -39,6 +39,10 @@ const { urlToOptions, searchParamsSymbol } = require('internal/url');
 const { ERR_INVALID_DOMAIN_NAME } = require('internal/errors').codes;
 const { IncomingMessage, ServerResponse } = require('http');
 const { kIncomingMessage } = require('_http_common');
+const { getOptionValue } = require('internal/options');
+
+const kDefaultHttpServerTimeout =
+  getOptionValue('--http-server-default-timeout');
 
 function Server(opts, requestListener) {
   if (!(this instanceof Server)) return new Server(opts, requestListener);
@@ -72,7 +76,7 @@ function Server(opts, requestListener) {
       conn.destroy(err);
   });
 
-  this.timeout = 2 * 60 * 1000;
+  this.timeout = kDefaultHttpServerTimeout;
   this.keepAliveTimeout = 5000;
   this.maxHeadersCount = null;
   this.headersTimeout = 40 * 1000; // 40 seconds
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index a57e63c235398a..6ccc130efb3d6c 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -126,6 +126,7 @@ const { UV_EOF } = process.binding('uv');
 const { StreamPipe } = internalBinding('stream_pipe');
 const { _connectionListener: httpConnectionListener } = http;
 const debug = util.debuglog('http2');
+const { getOptionValue } = require('internal/options');
 
 const kMaxFrameSize = (2 ** 24) - 1;
 const kMaxInt = (2 ** 32) - 1;
@@ -162,7 +163,8 @@ const kState = Symbol('state');
 const kType = Symbol('type');
 const kWriteGeneric = Symbol('write-generic');
 
-const kDefaultSocketTimeout = 2 * 60 * 1000;
+const kDefaultHttpServerTimeout =
+  getOptionValue('--http-server-default-timeout');
 
 const {
   paddingBuffer,
@@ -2726,7 +2728,7 @@ class Http2SecureServer extends TLSServer {
     options = initializeTLSOptions(options);
     super(options, connectionListener);
     this[kOptions] = options;
-    this.timeout = kDefaultSocketTimeout;
+    this.timeout = kDefaultHttpServerTimeout;
     this.on('newListener', setupCompat);
     if (typeof requestListener === 'function')
       this.on('request', requestListener);
@@ -2748,7 +2750,7 @@ class Http2Server extends NETServer {
   constructor(options, requestListener) {
     super(connectionListener);
     this[kOptions] = initializeOptions(options);
-    this.timeout = kDefaultSocketTimeout;
+    this.timeout = kDefaultHttpServerTimeout;
     this.on('newListener', setupCompat);
     if (typeof requestListener === 'function')
       this.on('request', requestListener);
diff --git a/src/node_options.cc b/src/node_options.cc
index 79dff7adbf9d63..d8fbcb1947faa1 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -102,6 +102,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_worker,
             kAllowedInEnvironment);
   AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals);
+  AddOption("--http-server-default-timeout",
+            "Default http server socket timeout in ms "
+            "(default: 120000)",
+            &EnvironmentOptions::http_server_default_timeout,
+            kAllowedInEnvironment);
   AddOption("--loader",
             "(with --experimental-modules) use the specified file as a "
             "custom loader",
diff --git a/src/node_options.h b/src/node_options.h
index 0345e0869aae56..b7578abddcc8b3 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -72,6 +72,7 @@ class EnvironmentOptions : public Options {
   bool experimental_vm_modules = false;
   bool experimental_worker = false;
   bool expose_internals = false;
+  uint64_t http_server_default_timeout = 120000;
   bool no_deprecation = false;
   bool no_force_async_hooks_checks = false;
   bool no_warnings = false;
diff --git a/test/parallel/test-http-timeout-flag.js b/test/parallel/test-http-timeout-flag.js
new file mode 100644
index 00000000000000..f816ce2f0a4f4b
--- /dev/null
+++ b/test/parallel/test-http-timeout-flag.js
@@ -0,0 +1,41 @@
+'use strict';
+const common = require('../common');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const fixtures = require('../common/fixtures');
+const http = require('http');
+const https = require('https');
+const http2 = require('http2');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+// Make sure the defaults are correct.
+const servers = [
+  http.createServer(),
+  https.createServer({
+    key: fixtures.readKey('agent1-key.pem'),
+    cert: fixtures.readKey('agent1-cert.pem')
+  }),
+  http2.createServer()
+];
+
+for (const server of servers) {
+  assert.strictEqual(server.timeout, 120000);
+  server.close();
+}
+
+// Ensure that command line flag overrides the default timeout.
+const child1 = spawnSync(process.execPath, ['--http-server-default-timeout=10',
+                                            '-p', 'http.createServer().timeout'
+]);
+assert.strictEqual(+child1.stdout.toString().trim(), 10);
+
+// Ensure that the flag is whitelisted for NODE_OPTIONS.
+const env = Object.assign({}, process.env, {
+  NODE_OPTIONS: '--http-server-default-timeout=10'
+});
+const child2 = spawnSync(process.execPath,
+                         [ '-p', 'http.createServer().timeout'], { env });
+assert.strictEqual(+child2.stdout.toString().trim(), 10);