Skip to content

http: cache parser callbacks #66152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mcollina wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −7
Open

http: cache parser callbacks #66152

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/node_http_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,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, &on_headers_complete_cb_);

if (!cb->IsFunction())
return 0;
Expand Down Expand Up @@ -521,7 +520,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, &on_body_cb_);

if (!cb->IsFunction())
return 0;
Expand Down Expand Up @@ -554,9 +553,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, &on_message_complete_cb_);

if (!cb->IsFunction())
return 0;
Expand Down Expand Up @@ -940,6 +938,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());
}
Expand Down Expand Up @@ -974,11 +975,27 @@ class Parser : public AsyncWrap, public StreamListener {
have_flushed_ = true;
}

// Caches a set-once callback property; the cache is cleared in Init().
Local<Value> CachedCallback(uint32_t index, v8::Global<v8::Value>* cache) {

@Flarna Flarna Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Local<Value> CachedCallback(uint32_t index, v8::Global<v8::Value>* cache) {
Local<Value> CachedCallback(uint32_t index, v8::Global<v8::Value>& cache) {

maybe?
And change -> by . to access it.

Isolate* isolate = env()->isolate();
if (!cache->IsEmpty()) {
return cache->Get(isolate);
}
Local<Value> cb = object()->Get(env()->context(), index).ToLocalChecked();
if (cb->IsFunction()) {
cache->Reset(isolate, cb);
}
return cb;
}

void Init(llhttp_type_t type, uint64_t max_http_header_size,
uint32_t lenient_flags) {
llhttp_init(&parser_, type, &settings);

on_headers_complete_cb_.Reset();
on_body_cb_.Reset();

@Flarna Flarna Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be done earlier, e.g. when parser is moved into freelist on managed side instead on next use?

on_message_complete_cb_.Reset();

if (lenient_flags & kLenientHeaders) {
llhttp_set_lenient_headers(&parser_, 1);
}
Expand Down Expand Up @@ -1102,6 +1119,10 @@ class Parser : public AsyncWrap, public StreamListener {
size_t header_pairs_ = 0;
double max_header_pairs_ = -1;
bool pending_pause_ = false;

v8::Global<v8::Value> on_headers_complete_cb_;
v8::Global<v8::Value> on_body_cb_;
v8::Global<v8::Value> on_message_complete_cb_;

@Flarna Flarna Sep 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why kOnHeaders callback is not cached?
Are the two complete callbacks called more then once per HTTP request? I assume caching for single use would be not helpful.

bool received_data_ = false;
uint64_t header_nread_ = 0;
uint64_t chunk_extensions_nread_ = 0;
Expand Down
Loading