Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 37 additions & 7 deletions src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}
Expand Down
61 changes: 61 additions & 0 deletions test/parallel/test-http-parser-cached-callbacks.js
Original file line number Diff line number Diff line change
@@ -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);
}));
Loading