Commit 0519f500b48 for nodejs
commit 0519f500b485d7e3497e5b29c48b24290d521a04
Author: Matteo Collina <hello@matteocollina.com>
Date: Sun Sep 27 10:10:01 2026 +0200
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 <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/66152
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index 55055949818..3bef547c405 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<Value> argv[A_MAX];
- Local<Object> obj = object();
- Local<Value> cb = obj->Get(env()->context(),
- kOnHeadersComplete).ToLocalChecked();
+ Local<Value> 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<Value> cb = object()->Get(env->context(), kOnBody).ToLocalChecked();
+ Local<Value> cb = CachedCallback(kOnBody, kOnBodyCallback);
if (!cb->IsFunction())
return 0;
@@ -553,9 +559,8 @@ class Parser : public AsyncWrap, public StreamListener {
header_pairs_ = 0;
- Local<Object> obj = object();
- Local<Value> cb = obj->Get(env()->context(),
- kOnMessageComplete).ToLocalChecked();
+ Local<Value> 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<Value> 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<Value> 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<Value> CachedCallback(uint32_t index, int field) {
+ Local<Object> obj = object();
+ Local<Value> cb = obj->GetInternalField(field).As<Value>();
+ 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 00000000000..f9f0234089b
--- /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);
+ }));