Skip to content

Commit fd63e1c

Browse files
bnoordhuisrvagg
authored andcommitted
src: introduce internal Buffer::Copy() function
Rename the three argument overload of Buffer::New() to Buffer::Copy() and update the code base accordingly. The reason for renaming is to make it impossible to miss a call site. This coincidentally plugs a small memory leak in crypto.getAuthTag(). Fixes: #2308 PR-URL: #2352 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
1 parent 5586cec commit fd63e1c

File tree

5 files changed

+24
-23
lines changed

5 files changed

+24
-23
lines changed

src/js_stream.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ int JSStream::DoWrite(WriteWrap* w,
9090
Local<Array> bufs_arr = Array::New(env()->isolate(), count);
9191
Local<Object> buf;
9292
for (size_t i = 0; i < count; i++) {
93-
buf = Buffer::New(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
93+
buf = Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
9494
bufs_arr->Set(i, buf);
9595
}
9696

src/node_buffer.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -281,13 +281,13 @@ MaybeLocal<Object> Copy(Isolate* isolate, const char* data, size_t length) {
281281
Environment* env = Environment::GetCurrent(isolate);
282282
EscapableHandleScope handle_scope(env->isolate());
283283
Local<Object> obj;
284-
if (Buffer::New(env, data, length).ToLocal(&obj))
284+
if (Buffer::Copy(env, data, length).ToLocal(&obj))
285285
return handle_scope.Escape(obj);
286286
return Local<Object>();
287287
}
288288

289289

290-
MaybeLocal<Object> New(Environment* env, const char* data, size_t length) {
290+
MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
291291
EscapableHandleScope scope(env->isolate());
292292

293293
// V8 currently only allows a maximum Typed Array index of max Smi.
@@ -365,7 +365,7 @@ MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
365365
Environment* env = Environment::GetCurrent(isolate);
366366
EscapableHandleScope handle_scope(env->isolate());
367367
Local<Object> obj;
368-
if (Buffer::New(env, data, length).ToLocal(&obj))
368+
if (Buffer::Use(env, data, length).ToLocal(&obj))
369369
return handle_scope.Escape(obj);
370370
return Local<Object>();
371371
}

src/node_crypto.cc

+18-16
Original file line numberDiff line numberDiff line change
@@ -1021,12 +1021,12 @@ int SecureContext::TicketKeyCallback(SSL* ssl,
10211021
Context::Scope context_scope(env->context());
10221022

10231023
Local<Value> argv[] = {
1024-
Buffer::New(env,
1025-
reinterpret_cast<char*>(name),
1026-
kTicketPartSize).ToLocalChecked(),
1027-
Buffer::New(env,
1028-
reinterpret_cast<char*>(iv),
1029-
kTicketPartSize).ToLocalChecked(),
1024+
Buffer::Copy(env,
1025+
reinterpret_cast<char*>(name),
1026+
kTicketPartSize).ToLocalChecked(),
1027+
Buffer::Copy(env,
1028+
reinterpret_cast<char*>(iv),
1029+
kTicketPartSize).ToLocalChecked(),
10301030
Boolean::New(env->isolate(), enc != 0)
10311031
};
10321032
Local<Value> ret = node::MakeCallback(env,
@@ -1219,7 +1219,7 @@ int SSLWrap<Base>::NewSessionCallback(SSL* s, SSL_SESSION* sess) {
12191219
memset(serialized, 0, size);
12201220
i2d_SSL_SESSION(sess, &serialized);
12211221

1222-
Local<Object> session = Buffer::New(
1222+
Local<Object> session = Buffer::Copy(
12231223
env,
12241224
reinterpret_cast<char*>(sess->session_id),
12251225
sess->session_id_length).ToLocalChecked();
@@ -1240,7 +1240,7 @@ void SSLWrap<Base>::OnClientHello(void* arg,
12401240
Context::Scope context_scope(env->context());
12411241

12421242
Local<Object> hello_obj = Object::New(env->isolate());
1243-
Local<Object> buff = Buffer::New(
1243+
Local<Object> buff = Buffer::Copy(
12441244
env,
12451245
reinterpret_cast<const char*>(hello.session_id()),
12461246
hello.session_size()).ToLocalChecked();
@@ -1701,7 +1701,7 @@ void SSLWrap<Base>::GetTLSTicket(const FunctionCallbackInfo<Value>& args) {
17011701
if (sess == nullptr || sess->tlsext_tick == nullptr)
17021702
return;
17031703

1704-
Local<Object> buff = Buffer::New(
1704+
Local<Object> buff = Buffer::Copy(
17051705
env,
17061706
reinterpret_cast<char*>(sess->tlsext_tick),
17071707
sess->tlsext_ticklen).ToLocalChecked();
@@ -1983,7 +1983,7 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
19831983
if (resp == nullptr) {
19841984
arg = Null(env->isolate());
19851985
} else {
1986-
arg = Buffer::New(
1986+
arg = Buffer::Copy(
19871987
env,
19881988
reinterpret_cast<char*>(const_cast<unsigned char*>(resp)),
19891989
len).ToLocalChecked();
@@ -2989,7 +2989,8 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const {
29892989
if (initialised_ || kind_ != kCipher || !auth_tag_)
29902990
return false;
29912991
*out_len = auth_tag_len_;
2992-
*out = new char[auth_tag_len_];
2992+
*out = static_cast<char*>(malloc(auth_tag_len_));
2993+
CHECK_NE(*out, nullptr);
29932994
memcpy(*out, auth_tag_, auth_tag_len_);
29942995
return true;
29952996
}
@@ -3003,7 +3004,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo<Value>& args) {
30033004
unsigned int out_len = 0;
30043005

30053006
if (cipher->GetAuthTag(&out, &out_len)) {
3006-
Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked();
3007+
Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();
30073008
args.GetReturnValue().Set(buf);
30083009
} else {
30093010
env->ThrowError("Attempting to get auth tag in unsupported state");
@@ -3120,8 +3121,9 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
31203121
"Trying to add data in unsupported state");
31213122
}
31223123

3124+
CHECK(out != nullptr || out_len == 0);
31233125
Local<Object> buf =
3124-
Buffer::New(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked();
3126+
Buffer::Copy(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked();
31253127
if (out)
31263128
delete[] out;
31273129

@@ -3196,7 +3198,7 @@ void CipherBase::Final(const FunctionCallbackInfo<Value>& args) {
31963198
}
31973199
}
31983200

3199-
Local<Object> buf = Buffer::New(
3201+
Local<Object> buf = Buffer::Copy(
32003202
env,
32013203
reinterpret_cast<char*>(out_value),
32023204
out_len).ToLocalChecked();
@@ -3978,7 +3980,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
39783980
}
39793981
}
39803982

3981-
Local<Object> vbuf = Buffer::New(
3983+
Local<Object> vbuf = Buffer::Copy(
39823984
env,
39833985
reinterpret_cast<char*>(out_value),
39843986
out_len).ToLocalChecked();
@@ -4515,7 +4517,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
45154517
}
45164518

45174519
Local<Object> buf =
4518-
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
4520+
Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
45194521
args.GetReturnValue().Set(buf);
45204522
}
45214523

src/node_internals.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,8 @@ class NodeInstanceData {
273273
};
274274

275275
namespace Buffer {
276+
v8::MaybeLocal<v8::Object> Copy(Environment* env, const char* data, size_t len);
276277
v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
277-
// Makes a copy of |data|.
278-
v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
279278
// Takes ownership of |data|.
280279
v8::MaybeLocal<v8::Object> New(Environment* env,
281280
char* data,

src/tls_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ void TLSWrap::OnReadSelf(ssize_t nread,
660660
TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
661661
Local<Object> buf_obj;
662662
if (buf != nullptr)
663-
buf_obj = Buffer::New(wrap->env(), buf->base, buf->len).ToLocalChecked();
663+
buf_obj = Buffer::Use(wrap->env(), buf->base, buf->len).ToLocalChecked();
664664
wrap->EmitData(nread, buf_obj, Local<Object>());
665665
}
666666

0 commit comments

Comments
 (0)