Default http[2s] server timeout is problematic

[ofrobots]

Today, Node.js http servers timeout and close the socket after 2 minutes. This means that if a request needs to do work that takes longer than 2 minutes to complete; the socket would be unceremoniously closed and the client will get an empty response.

$ curl localhost:8080
curl: (52) Empty reply from server

This is a common problem (search results) that users have to work-around (but most don't know about it). This is specially problematic in serverless environments where users may be doing non-interactive event processing (e.g. photo / image transcoding, uploading large datasets somewhere). Users have to know that this problem is possible and must adjust the timeout to make sure the service works correctly.

This is problematic for PaaS hosting providers where the best we can do is document to our users that timeout must be adjusted. This is specially a problem for serverless containers (e.g. Google Cloud Run).

IMO the default value should be removed:

• Node.js seems to be the only environment where this timeout exists. Here's comparable HTTP server code in Ruby, Python, Rust, Go and Node.js for a request that takes ~10 minutes of work. Only the Node.js example terminates the socket unceremoniously.
• The timeout value of 120sec was added very early on in the history of the Node.js project, but no clear rationale was articulated for why the timeout should exist.

I would like propose that we remove the timeout. I am working on a PR to do so. Alternatively, If there is strong reason for us to have a default timeout value, let's clearly articulate the reason, and at least provide environment knobs to allow e.g. Serverless hosts to configure a different default value.

Thoughts? Opinions?

/cc @nodejs/http @nodejs/http2

[ofrobots]

#27558 removes the default value on the master branch and has landed. It is a semver major change however. We still have the issue that for existing branches in service, we need a way for platform providers (e.g. serverless environments) to configure a proper default (no timeout) and not have to depend on users knowing and working around this limitation.

I propose that we add an environment variable (NODE_DEFAULT_HTTP_SERVER_TIMEOUT) to allow the default value (2mins) to be overridden, but only on the currently supported branches (i.e. not on master). I am working on a PR to do this.

[ofrobots]

To elaborate, it would be simpler to add the environment variable across all branches, but that has the following downsides:

1. We will need to keep around this environment variable forever; even though the underlying problem has been fixed on master.
2. The environment variable may be perceived to set precedent for configuring arbitrary knobs (various timeouts, heuristics) through the environment.

[richardlau]

@ofrobots Does it need to be its own environment variable? It could be a command line option that could be allowed under NODE_OPTIONS?

[sam-github]

^--- I'm not sure if the config option should be exposed outside of the JS API, but if it is, I'd prefer a CLI option that can go in NODE_OPTIONS.

[ofrobots]

The reason for allowing configurability outside of the JS API is that hosting environments (e.g. serverless environment, specially serverless containers) usually don't have control over user code, but they do have control over the process environment. If it is exposed only via the JS API, then all we can do is document this for the user as a pitfall.

@sam-github @richardlau cli flag is probably be fine.

[ofrobots]

#27704 is a semver minor change to 12.x to fix this. Once that lands, it can be further back-ported to 10.x and 8.x.

[aduh95]

As pointed out by @natlibfi-arlehiko in #31378, LTS documentation on this topic is misleading:

node/doc/api/net.md

Lines 886 to 896 in d617fdf

	### socket.setTimeout(timeout\[, callback\])
	<!-- YAML
	added: v0.1.90
	-->
	* `timeout` {number}
	* `callback` {Function}
	* Returns: {net.Socket} The socket itself.
	Sets the socket to timeout after `timeout` milliseconds of inactivity on
	the socket. By default `net.Socket` do not have a timeout.

Same on v10:

node/doc/api/net.md

Line 829 in 308b8b9

the socket. By default `net.Socket` do not have a timeout.

[allyraza]

Hello, I see the change has been merged already I wonder how this got merged into upstream without being caught, If the timeout is removed this opens up a possible security vulnerability.

Consider the following scenario I have node server running on linux with default file descriptors which is 1024, all an adversary has to do open 1024 connection and not send any data to break your system.

There is an issue open on golang issue tracker (golang/go#16100) to add timeout support to avoid this problem since author seems to compare this with go as one of the language.

[mcollina]

I just want to note that this comment was addressed in https://nodejs.org/en/blog/vulnerability/september-2020-security-releases/. There is a new requestTimeout configuration option that can be set in case Node.js is deployed without a reverse proxy/load balancer in front.

[clshortfuse]

Having no timeout appears to have the consequence that a session without any streams can linger in memory and never close. I think it's rare for any application code (handler) interact with sessions directly. They're likely to interact with the streams.

What can happen is that a session can either never create a stream, thus application never knowing a session was created. Or, an application can handle and close a stream, but the session just stays in memory because the client never sent something to go away. In other words:

• Client connects and creates new Session
• Client disconnects without signaling
• No more data is ever exchanged. Leaving session in memory because of no timeout.

OR

• Client connections and creates a new Session
• Client requests resource with new stream
• Resource is returned
• Stream is closed
• Client disconnects without signaling
• No more data is ever exchanged. Leaving session in memory because of no timeout.

I see multiple solutions:

• Use a default timeout, and suggest the application use stream.session.ping() to keep a stream alive.
• Have NodeJS always ping the session periodically which would allow nghttp2 to automatically close a dead stream on timeout
• Only use a default timeout on a session when there is no stream

If I had more low level experience, I could probably create an exploit that would bankrupt an HTTP2 server of its RAM (DOS attack) by spawning sessions with 0 requests, or bad requests and just leaving.

---

Go has a 15-second ping interval if no frames have been exchanged: golang/net#55

.NET has 20 seconds and a default option to only ping by default if there's an active request (aka stream), or always: dotnet/runtime#40257

Chrome uses a 10 second ping interval: https://chromium.googlesource.com/chromium/src/+/c9ec5e3acbfaacef722bbe5505d7998dfe39edf1/net/spdy/spdy_session.cc#109