http: fix incorrect headersTimeout measurement

For keep-alive connections, the headersTimeout may fire during
subsequent request because the measurement was reset after
a request and not before a request.

Fixes: nodejs#27363

Author: OrKoN

lib/_http_common.js

@@ -47,6 +47,7 @@ const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
 const kOnBody = HTTPParser.kOnBody | 0;
 const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
 const kOnExecute = HTTPParser.kOnExecute | 0;
+const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 
 const MAX_HEADER_PAIRS = 2000;
 
@@ -165,6 +166,7 @@ const parsers = new FreeList('parsers', 1000, function parsersCb() {
   parser[kOnHeadersComplete] = parserOnHeadersComplete;
   parser[kOnBody] = parserOnBody;
   parser[kOnMessageComplete] = parserOnMessageComplete;
+  parser[kOnMessageBegin] = null;
 
   return parser;
 });

lib/_http_server.js

@@ -142,6 +142,7 @@ const STATUS_CODES = {
 };
 
 const kOnExecute = HTTPParser.kOnExecute | 0;
+const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 
 class HTTPServerAsyncResource {
   constructor(type, socket) {
@@ -428,9 +429,6 @@ function connectionListenerInternal(server, socket) {
       isLenient() : server.insecureHTTPParser,
   );
   parser.socket = socket;
-
-  // We are starting to wait for our headers.
-  parser.parsingHeadersStart = nowDate();
   socket.parser = parser;
 
   // Propagate headers limit from server instance to parser
@@ -481,6 +479,7 @@ function connectionListenerInternal(server, socket) {
   }
   parser[kOnExecute] =
     onParserExecute.bind(undefined, server, socket, parser, state);
+  parser[kOnMessageBegin] = onParserMessageBegin.bind(undefined, parser);
 
   socket._paused = false;
 }
@@ -568,11 +567,17 @@ function socketOnData(server, socket, parser, state, d) {
   onParserExecuteCommon(server, socket, parser, state, ret, d);
 }
 
+function onParserMessageBegin(parser) {
+  // We are starting to wait for the headers.
+  parser.parsingHeadersStart = nowDate();
+}
+
 function onParserExecute(server, socket, parser, state, ret) {
   socket._unrefTimer();
-  const start = parser.parsingHeadersStart;
+
   debug('SERVER socketOnParserExecute %d', ret);
 
+  const start = parser.parsingHeadersStart;
   // If we have not parsed the headers, destroy the socket
   // after server.headersTimeout to protect from DoS attacks.
   // start === 0 means that we have parsed headers.
@@ -720,10 +725,6 @@ function emitCloseNT(self) {
 function parserOnIncoming(server, socket, state, req, keepAlive) {
   resetSocketTimeout(server, socket, state);
 
-  if (server.keepAliveTimeout > 0) {
-    req.on('end', resetHeadersTimeoutOnReqEnd);
-  }
-
   // Set to zero to communicate that we have finished parsing.
   socket.parser.parsingHeadersStart = 0;
 
@@ -851,17 +852,6 @@ function generateSocketListenerWrapper(originalFnName) {
   };
 }
 
-function resetHeadersTimeoutOnReqEnd() {
-  debug('resetHeadersTimeoutOnReqEnd');
-
-  const parser = this.socket.parser;
-  // Parser can be null if the socket was destroyed
-  // in that case, there is nothing to do.
-  if (parser) {
-    parser.parsingHeadersStart = nowDate();
-  }
-}
-
 module.exports = {
   STATUS_CODES,
   Server,

src/node_http_parser.cc

@@ -74,6 +74,7 @@ const uint32_t kOnHeadersComplete = 1;
 const uint32_t kOnBody = 2;
 const uint32_t kOnMessageComplete = 3;
 const uint32_t kOnExecute = 4;
+const uint32_t kOnMessageBegin = 5;
 // Any more fields than this will be flushed into JS
 const size_t kMaxHeaderFieldsCount = 32;
 
@@ -181,6 +182,23 @@ class Parser : public AsyncWrap, public StreamListener {
     num_fields_ = num_values_ = 0;
     url_.Reset();
     status_message_.Reset();
+
+    Local<Object> obj = object();
+    Local<Value> cb = obj->Get(env()->context(),
+                               kOnMessageBegin).ToLocalChecked();
+
+    if (!cb->IsFunction())
+      return 0;
+
+    Local<Value> argv[0];
+    MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, argv);
+
+    if (r.IsEmpty()) {
+      got_exception_ = true;
+      llhttp_set_error_reason(&parser_, "HPE_JS_EXCEPTION:JS Exception");
+      return HPE_USER;
+    }
+
     return 0;
   }
 
@@ -890,6 +908,8 @@ void InitializeHttpParser(Local<Object> target,
          Integer::NewFromUnsigned(env->isolate(), kOnMessageComplete));
   t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnExecute"),
          Integer::NewFromUnsigned(env->isolate(), kOnExecute));
+  t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnMessageBegin"),
+         Integer::NewFromUnsigned(env->isolate(), kOnMessageBegin));
 
   Local<Array> methods = Array::New(env->isolate());
 #define V(num, name, string)                                                  \

test/parallel/test-http-slow-headers-keepalive-multiple-requests.js

@@ -0,0 +1,50 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+const net = require('net');
+const { finished } = require('stream');
+
+const headers =
+  'GET / HTTP/1.1\r\n' +
+  'Host: localhost\r\n' +
+  'Connection: keep-alive\r\n' +
+  'Agent: node\r\n';
+
+const baseTimeout = 1000;
+
+const server = http.createServer(common.mustCall((req, res) => {
+  req.resume();
+  res.writeHead(200);
+  res.end();
+}, 2));
+
+server.keepAliveTimeout = 10 * baseTimeout;
+server.headersTimeout = baseTimeout;
+
+server.once('timeout', common.mustNotCall((socket) => {
+  socket.destroy();
+}));
+
+server.listen(0, () => {
+  const client = net.connect(server.address().port);
+
+  // first request
+  client.write(headers);
+  client.write('\r\n');
+
+  setTimeout(() => {
+    // second request
+    client.write(headers);
+    // `headersTimeout` doesn't seem to fire if request headers
+    // are sent in one packet.
+    setTimeout(() => {
+      client.write('\r\n');
+      client.end();
+    }, 10);
+  }, baseTimeout + 10);
+
+  finished(client, common.mustCall((err) => {
+    server.close();
+  }));
+});