http: fix timeout reset after keep-alive timeout

Fix the logic of resetting the socket timeout of keep-alive HTTP
connections and add two tests:

* `test-http-server-keep-alive-timeout-slow-server` is a regression test
  for GH-13391.  It ensures that the server-side keep-alive timeout will
  not fire during processing of a request.

* `test-http-server-keep-alive-timeout-slow-client-headers` ensures that
  the regular socket timeout is restored as soon as a client starts
  sending a new request, not as soon as the whole message is received,
  so that the keep-alive timeout will not fire while, e.g., the client
  is sending large cookies.

Refs: https://github.com/nodejs/node/pull/2534
Fixes: https://github.com/nodejs/node/issues/13391
PR-URL: https://github.com/nodejs/node/pull/13549
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>

1 files changed, 12 insertions, 8 deletions

diff --git a/lib/_http_server.js b/lib/_http_server.js
index 6d5dbf4584..854ae56387 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -441,14 +441,6 @@ function socketOnData(server, socket, parser, state, d) {
   assert(!socket._paused);
   debug('SERVER socketOnData %d', d.length);
 
-  if (state.keepAliveTimeoutSet) {
-    socket.setTimeout(0);
-    if (server.timeout) {
-      socket.setTimeout(server.timeout);
-    }
-    state.keepAliveTimeoutSet = false;
-  }
-
   var ret = parser.execute(d);
   onParserExecuteCommon(server, socket, parser, state, ret, d);
 }
@@ -469,6 +461,8 @@ function socketOnError(e) {
 }
 
 function onParserExecuteCommon(server, socket, parser, state, ret, d) {
+  resetSocketTimeout(server, socket, state);
+
   if (ret instanceof Error) {
     debug('parse error', ret);
     socketOnError.call(socket, ret);
@@ -550,6 +544,8 @@ function resOnFinish(req, res, socket, state, server) {
 // new message. In this callback we setup the response object and pass it
 // to the user.
 function parserOnIncoming(server, socket, state, req, keepAlive) {
+  resetSocketTimeout(server, socket, state);
+
   state.incoming.push(req);
 
   // If the writable end isn't consuming, then stop reading
@@ -611,6 +607,14 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
   return false; // Not a HEAD response. (Not even a response!)
 }
 
+function resetSocketTimeout(server, socket, state) {
+  if (!state.keepAliveTimeoutSet)
+    return;
+
+  socket.setTimeout(server.timeout || 0);
+  state.keepAliveTimeoutSet = false;
+}
+
 function onSocketResume() {
   // It may seem that the socket is resumed, but this is an enemy's trick to
   // deceive us! `resume` is emitted asynchronously, and may be called from