http: pass maxHeaderPairs to parser.initialize()

The parser read the `maxHeaderPairs` property from its JS object in
C++ once per header section to enforce the header count limit. That
lookup is a runtime property load for every parsed request, and costs
up to 7% on the parser benchmark for requests with few headers.

Pass the limit to `initialize()` instead and keep it in a field. The
property is still set, as the JS side uses it to trim the header list.

Refs: https://github.com/nodejs/node/pull/64988
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/66250
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>

benchmark/http/bench-parser.js

@@ -16,6 +16,7 @@ function main({ len, n }) {
   const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
   const kOnBody = HTTPParser.kOnBody | 0;
   const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
+  const kMaxHeaderPairs = 2000;
 
   function processHeader(header, n) {
     const parser = newParser(REQUEST);
@@ -23,16 +24,15 @@ function main({ len, n }) {
     bench.start();
     for (let i = 0; i < n; i++) {
       parser.execute(header, 0, header.length);
-      parser.initialize(REQUEST, {});
+      parser.initialize(REQUEST, {}, 0, 0, undefined, kMaxHeaderPairs);
     }
     bench.end(n);
   }
 
   function newParser(type) {
     const parser = new HTTPParser();
-    parser.initialize(type, {});
     // Direct parsers bypass cleanParser(); use its production default.
-    parser.maxHeaderPairs = 2000;
+    parser.initialize(type, {}, 0, 0, undefined, kMaxHeaderPairs);
 
     parser.headers = [];
 

lib/_http_client.js

@@ -1102,10 +1102,16 @@ function tickOnSocket(req, socket) {
   const parser = parsers.alloc();
   req.socket = socket;
   const lenientFlags = calculateLenientFlags(req.httpValidation, req.insecureHTTPParser);
+  // Propagate headers limit from request object to parser
+  if (typeof req.maxHeadersCount === 'number') {
+    parser.maxHeaderPairs = req.maxHeadersCount << 1;
+  }
   parser.initialize(HTTPParser.RESPONSE,
                     new HTTPClientAsyncResource('HTTPINCOMINGMESSAGE', req),
                     req.maxHeaderSize || 0,
-                    lenientFlags);
+                    lenientFlags,
+                    undefined,
+                    parser.maxHeaderPairs);
   parser.socket = socket;
   parser.outgoing = req;
   req.parser = parser;
@@ -1113,11 +1119,6 @@ function tickOnSocket(req, socket) {
   socket.parser = parser;
   socket._httpMessage = req;
 
-  // Propagate headers limit from request object to parser
-  if (typeof req.maxHeadersCount === 'number') {
-    parser.maxHeaderPairs = req.maxHeadersCount << 1;
-  }
-
   parser.joinDuplicateHeaders = req.joinDuplicateHeaders;
 
   parser.onIncoming = parserOnIncomingClient;

lib/_http_server.js

@@ -811,6 +811,11 @@ function connectionListenerInternal(server, socket) {
 
   const lenientFlags = calculateLenientFlags(server.httpValidation, server.insecureHTTPParser);
 
+  // Propagate headers limit from server instance to parser
+  if (typeof server.maxHeadersCount === 'number') {
+    parser.maxHeaderPairs = server.maxHeadersCount << 1;
+  }
+
   // TODO(addaleax): This doesn't play well with the
   // `async_hooks.currentResource()` proposal, see
   // https://github.com/nodejs/node/pull/21313
@@ -820,15 +825,11 @@ function connectionListenerInternal(server, socket) {
     server.maxHeaderSize || 0,
     lenientFlags,
     server[kConnections],
+    parser.maxHeaderPairs,
   );
   parser.socket = socket;
   socket.parser = parser;
 
-  // Propagate headers limit from server instance to parser
-  if (typeof server.maxHeadersCount === 'number') {
-    parser.maxHeaderPairs = server.maxHeadersCount << 1;
-  }
-
   const state = {
     onData: null,
     onEnd: null,

src/node_http_parser.cc

@@ -31,6 +31,7 @@
 #include "stream_base-inl.h"
 #include "v8.h"
 
+#include <algorithm>
 #include <cstdlib>  // free()
 #include <cstring>  // strdup(), strchr()
 
@@ -324,7 +325,6 @@ class Parser : public AsyncWrap, public StreamListener {
     allocator_.Reset();
     url_.Reset();
     status_message_.Reset();
-    max_header_pairs_ = -1;
 
     if (connectionsList_ != nullptr) {
       connectionsList_->PushActive(this);
@@ -465,7 +465,6 @@ class Parser : public AsyncWrap, public StreamListener {
     num_fields_ = 0;
     num_values_ = 0;
     header_pairs_ = 0;
-    max_header_pairs_ = -1;
 
     // METHOD
     if (parser_.type == HTTP_REQUEST) {
@@ -683,6 +682,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
     uint64_t max_http_header_size = 0;
     uint32_t lenient_flags = kLenientNone;
+    size_t max_header_pairs = 0;
     ConnectionsList* connectionsList = nullptr;
 
     CHECK(args[0]->IsInt32());
@@ -707,6 +707,12 @@ class Parser : public AsyncWrap, public StreamListener {
       ASSIGN_OR_RETURN_UNWRAP(&connectionsList, args[4]);
     }
 
+    // Non-positive values mean no limit.
+    if (args.Length() > 5 && !args[5]->IsUndefined()) {
+      CHECK(args[5]->IsInt32());
+      max_header_pairs = std::max(args[5].As<Int32>()->Value(), 0);
+    }
+
     llhttp_type_t type =
         static_cast<llhttp_type_t>(args[0].As<Int32>()->Value());
 
@@ -723,7 +729,7 @@ class Parser : public AsyncWrap, public StreamListener {
 
     parser->set_provider_type(provider);
     parser->AsyncReset(args[1].As<Object>());
-    parser->Init(type, max_http_header_size, lenient_flags);
+    parser->Init(type, max_http_header_size, lenient_flags, max_header_pairs);
 
     if (connectionsList != nullptr) {
       parser->connectionsList_ = connectionsList;
@@ -974,9 +980,10 @@ class Parser : public AsyncWrap, public StreamListener {
     have_flushed_ = true;
   }
 
-
-  void Init(llhttp_type_t type, uint64_t max_http_header_size,
-            uint32_t lenient_flags) {
+  void Init(llhttp_type_t type,
+            uint64_t max_http_header_size,
+            uint32_t lenient_flags,
+            size_t max_header_pairs) {
     llhttp_init(&parser_, type, &settings);
 
     if (lenient_flags & kLenientHeaders) {
@@ -1026,10 +1033,9 @@ class Parser : public AsyncWrap, public StreamListener {
     headers_completed_ = false;
     max_http_header_size_ = max_http_header_size;
     header_pairs_ = 0;
-    max_header_pairs_ = -1;
+    max_header_pairs_ = max_header_pairs;
   }
 
-
   int TrackHeader(size_t len) {
     header_nread_ += len;
     if (header_nread_ >= max_http_header_size_) {
@@ -1042,22 +1048,6 @@ class Parser : public AsyncWrap, public StreamListener {
   int TrackHeaderPair() {
     header_pairs_ += 2;
 
-    if (max_header_pairs_ < 0) {
-      Local<Value> max_header_pairs_v;
-      if (!object()
-               ->Get(env()->context(),
-                     FIXED_ONE_BYTE_STRING(env()->isolate(), "maxHeaderPairs"))
-               .ToLocal(&max_header_pairs_v)) {
-        got_exception_ = true;
-        return -1;
-      }
-
-      const double value = max_header_pairs_v->IsNumber()
-                               ? max_header_pairs_v.As<Number>()->Value()
-                               : 0;
-      max_header_pairs_ = value > 0 ? value : 0;
-    }
-
     if (max_header_pairs_ > 0 && header_pairs_ > max_header_pairs_) {
       llhttp_set_error_reason(&parser_, "HPE_HEADER_OVERFLOW:Header overflow");
       return HPE_USER;
@@ -1100,7 +1090,7 @@ class Parser : public AsyncWrap, public StreamListener {
   const char* current_buffer_data_;
   bool headers_completed_ = false;
   size_t header_pairs_ = 0;
-  double max_header_pairs_ = -1;
+  size_t max_header_pairs_ = 0;
   bool pending_pause_ = false;
   bool received_data_ = false;
   uint64_t header_nread_ = 0;

test/parallel/test-http-parser-max-header-pairs-cache.js

@@ -1,77 +0,0 @@
-'use strict';
-
-const common = require('../common');
-const assert = require('assert');
-const { HTTPParser } = require('_http_common');
-
-const { REQUEST } = HTTPParser;
-const kOnHeaders = HTTPParser.kOnHeaders | 0;
-const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
-const kOnBody = HTTPParser.kOnBody | 0;
-const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
-
-function createParser() {
-  const parser = new HTTPParser();
-  parser.initialize(REQUEST, {});
-  parser[kOnHeaders] = () => {};
-  parser[kOnHeadersComplete] = () => {};
-  parser[kOnBody] = common.mustNotCall();
-  parser[kOnMessageComplete] = () => {};
-  return parser;
-}
-
-// maxHeaderPairs is cached once for each independent header section. Main
-// headers, trailers, the next message, and a reinitialized parser must each
-// observe a fresh value.
-{
-  const parser = createParser();
-  const limits = [2, 4, 2, 2];
-
-  Object.defineProperty(parser, 'maxHeaderPairs', {
-    configurable: true,
-    get: common.mustCall(() => limits.shift(), limits.length),
-  });
-
-  parser[kOnHeadersComplete] = common.mustCall(undefined, 3);
-  parser[kOnMessageComplete] = common.mustCall(undefined, 3);
-
-  const pipelined = Buffer.from(
-    'POST /first HTTP/1.1\r\n' +
-    'Transfer-Encoding: chunked\r\n' +
-    '\r\n' +
-    '0\r\n' +
-    'X-A: a\r\n' +
-    'X-B: b\r\n' +
-    '\r\n' +
-    'GET /second HTTP/1.1\r\n' +
-    'X-C: c\r\n' +
-    '\r\n'
-  );
-  assert.strictEqual(parser.execute(pipelined, 0, pipelined.length), pipelined.length);
-
-  parser.initialize(REQUEST, {});
-  const reused = Buffer.from('GET /reused HTTP/1.1\r\nX-D: d\r\n\r\n');
-  assert.strictEqual(parser.execute(reused, 0, reused.length), reused.length);
-  assert.deepStrictEqual(limits, []);
-}
-
-// Preserve the existing exception behavior for the first property lookup.
-{
-  const parser = createParser();
-  const expected = new Error('maxHeaderPairs getter');
-  Object.defineProperty(parser, 'maxHeaderPairs', {
-    get: common.mustCall(() => { throw expected; }),
-  });
-  const request = Buffer.from('GET / HTTP/1.1\r\nX-A: a\r\n\r\n');
-  assert.throws(() => parser.execute(request, 0, request.length), expected);
-}
-
-// Non-positive and non-number values continue to mean unlimited.
-for (const maxHeaderPairs of [undefined, null, NaN, 0, -1, new Number(2)]) {
-  const parser = createParser();
-  parser.maxHeaderPairs = maxHeaderPairs;
-  const request = Buffer.from(
-    'GET / HTTP/1.1\r\nX-A: a\r\nX-B: b\r\nX-C: c\r\n\r\n'
-  );
-  assert.strictEqual(parser.execute(request, 0, request.length), request.length);
-}

test/parallel/test-http-parser-max-header-pairs.js

@@ -0,0 +1,90 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { HTTPParser } = require('_http_common');
+
+const { REQUEST, RESPONSE } = HTTPParser;
+const kOnHeaders = HTTPParser.kOnHeaders | 0;
+const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
+const kOnBody = HTTPParser.kOnBody | 0;
+const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
+
+function createParser(type, ...initArgs) {
+  const parser = new HTTPParser();
+  parser.initialize(type, {}, ...initArgs);
+  parser[kOnHeaders] = () => {};
+  parser[kOnHeadersComplete] = () => {};
+  parser[kOnBody] = () => {};
+  parser[kOnMessageComplete] = () => {};
+  return parser;
+}
+
+function assertOverflow(result) {
+  assert.ok(result instanceof Error);
+  assert.strictEqual(result.code, 'HPE_HEADER_OVERFLOW');
+}
+
+const twoHeaders = 'X-A: a\r\nX-B: b\r\n';
+const threeHeaders = 'X-A: a\r\nX-B: b\r\nX-C: c\r\n';
+
+// The limit passed to initialize() applies to requests and responses.
+for (const [type, startLine] of [
+  [REQUEST, 'GET / HTTP/1.1\r\n'],
+  [RESPONSE, 'HTTP/1.1 200 OK\r\nContent-Length: 0\r\n'],
+]) {
+  // The response start line carries one header of its own.
+  const limit = type === REQUEST ? 4 : 6;
+
+  const ok = Buffer.from(`${startLine}${twoHeaders}\r\n`);
+  const parser = createParser(type, 0, 0, undefined, limit);
+  assert.strictEqual(parser.execute(ok, 0, ok.length), ok.length);
+
+  const tooMany = Buffer.from(`${startLine}${threeHeaders}\r\n`);
+  assertOverflow(createParser(type, 0, 0, undefined, limit)
+    .execute(tooMany, 0, tooMany.length));
+}
+
+// The parser does not read the maxHeaderPairs property.
+{
+  const parser = createParser(REQUEST, 0, 0, undefined, 2);
+  Object.defineProperty(parser, 'maxHeaderPairs', {
+    get: common.mustNotCall(),
+  });
+  const request = Buffer.from(`GET / HTTP/1.1\r\n${twoHeaders}\r\n`);
+  assertOverflow(parser.execute(request, 0, request.length));
+}
+
+// Main headers, trailers, and the next pipelined message are each counted
+// separately against the same limit.
+{
+  const parser = createParser(REQUEST, 0, 0, undefined, 4);
+  parser[kOnHeadersComplete] = common.mustCall(undefined, 2);
+  parser[kOnMessageComplete] = common.mustCall(undefined, 2);
+
+  const pipelined = Buffer.from(
+    'POST /first HTTP/1.1\r\n' +
+    'Transfer-Encoding: chunked\r\n' +
+    '\r\n' +
+    '0\r\n' +
+    twoHeaders +
+    '\r\n' +
+    `GET /second HTTP/1.1\r\n${twoHeaders}\r\n`
+  );
+  assert.strictEqual(parser.execute(pipelined, 0, pipelined.length), pipelined.length);
+}
+
+// Reinitializing the parser replaces the limit.
+{
+  const parser = createParser(REQUEST, 0, 0, undefined, 2);
+  parser.initialize(REQUEST, {}, 0, 0, undefined, 6);
+  const request = Buffer.from(`GET / HTTP/1.1\r\n${threeHeaders}\r\n`);
+  assert.strictEqual(parser.execute(request, 0, request.length), request.length);
+}
+
+// An omitted or non-positive limit means unlimited.
+for (const initArgs of [[], [0, 0, undefined, 0], [0, 0, undefined, -1]]) {
+  const parser = createParser(REQUEST, ...initArgs);
+  const request = Buffer.from(`GET / HTTP/1.1\r\n${threeHeaders}\r\n`);
+  assert.strictEqual(parser.execute(request, 0, request.length), request.length);
+}

typings/internalBinding/http_parser.d.ts

@@ -46,7 +46,8 @@ declare namespace InternalHttpParserBinding {
       resource: object,
       maxHeaderSize?: number,
       lenient?: number,
-      headersTimeout?: number,
+      connectionsList?: object,
+      maxHeaderPairs?: number,
     ): void;
     pause(): void;
     resume(): void;