https: Node HTTPS server has no socket timeout by default

[bminer]

The HTTP (not HTTPS) documentation reads:

By default, the Server's timeout value is 2 minutes, and sockets are
destroyed automatically if they time out. However, if you assign a
callback to the Server's 'timeout' event, then you are responsible
for handling socket timeouts.

This is not true for HTTPS servers. As you can see from this snippet, tls.js is overriding the socket timeout during the TLS handshake (setting it to options.handshakeTimeout || (120 * 1000), as appropriate). Then, after the TLS handshake, the socket timeout is disabled completely.

This behavior should be changed, or the documentation for the HTTPS server should be updated.

This particular issue has caused me massive headaches, as our Node HTTPS server has been crashing due to hanging connections (we have a lot of 3G modems connecting to our server).

As a workaround, I am using the following to restore the default 2 minute timeout:

server.on("secureConnection", function(clearText) {
    clearText.setTimeout(2 * 60 * 1000);
});

EDIT: I should also note that clearText.setTimeout is also NOT documented on the Node API.

[danielstjules]

Still there in 0.10.29: https://github.com/joyent/node/blob/v0.10.29/lib/tls.js#L1173-L1175

[bminer]

What is the best way to fix this problem? Here is one suggestion...

As http.js does, store the timeout as a property of the Server instance (actually no code needed for this since HTTPS server inherits from the HTTP server). So, basically you change this line to read:

socket.removeListener("timeout", listener);
socket.setTimeout(self.timeout || 0);

It would also be advisable to change the name of the local var timeout variable to something like handshakeTimeout to distinguish the two timeouts to the reader.

Thoughts?

[bminer]

Any thoughts on how we should fix this problem? I'd like to submit a PR soon.

[indutny]

A PR would definitely be welcome!

[adohe-zz]

@bminer in the https.js, there is a property like this this.timeout = 2 * 60 * 1000, but it seems useless.

[bminer]

@AdoHe - the timeout property is used here: https://github.com/joyent/node/blob/73343d5ceef7cb4ddee1ed0ddd2c51d1958e3bb1/lib/_http_server.js#L303

I'll submit a PR shortly.

[diegoperini]

This bug still exists afaik. Our iOS clients are able to hang their SSL handshakes which in the end causes infinite waits on server. We crash our processes when they occupy Node's event loop longer than our hard-coded threshold.

[rvagg]

@diegoperini what version of Node are you using?

[diegoperini]

v5.1.0 on an Ubuntu Trusty based Docker container.

[rvagg]

@diegoperini would you mind creating a new issue in https://github.com/nodejs/node with the problem you're experiencing, and if possible, a reduced test case that can be used to reproduce the error even with specifics about clients and server environment. I'm afraid it's going to be difficult to get action on this over here, particularly given #8021 was merged and supposed to deal with this; it could be a separate bug, maybe a new one in v5.

[bminer]

@rvagg - The PR #8021 was never merged. It's likely the same problem as before. Someone please post the relevant nodejs/node issue number here for everyone's reference.