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

File tree

7 files changed

+52
-20
lines changed

7 files changed

+52
-20
lines changed

src/quic/data.cc

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,29 @@ Maybe<Store> Store::From(Local<ArrayBufferView> view, Local<Value> detach_key) {
115115
return Just(Store(std::move(backing), length, offset));
116116
}
117117

118+
Store Store::CopyFrom(Local<ArrayBuffer> buffer) {
119+
v8::Isolate* isolate = v8::Isolate::GetCurrent();
120+
auto backing = buffer->GetBackingStore();
121+
auto length = buffer->ByteLength();
122+
auto dest = ArrayBuffer::NewBackingStore(
123+
isolate, length, v8::BackingStoreInitializationMode::kUninitialized);
124+
// copy content
125+
memcpy(dest->Data(), backing->Data(), length);
126+
return Store(std::move(dest), length, 0);
127+
}
128+
129+
Store Store::CopyFrom(Local<ArrayBufferView> view) {
130+
v8::Isolate* isolate = v8::Isolate::GetCurrent();
131+
auto backing = view->Buffer()->GetBackingStore();
132+
auto length = view->ByteLength();
133+
auto offset = view->ByteOffset();
134+
auto dest = ArrayBuffer::NewBackingStore(
135+
isolate, length, v8::BackingStoreInitializationMode::kUninitialized);
136+
// copy content
137+
memcpy(dest->Data(), static_cast<char*>(backing->Data()) + offset, length);
138+
return Store(std::move(dest), length, 0);
139+
}
140+
118141
Local<Uint8Array> Store::ToUint8Array(Environment* env) const {
119142
return !store_
120143
? Uint8Array::New(ArrayBuffer::New(env->isolate(), 0), 0, 0)

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
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,23 @@ assert.strictEqual(endpoint.address, undefined);
2828
await assert.rejects(listen(123, { keys, certs, endpoint }), {
2929
code: 'ERR_INVALID_ARG_TYPE',
3030
});
31+
// Buffer is not detached.
32+
assert.strictEqual(certs.buffer.detached, false);
3133

3234
await assert.rejects(listen(() => {}, 123), {
3335
code: 'ERR_INVALID_ARG_TYPE',
3436
});
3537

3638
await listen(() => {}, { keys, certs, endpoint });
39+
// Buffer is not detached.
40+
assert.strictEqual(certs.buffer.detached, false);
41+
3742
await assert.rejects(listen(() => {}, { keys, certs, endpoint }), {
3843
code: 'ERR_INVALID_STATE',
3944
});
45+
// Buffer is not detached.
46+
assert.strictEqual(certs.buffer.detached, false);
47+
4048
assert.ok(state.isBound);
4149
assert.ok(state.isReceiving);
4250
assert.ok(state.isListening);
@@ -58,6 +66,9 @@ assert.ok(endpoint.destroyed);
5866
await assert.rejects(listen(() => {}, { keys, certs, endpoint }), {
5967
code: 'ERR_INVALID_STATE',
6068
});
69+
// Buffer is not detached.
70+
assert.strictEqual(certs.buffer.detached, false);
71+
6172
assert.throws(() => { endpoint.busy = true; }, {
6273
code: 'ERR_INVALID_STATE',
6374
});

0 commit comments

Comments
 (0)