quic: copy options.certs buffer instead of detaching · nodejs/node@62d71eb · GitHub
Skip to content

Commit 62d71eb

Browse files
legendecasaduh95
authored andcommitted
quic: copy options.certs buffer instead of detaching
The certs could be allocated in a pooled buffer, like `Buffer.from`, and `Buffer.allocUnsafe` (used by `fs.readFileSync`, etc). PR-URL: #61403 Refs: #61372 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent e88dd01 commit 62d71eb

7 files changed

Lines changed: 52 additions & 20 deletions

File tree

src/quic/data.cc

Lines changed: 23 additions & 0 deletions

src/quic/data.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,14 @@ class Store final : public MemoryRetainer {
7070
v8::Local<v8::ArrayBufferView> view,
7171
v8::Local<v8::Value> detach_key = v8::Local<v8::Value>());
7272

73+
// Creates a Store from the contents of an ArrayBuffer, always copying the
74+
// content.
75+
static Store CopyFrom(v8::Local<v8::ArrayBuffer> buffer);
76+
77+
// Creates a Store from the contents of an ArrayBufferView, always copying the
78+
// content.
79+
static Store CopyFrom(v8::Local<v8::ArrayBufferView> view);
80+
7381
v8::Local<v8::Uint8Array> ToUint8Array(Environment* env) const;
7482
inline v8::Local<v8::Uint8Array> ToUint8Array(Realm* realm) const {
7583
return ToUint8Array(realm->env());

src/quic/endpoint.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,10 +154,7 @@ bool SetOption(Environment* env,
154154
env, "The %s option must be an ArrayBufferView", nameStr);
155155
return false;
156156
}
157-
Store store;
158-
if (!Store::From(value.As<ArrayBufferView>()).To(&store)) {
159-
return false;
160-
}
157+
Store store = Store::CopyFrom(value.As<ArrayBufferView>());
161158
if (store.length() != TokenSecret::QUIC_TOKENSECRET_LEN) {
162159
Utf8Value nameStr(env->isolate(), name);
163160
THROW_ERR_INVALID_ARG_VALUE(

src/quic/tlscontext.cc

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,10 @@ bool SetOption(Environment* env,
133133
}
134134
} else if constexpr (std::is_same<T, Store>::value) {
135135
if (item->IsArrayBufferView()) {
136-
Store store;
137-
if (!Store::From(item.As<v8::ArrayBufferView>()).To(&store)) {
138-
return false;
139-
}
136+
Store store = Store::CopyFrom(item.As<v8::ArrayBufferView>());
140137
(options->*member).push_back(std::move(store));
141138
} else if (item->IsArrayBuffer()) {
142-
Store store;
143-
if (!Store::From(item.As<ArrayBuffer>()).To(&store)) {
144-
return false;
145-
}
139+
Store store = Store::CopyFrom(item.As<ArrayBuffer>());
146140
(options->*member).push_back(std::move(store));
147141
} else {
148142
Utf8Value namestr(env->isolate(), name);
@@ -168,16 +162,10 @@ bool SetOption(Environment* env,
168162
}
169163
} else if constexpr (std::is_same<T, Store>::value) {
170164
if (value->IsArrayBufferView()) {
171-
Store store;
172-
if (!Store::From(value.As<v8::ArrayBufferView>()).To(&store)) {
173-
return false;
174-
}
165+
Store store = Store::CopyFrom(value.As<v8::ArrayBufferView>());
175166
(options->*member).push_back(std::move(store));
176167
} else if (value->IsArrayBuffer()) {
177-
Store store;
178-
if (!Store::From(value.As<ArrayBuffer>()).To(&store)) {
179-
return false;
180-
}
168+
Store store = Store::CopyFrom(value.As<ArrayBuffer>());
181169
(options->*member).push_back(std::move(store));
182170
} else {
183171
Utf8Value namestr(env->isolate(), name);

test/parallel/test-quic-handshake-ipv6-only.mjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
4747
},
4848
ipv6Only: true,
4949
} });
50+
// Buffer is not detached.
51+
assert.strictEqual(certs.buffer.detached, false);
5052

5153
// The server must have an address to connect to after listen resolves.
5254
assert.ok(serverEndpoint.address !== undefined);

test/parallel/test-quic-handshake.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ const serverEndpoint = await listen(mustCall((serverSession) => {
3838
}).then(mustCall());
3939
}), { keys, certs });
4040

41+
// Buffer is not detached.
42+
assert.strictEqual(certs.buffer.detached, false);
43+
4144
// The server must have an address to connect to after listen resolves.
4245
assert.ok(serverEndpoint.address !== undefined);
4346

test/parallel/test-quic-internal-endpoint-listen-defaults.mjs

Lines changed: 11 additions & 0 deletions

0 commit comments

Comments
 (0)