Skip to content

Commit

Permalink
http, http2: flag for overriding server timeout
Browse files Browse the repository at this point in the history
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.

Ref: #27558
Ref: #27556

PR-URL: #27704
Refs: #27558
Refs: #27556
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
ofrobots committed May 23, 2019
1 parent c0cf173 commit 588fd0c
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 5 deletions.
12 changes: 12 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,17 @@ This flag exists to aid in experimentation with the internal implementation of
the Node.js http parser.
This flag is likely to become a no-op and removed at some point in the future.

### `--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
Expand Down Expand Up @@ -917,6 +928,7 @@ Node.js options that are allowed are:
- `--force-fips`
- `--frozen-intrinsics`
- `--heapsnapshot-signal`
- `--http-server-default-timeout`
- `--icu-data-dir`
- `--inspect`
- `--inspect-brk`
Expand Down
7 changes: 7 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,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
Expand All @@ -1045,6 +1048,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
Expand Down Expand Up @@ -2150,6 +2156,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
Expand Down
7 changes: 7 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -1728,6 +1728,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
Expand Down Expand Up @@ -1758,6 +1761,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
Expand Down Expand Up @@ -3426,6 +3432,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
Expand Down
3 changes: 3 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,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
Expand Down
5 changes: 4 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ const {
DTRACE_HTTP_SERVER_REQUEST,
DTRACE_HTTP_SERVER_RESPONSE
} = require('internal/dtrace');
const { getOptionValue } = require('internal/options');

const kServerResponse = Symbol('ServerResponse');
const kDefaultHttpServerTimeout =
getOptionValue('--http-server-default-timeout');

const STATUS_CODES = {
100: 'Continue',
Expand Down Expand Up @@ -315,7 +318,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
Expand Down
6 changes: 5 additions & 1 deletion lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ const debug = require('internal/util/debuglog').debuglog('https');
const { URL, urlToOptions, searchParamsSymbol } = require('internal/url');
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);
Expand Down Expand Up @@ -71,7 +75,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
Expand Down
8 changes: 5 additions & 3 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ const { UV_EOF } = internalBinding('uv');
const { StreamPipe } = internalBinding('stream_pipe');
const { _connectionListener: httpConnectionListener } = http;
const debug = require('internal/util/debuglog').debuglog('http2');
const { getOptionValue } = require('internal/options');

const kMaxFrameSize = (2 ** 24) - 1;
const kMaxInt = (2 ** 32) - 1;
Expand Down Expand Up @@ -171,7 +172,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,
Expand Down Expand Up @@ -2679,7 +2681,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);
Expand All @@ -2701,7 +2703,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);
Expand Down
5 changes: 5 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"(default: llhttp).",
&EnvironmentOptions::http_parser,
kAllowedInEnvironment);
AddOption("--http-server-default-timeout",
"Default http server socket timeout in ms "
"(default: 120000)",
&EnvironmentOptions::http_server_default_timeout,
kAllowedInEnvironment);
AddOption("--input-type",
"set module type for string input",
&EnvironmentOptions::module_type,
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class EnvironmentOptions : public Options {
bool frozen_intrinsics = false;
std::string heap_snapshot_signal;
std::string http_parser = "llhttp";
uint64_t http_server_default_timeout = 120000;
bool no_deprecation = false;
bool no_force_async_hooks_checks = false;
bool no_warnings = false;
Expand Down
41 changes: 41 additions & 0 deletions test/parallel/test-http-timeout-flag.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 588fd0c

Please sign in to comment.