From 2504fd19c6eee05953435a8a98c32a281aa8f7b5 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 20 Sep 2026 09:37:15 +0200 Subject: [PATCH] http: cache parser callbacks Cache the per-message callback lookups on the parser's JS object instead of retaining callback functions in strong v8::Global handles. A callback that captures its parser can otherwise keep the parser alive. Clear the cache when a parser is initialized or freed so reused parsers can load replacement callbacks and idle parsers do not retain them. Header field names remain non-internalized because they are supplied by clients. Assisted-by: pi Signed-off-by: Matteo Collina --- src/node_http_parser.cc | 44 ++++++++++--- .../test-http-parser-cached-callbacks.js | 61 +++++++++++++++++++ 2 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-http-parser-cached-callbacks.js diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 550559498187..3bef547c405c 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -312,6 +312,13 @@ class Parser : public AsyncWrap, public StreamListener { current_buffer_data_(nullptr), binding_data_(binding_data) {} + enum InternalFields { + kOnHeadersCompleteCallback = AsyncWrap::kInternalFieldCount, + kOnBodyCallback, + kOnMessageCompleteCallback, + kInternalFieldCount + }; + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(Parser) SET_SELF_SIZE(Parser) @@ -441,9 +448,8 @@ class Parser : public AsyncWrap, public StreamListener { }; Local argv[A_MAX]; - Local obj = object(); - Local cb = obj->Get(env()->context(), - kOnHeadersComplete).ToLocalChecked(); + Local cb = + CachedCallback(kOnHeadersComplete, kOnHeadersCompleteCallback); if (!cb->IsFunction()) return 0; @@ -520,7 +526,7 @@ class Parser : public AsyncWrap, public StreamListener { Environment* env = this->env(); HandleScope handle_scope(env->isolate()); - Local cb = object()->Get(env->context(), kOnBody).ToLocalChecked(); + Local cb = CachedCallback(kOnBody, kOnBodyCallback); if (!cb->IsFunction()) return 0; @@ -553,9 +559,8 @@ class Parser : public AsyncWrap, public StreamListener { header_pairs_ = 0; - Local obj = object(); - Local cb = obj->Get(env()->context(), - kOnMessageComplete).ToLocalChecked(); + Local cb = + CachedCallback(kOnMessageComplete, kOnMessageCompleteCallback); if (!cb->IsFunction()) return 0; @@ -624,6 +629,7 @@ class Parser : public AsyncWrap, public StreamListener { // it needs to be triggered manually. parser->EmitTraceEventDestroy(); parser->EmitDestroy(); + parser->ClearCachedCallbacks(); } // TODO(@anonrig): Add V8 Fast API @@ -946,6 +952,9 @@ class Parser : public AsyncWrap, public StreamListener { Local headers_v[kMaxHeaderFieldsCount * 2]; for (size_t i = 0; i < num_values_; ++i) { + // Field names are not internalized: header names are attacker + // controlled, so a flood of unique names would grow V8's string table + // and pay the interning cost on every request with no dedup benefit. headers_v[i * 2] = fields_[i].ToString(env()); headers_v[i * 2 + 1] = values_[i].ToTrimmedString(env()); } @@ -980,12 +989,33 @@ class Parser : public AsyncWrap, public StreamListener { have_flushed_ = true; } + void ClearCachedCallbacks() { + Local undefined = Undefined(env()->isolate()); + object()->SetInternalField(kOnHeadersCompleteCallback, undefined); + object()->SetInternalField(kOnBodyCallback, undefined); + object()->SetInternalField(kOnMessageCompleteCallback, undefined); + } + + // Keep cached callbacks on the JS object so they do not keep the parser + // alive when a callback closes over it. + Local CachedCallback(uint32_t index, int field) { + Local obj = object(); + Local cb = obj->GetInternalField(field).As(); + if (cb->IsFunction()) return cb; + + cb = obj->Get(env()->context(), index).ToLocalChecked(); + if (cb->IsFunction()) obj->SetInternalField(field, cb); + return cb; + } + 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); + ClearCachedCallbacks(); + if (lenient_flags & kLenientHeaders) { llhttp_set_lenient_headers(&parser_, 1); } diff --git a/test/parallel/test-http-parser-cached-callbacks.js b/test/parallel/test-http-parser-cached-callbacks.js new file mode 100644 index 000000000000..f9f0234089b6 --- /dev/null +++ b/test/parallel/test-http-parser-cached-callbacks.js @@ -0,0 +1,61 @@ +// Flags: --expose-gc +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { HTTPParser } = require('_http_common'); +const { gcUntil } = require('../common/gc'); + +const request = Buffer.from('POST / HTTP/1.1\r\nContent-Length: 4\r\n\r\nbody'); +const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0; +const kOnBody = HTTPParser.kOnBody | 0; +const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0; + +const parser = new HTTPParser(); +parser.tag = 'parser'; +parser.initialize(HTTPParser.REQUEST, {}); + +let calls = 0; +for (const callback of [kOnHeadersComplete, kOnBody, kOnMessageComplete]) { + parser[callback] = common.mustCall(() => { + assert.strictEqual(parser.tag, 'parser'); + calls++; + }, 2); +} + +parser.execute(request); +parser.execute(request); +assert.strictEqual(calls, 6); + +// Reinitializing must replace each cached callback. +parser.initialize(HTTPParser.REQUEST, {}); +for (const callback of [kOnHeadersComplete, kOnBody, kOnMessageComplete]) { + parser[callback] = common.mustCall(() => { + assert.strictEqual(parser.tag, 'parser'); + calls += 2; + }); +} +parser.execute(request); +assert.strictEqual(calls, 12); + +// Freeing a parser must release a cached callback even if the parser itself +// remains reachable and its JS callback property has been replaced. +function freedCallback() { + const parser = new HTTPParser(); + parser.initialize(HTTPParser.REQUEST, {}); + let callback = () => parser; + parser[kOnHeadersComplete] = callback; + parser.execute(request); + const ref = new WeakRef(callback); + parser[kOnHeadersComplete] = null; + callback = null; + parser.free(); + return { parser, ref }; +} + +const { parser: freedParser, ref } = freedCallback(); +gcUntil('freed HTTPParser callback', () => ref.deref() === undefined) + .then(common.mustCall(() => { + // Keep the parser alive while its callback is collected. + assert.ok(freedParser); + }));