From 0bb104225a5442616f48a6fd59880cce966f2cbb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 23 Sep 2026 23:15:49 +0200 Subject: [PATCH] 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 --- benchmark/http/bench-parser.js | 6 +- lib/_http_client.js | 13 +-- lib/_http_server.js | 11 +-- src/node_http_parser.cc | 40 ++++----- ...test-http-parser-max-header-pairs-cache.js | 77 ---------------- .../test-http-parser-max-header-pairs.js | 90 +++++++++++++++++++ typings/internalBinding/http_parser.d.ts | 3 +- 7 files changed, 123 insertions(+), 117 deletions(-) delete mode 100644 test/parallel/test-http-parser-max-header-pairs-cache.js create mode 100644 test/parallel/test-http-parser-max-header-pairs.js diff --git a/benchmark/http/bench-parser.js b/benchmark/http/bench-parser.js index 72cb2b6feb1..97062f6bea1 100644 --- a/benchmark/http/bench-parser.js +++ b/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 = []; diff --git a/lib/_http_client.js b/lib/_http_client.js index 4a9f3311b8b..c4bdb01c29a 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -1098,10 +1098,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; @@ -1109,11 +1115,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; diff --git a/lib/_http_server.js b/lib/_http_server.js index 64189a4a664..981b73ae404 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -808,6 +808,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 @@ -817,15 +822,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, diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index cec306cb47d..55055949818 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -31,6 +31,7 @@ #include "stream_base-inl.h" #include "v8.h" +#include #include // free() #include // 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()->Value(), 0); + } + llhttp_type_t type = static_cast(args[0].As()->Value()); @@ -723,7 +729,7 @@ class Parser : public AsyncWrap, public StreamListener { parser->set_provider_type(provider); parser->AsyncReset(args[1].As()); - 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 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()->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; diff --git a/test/parallel/test-http-parser-max-header-pairs-cache.js b/test/parallel/test-http-parser-max-header-pairs-cache.js deleted file mode 100644 index dc8a60f36b0..00000000000 --- a/test/parallel/test-http-parser-max-header-pairs-cache.js +++ /dev/null @@ -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); -} diff --git a/test/parallel/test-http-parser-max-header-pairs.js b/test/parallel/test-http-parser-max-header-pairs.js new file mode 100644 index 00000000000..808d21dd54a --- /dev/null +++ b/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); +} diff --git a/typings/internalBinding/http_parser.d.ts b/typings/internalBinding/http_parser.d.ts index 162081dc2cc..4954cd96b1a 100644 --- a/typings/internalBinding/http_parser.d.ts +++ b/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;