Browse Source

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: https://github.com/nodejs/node/issues/2308
PR-URL: https://github.com/nodejs/node/pull/2352
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
v4.0.0-rc
Ben Noordhuis 10 years ago
parent
commit
9a3929dcbf
  1. 2
      src/js_stream.cc
  2. 6
      src/node_buffer.cc
  3. 34
      src/node_crypto.cc
  4. 3
      src/node_internals.h
  5. 2
      src/tls_wrap.cc

2
src/js_stream.cc

@ -90,7 +90,7 @@ int JSStream::DoWrite(WriteWrap* w,
Local<Array> bufs_arr = Array::New(env()->isolate(), count); Local<Array> bufs_arr = Array::New(env()->isolate(), count);
Local<Object> buf; Local<Object> buf;
for (size_t i = 0; i < count; i++) { for (size_t i = 0; i < count; i++) {
buf = Buffer::New(env(), bufs[i].base, bufs[i].len).ToLocalChecked(); buf = Buffer::Copy(env(), bufs[i].base, bufs[i].len).ToLocalChecked();
bufs_arr->Set(i, buf); bufs_arr->Set(i, buf);
} }

6
src/node_buffer.cc

@ -281,13 +281,13 @@ MaybeLocal<Object> Copy(Isolate* isolate, const char* data, size_t length) {
Environment* env = Environment::GetCurrent(isolate); Environment* env = Environment::GetCurrent(isolate);
EscapableHandleScope handle_scope(env->isolate()); EscapableHandleScope handle_scope(env->isolate());
Local<Object> obj; Local<Object> obj;
if (Buffer::New(env, data, length).ToLocal(&obj)) if (Buffer::Copy(env, data, length).ToLocal(&obj))
return handle_scope.Escape(obj); return handle_scope.Escape(obj);
return Local<Object>(); return Local<Object>();
} }
MaybeLocal<Object> New(Environment* env, const char* data, size_t length) { MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
EscapableHandleScope scope(env->isolate()); EscapableHandleScope scope(env->isolate());
// V8 currently only allows a maximum Typed Array index of max Smi. // 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) {
Environment* env = Environment::GetCurrent(isolate); Environment* env = Environment::GetCurrent(isolate);
EscapableHandleScope handle_scope(env->isolate()); EscapableHandleScope handle_scope(env->isolate());
Local<Object> obj; Local<Object> obj;
if (Buffer::New(env, data, length).ToLocal(&obj)) if (Buffer::Use(env, data, length).ToLocal(&obj))
return handle_scope.Escape(obj); return handle_scope.Escape(obj);
return Local<Object>(); return Local<Object>();
} }

34
src/node_crypto.cc

