http: cache maxHeaderPairs per header section

Signed-off-by: GetThatCookie <NimmenKeks@gmx.de>
PR-URL: https://github.com/nodejs/node/pull/64988
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

benchmark/http/bench-parser.js

@@ -31,6 +31,8 @@ function main({ len, n }) {
   function newParser(type) {
     const parser = new HTTPParser();
     parser.initialize(type, {});
+    // Direct parsers bypass cleanParser(); use its production default.
+    parser.maxHeaderPairs = 2000;
 
     parser.headers = [];
 

src/node_http_parser.cc

@@ -323,6 +323,7 @@ class Parser : public AsyncWrap, public StreamListener {
     allocator_.Reset();
     url_.Reset();
     status_message_.Reset();
+    max_header_pairs_ = -1;
 
     if (connectionsList_ != nullptr) {
       connectionsList_->Push(this);
@@ -465,6 +466,7 @@ 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) {
@@ -1034,6 +1036,7 @@ class Parser : public AsyncWrap, public StreamListener {
     headers_completed_ = false;
     max_http_header_size_ = max_http_header_size;
     header_pairs_ = 0;
+    max_header_pairs_ = -1;
   }
 
 
@@ -1053,21 +1056,23 @@ class Parser : public AsyncWrap, public StreamListener {
 
     header_pairs_ += 2;
 
-    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;
+    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_v->IsNumber()) {
-      return 0;
-    }
-
-    const double max_header_pairs = max_header_pairs_v.As<Number>()->Value();
-    if (max_header_pairs > 0 && header_pairs_ > max_header_pairs) {
+    if (max_header_pairs_ > 0 && header_pairs_ > max_header_pairs_) {
       llhttp_set_error_reason(&parser_, "HPE_HEADER_OVERFLOW:Header overflow");
       return HPE_USER;
     }
@@ -1109,6 +1114,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;
   bool pending_pause_ = false;
   bool received_data_ = false;
   uint64_t header_nread_ = 0;

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

@@ -0,0 +1,77 @@
+'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);
+}