@ -1021,12 +1021,12 @@ int SecureContext::TicketKeyCallback(SSL* ssl,
Context::Scope context_scope(env->context()); Context::Scope context_scope(env->context());
Local<Value> argv[] = { Local<Value> argv[] = {
Buffer::New(env, Buffer::Copy(env,
reinterpret_cast<char*>(name), reinterpret_cast<char*>(name),
kTicketPartSize).ToLocalChecked(), kTicketPartSize).ToLocalChecked(),
Buffer::New(env, Buffer::Copy(env,
reinterpret_cast<char*>(iv), reinterpret_cast<char*>(iv),
kTicketPartSize).ToLocalChecked(), kTicketPartSize).ToLocalChecked(),
Boolean::New(env->isolate(), enc != 0) Boolean::New(env->isolate(), enc != 0)
}; };
Local<Value> ret = node::MakeCallback(env, Local<Value> ret = node::MakeCallback(env,
@ -1219,7 +1219,7 @@ int SSLWrap<Base>::NewSessionCallback(SSL* s, SSL_SESSION* sess) {
memset(serialized, 0, size); memset(serialized, 0, size);
i2d_SSL_SESSION(sess, &serialized); i2d_SSL_SESSION(sess, &serialized);
Local<Object> session = Buffer::New( Local<Object> session = Buffer::Copy(
env, env,
reinterpret_cast<char*>(sess->session_id), reinterpret_cast<char*>(sess->session_id),
sess->session_id_length).ToLocalChecked(); sess->session_id_length).ToLocalChecked();
@ -1240,7 +1240,7 @@ void SSLWrap<Base>::OnClientHello(void* arg,
Context::Scope context_scope(env->context()); Context::Scope context_scope(env->context());
Local<Object> hello_obj = Object::New(env->isolate()); Local<Object> hello_obj = Object::New(env->isolate());
Local<Object> buff = Buffer::New( Local<Object> buff = Buffer::Copy(
env, env,
reinterpret_cast<const char*>(hello.session_id()), reinterpret_cast<const char*>(hello.session_id()),
hello.session_size()).ToLocalChecked(); hello.session_size()).ToLocalChecked();
@ -1703,7 +1703,7 @@ void SSLWrap<Base>::GetTLSTicket(const FunctionCallbackInfo<Value>& args) {
if (sess == nullptr || sess->tlsext_tick == nullptr) if (sess == nullptr || sess->tlsext_tick == nullptr)
return; return;
Local<Object> buff = Buffer::New( Local<Object> buff = Buffer::Copy(
env, env,
reinterpret_cast<char*>(sess->tlsext_tick), reinterpret_cast<char*>(sess->tlsext_tick),
sess->tlsext_ticklen).ToLocalChecked(); sess->tlsext_ticklen).ToLocalChecked();
@ -1985,7 +1985,7 @@ int SSLWrap<Base>::TLSExtStatusCallback(SSL* s, void* arg) {
if (resp == nullptr) { if (resp == nullptr) {
arg = Null(env->isolate()); arg = Null(env->isolate());
} else { } else {
arg = Buffer::New( arg = Buffer::Copy(
env, env,
reinterpret_cast<char*>(const_cast<unsigned char*>(resp)), reinterpret_cast<char*>(const_cast<unsigned char*>(resp)),
len).ToLocalChecked(); len).ToLocalChecked();
@ -2991,7 +2991,8 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const {
if (initialised_ || kind_ != kCipher || !auth_tag_) if (initialised_ || kind_ != kCipher || !auth_tag_)
return false; return false;
*out_len = auth_tag_len_; *out_len = auth_tag_len_;
*out = new char[auth_tag_len_]; *out = static_cast<char*>(malloc(auth_tag_len_));
CHECK_NE(*out, nullptr);
memcpy(*out, auth_tag_, auth_tag_len_); memcpy(*out, auth_tag_, auth_tag_len_);
return true; return true;
} }
@ -3005,7 +3006,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo<Value>& args) {
unsigned int out_len = 0; unsigned int out_len = 0;
if (cipher->GetAuthTag(&out, &out_len)) { if (cipher->GetAuthTag(&out, &out_len)) {
Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked(); Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();
args.GetReturnValue().Set(buf); args.GetReturnValue().Set(buf);
} else { } else {
env->ThrowError("Attempting to get auth tag in unsupported state"); env->ThrowError("Attempting to get auth tag in unsupported state");
@ -3122,8 +3123,9 @@ void CipherBase::Update(const FunctionCallbackInfo<Value>& args) {
"Trying to add data in unsupported state"); "Trying to add data in unsupported state");
} }
CHECK(out != nullptr || out_len == 0);
Local<Object> buf = Local<Object> buf =
Buffer::New(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked(); Buffer::Copy(env, reinterpret_cast<char*>(out), out_len).ToLocalChecked();
if (out) if (out)
delete[] out; delete[] out;
@ -3198,7 +3200,7 @@ void CipherBase::Final(const FunctionCallbackInfo<Value>& args) {
} }
} }
Local<Object> buf = Buffer::New( Local<Object> buf = Buffer::Copy(
env, env,
reinterpret_cast<char*>(out_value), reinterpret_cast<char*>(out_value),
out_len).ToLocalChecked(); out_len).ToLocalChecked();
@ -3980,7 +3982,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo<Value>& args) {
} }
} }
Local<Object> vbuf = Buffer::New( Local<Object> vbuf = Buffer::Copy(
env, env,
reinterpret_cast<char*>(out_value), reinterpret_cast<char*>(out_value),
out_len).ToLocalChecked(); out_len).ToLocalChecked();
@ -4517,7 +4519,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo<Value>& args) {
} }
Local<Object> buf = Local<Object> buf =
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked(); Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
args.GetReturnValue().Set(buf); args.GetReturnValue().Set(buf);
} }

3
src/node_internals.h

@ -273,9 +273,8 @@ class NodeInstanceData {
}; };
namespace Buffer { namespace Buffer {
v8::MaybeLocal<v8::Object> Copy(Environment* env, const char* data, size_t len);
v8::MaybeLocal<v8::Object> New(Environment* env, size_t size); v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
// Makes a copy of |data|.
v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
// Takes ownership of |data|. // Takes ownership of |data|.
v8::MaybeLocal<v8::Object> New(Environment* env, v8::MaybeLocal<v8::Object> New(Environment* env,
char* data, char* data,

2
src/tls_wrap.cc

@ -660,7 +660,7 @@ void TLSWrap::OnReadSelf(ssize_t nread,
TLSWrap* wrap = static_cast<TLSWrap*>(ctx); TLSWrap* wrap = static_cast<TLSWrap*>(ctx);
Local<Object> buf_obj; Local<Object> buf_obj;
if (buf != nullptr) if (buf != nullptr)
buf_obj = Buffer::New(wrap->env(), buf->base, buf->len).ToLocalChecked(); buf_obj = Buffer::Use(wrap->env(), buf->base, buf->len).ToLocalChecked();
wrap->EmitData(nread, buf_obj, Local<Object>()); wrap->EmitData(nread, buf_obj, Local<Object>());
} }

Loading…
Cancel
Save