From cc07cc972937bfa08b72902101b5e425212bbf52 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 2 Jun 2015 11:09:03 -0600 Subject: [PATCH] buffer: switch API to return MaybeLocal Instead of aborting in case of internal failure, return an empty Local. Using the MaybeLocal API, users must check their return values. PR-URL: https://github.com/nodejs/io.js/pull/1825 Reviewed-By: Ben Noordhuis --- src/js_stream.cc | 19 ++++--- src/node_buffer.cc | 132 ++++++++++++++++++++++++-------------------- src/node_buffer.h | 66 +++++++++++----------- src/node_crypto.cc | 64 ++++++++++++--------- src/spawn_sync.cc | 2 +- src/stream_wrap.cc | 3 +- src/string_bytes.cc | 6 +- src/tls_wrap.cc | 2 +- src/udp_wrap.cc | 2 +- 9 files changed, 164 insertions(+), 132 deletions(-) diff --git a/src/js_stream.cc b/src/js_stream.cc index 91041d0201188d..aa8de3a9ad9b8f 100644 --- a/src/js_stream.cc +++ b/src/js_stream.cc @@ -88,8 +88,11 @@ int JSStream::DoWrite(WriteWrap* w, HandleScope scope(env()->isolate()); Local bufs_arr = Array::New(env()->isolate(), count); - for (size_t i = 0; i < count; i++) - bufs_arr->Set(i, Buffer::New(env(), bufs[i].base, bufs[i].len)); + Local buf; + for (size_t i = 0; i < count; i++) { + buf = Buffer::New(env(), bufs[i].base, bufs[i].len).ToLocalChecked(); + bufs_arr->Set(i, buf); + } Local argv[] = { w->object(), @@ -134,11 +137,13 @@ void JSStream::DoAlloc(const FunctionCallbackInfo& args) { uv_buf_t buf; wrap->OnAlloc(args[0]->Int32Value(), &buf); - args.GetReturnValue().Set(Buffer::New(wrap->env(), - buf.base, - buf.len, - FreeCallback, - nullptr)); + Local vbuf = Buffer::New( + wrap->env(), + buf.base, + buf.len, + FreeCallback, + nullptr).ToLocalChecked(); + return args.GetReturnValue().Set(vbuf); } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 765f642d10faf7..0b8b22b1ee3f95 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -74,6 +74,7 @@ using v8::HandleScope; using v8::Isolate; using v8::Local; using v8::Maybe; +using v8::MaybeLocal; using v8::Number; using v8::Object; using v8::Persistent; @@ -234,7 +235,9 @@ size_t Length(Handle obj) { } -Local New(Isolate* isolate, Handle string, enum encoding enc) { +MaybeLocal New(Isolate* isolate, + Local string, + enum encoding enc) { EscapableHandleScope scope(isolate); size_t length = StringBytes::Size(isolate, string, enc); @@ -251,19 +254,26 @@ Local New(Isolate* isolate, Handle string, enum encoding enc) { CHECK_NE(data, nullptr); } - Local buf = Use(isolate, data, actual); - return scope.Escape(buf); + Local buf; + if (Use(isolate, data, actual).ToLocal(&buf)) + return scope.Escape(buf); + + // Object failed to be created. Clean up resources. + free(data); + return Local(); } -Local New(Isolate* isolate, size_t length) { +MaybeLocal New(Isolate* isolate, size_t length) { EscapableHandleScope handle_scope(isolate); - Local obj = Buffer::New(Environment::GetCurrent(isolate), length); - return handle_scope.Escape(obj); + Local obj; + if (Buffer::New(Environment::GetCurrent(isolate), length).ToLocal(&obj)) + return handle_scope.Escape(obj); + return Local(); } -Local New(Environment* env, size_t length) { +MaybeLocal New(Environment* env, size_t length) { EscapableHandleScope scope(env->isolate()); if (using_old_buffer) { @@ -286,13 +296,12 @@ Local New(Environment* env, size_t length) { void* data; if (length > 0) { data = malloc(length); - // NOTE: API change. Must check .IsEmpty() on the return object to see if - // the data was able to be allocated. if (data == nullptr) return Local(); } else { data = nullptr; } + Local ab = ArrayBuffer::New(env->isolate(), data, @@ -301,25 +310,27 @@ Local New(Environment* env, size_t length) { Local ui = Uint8Array::New(ab, 0, length); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (!mb.FromMaybe(false)) { - FatalError("node::Buffer::New(Environment*, size_t)", - "Could not set Object prototype"); - UNREACHABLE(); - } - return scope.Escape(ui); + if (mb.FromMaybe(false)) + return scope.Escape(ui); + + // Object failed to be created. Clean up resources. + free(data); + return Local(); } -Local New(Isolate* isolate, const char* data, size_t length) { +MaybeLocal New(Isolate* isolate, const char* data, size_t length) { Environment* env = Environment::GetCurrent(isolate); EscapableHandleScope handle_scope(env->isolate()); - Local obj = Buffer::New(env, data, length); - return handle_scope.Escape(obj); + Local obj; + if (Buffer::New(env, data, length).ToLocal(&obj)) + return handle_scope.Escape(obj); + return Local(); } // Make a copy of "data". Why this isn't called "Copy", we'll never know. -Local New(Environment* env, const char* data, size_t length) { +MaybeLocal New(Environment* env, const char* data, size_t length) { EscapableHandleScope scope(env->isolate()); if (using_old_buffer) { @@ -353,8 +364,6 @@ Local New(Environment* env, const char* data, size_t length) { if (length > 0) { CHECK_NE(data, nullptr); new_data = malloc(length); - // NOTE: API change. Must check .IsEmpty() on the return object to see if - // the data was able to be allocated. if (new_data == nullptr) return Local(); memcpy(new_data, data, length); @@ -370,33 +379,34 @@ Local New(Environment* env, const char* data, size_t length) { Local ui = Uint8Array::New(ab, 0, length); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (!mb.FromMaybe(false)) { - FatalError("node::Buffer::New(Environment*, char*, size_t)", - "Could not set Object prototype"); - UNREACHABLE(); - } + if (mb.FromMaybe(false)) + return scope.Escape(ui); - return scope.Escape(ui); + // Object failed to be created. Clean up resources. + free(new_data); + return Local(); } -Local New(Isolate* isolate, - char* data, - size_t length, - FreeCallback callback, - void* hint) { +MaybeLocal New(Isolate* isolate, + char* data, + size_t length, + FreeCallback callback, + void* hint) { Environment* env = Environment::GetCurrent(isolate); EscapableHandleScope handle_scope(env->isolate()); - Local obj = Buffer::New(env, data, length, callback, hint); - return handle_scope.Escape(obj); + Local obj; + if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj)) + return handle_scope.Escape(obj); + return Local(); } -Local New(Environment* env, - char* data, - size_t length, - FreeCallback callback, - void* hint) { +MaybeLocal New(Environment* env, + char* data, + size_t length, + FreeCallback callback, + void* hint) { EscapableHandleScope scope(env->isolate()); if (using_old_buffer) { @@ -416,26 +426,26 @@ Local New(Environment* env, Local ui = Uint8Array::New(ab, 0, length); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (!mb.FromMaybe(false)) { - FatalError("node::Buffer::New(Environment*, char*, size_t," - " FreeCallback, void*)", - "Could not set Object prototype"); - UNREACHABLE(); - } + + if (!mb.FromMaybe(false)) + return Local(); + CallbackInfo::New(env->isolate(), ui, callback, hint); return scope.Escape(ui); } -Local Use(Isolate* isolate, char* data, size_t length) { +MaybeLocal Use(Isolate* isolate, char* data, size_t length) { Environment* env = Environment::GetCurrent(isolate); EscapableHandleScope handle_scope(env->isolate()); - Local obj = Buffer::Use(env, data, length); - return handle_scope.Escape(obj); + Local obj; + if (Buffer::Use(env, data, length).ToLocal(&obj)) + return handle_scope.Escape(obj); + return Local(); } -Local Use(Environment* env, char* data, size_t length) { +MaybeLocal Use(Environment* env, char* data, size_t length) { EscapableHandleScope scope(env->isolate()); if (using_old_buffer) { @@ -463,12 +473,9 @@ Local Use(Environment* env, char* data, size_t length) { Local ui = Uint8Array::New(ab, 0, length); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (!mb.FromMaybe(false)) { - FatalError("node::Buffer::Use(Environment*, char*, size_t)", - "Could not set Object prototype"); - UNREACHABLE(); - } - return scope.Escape(ui); + if (mb.FromMaybe(false)) + return scope.Escape(ui); + return Local(); } @@ -501,8 +508,9 @@ void Create(const FunctionCallbackInfo& args) { Local ui = Uint8Array::New(ab, 0, length); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (mb.FromMaybe(false)) - args.GetReturnValue().Set(ui); + if (!mb.FromMaybe(false)) + return env->ThrowError("Unable to set Object prototype"); + args.GetReturnValue().Set(ui); } @@ -513,8 +521,9 @@ void CreateFromString(const FunctionCallbackInfo& args) { enum encoding enc = ParseEncoding(args.GetIsolate(), args[1].As(), UTF8); - Local buf = New(args.GetIsolate(), args[0].As(), enc); - args.GetReturnValue().Set(buf); + Local buf; + if (New(args.GetIsolate(), args[0].As(), enc).ToLocal(&buf)) + args.GetReturnValue().Set(buf); } @@ -535,8 +544,9 @@ void Slice(const FunctionCallbackInfo& args) { Local ui = Uint8Array::New(ab, start, size); Maybe mb = ui->SetPrototype(env->context(), env->buffer_prototype_object()); - if (mb.FromMaybe(false)) - args.GetReturnValue().Set(ui); + if (!mb.FromMaybe(false)) + env->ThrowError("Unable to set Object prototype"); + args.GetReturnValue().Set(ui); } diff --git a/src/node_buffer.h b/src/node_buffer.h index 568b68f83dbd5c..d59c44692b12fb 100644 --- a/src/node_buffer.h +++ b/src/node_buffer.h @@ -24,50 +24,52 @@ NODE_EXTERN size_t Length(v8::Handle val); NODE_EXTERN size_t Length(v8::Handle val); // public constructor -NODE_EXTERN v8::Local New(v8::Isolate* isolate, size_t length); +NODE_EXTERN v8::MaybeLocal New(v8::Isolate* isolate, size_t length); NODE_DEPRECATED("Use New(isolate, ...)", - inline v8::Local New(size_t length) { + inline v8::MaybeLocal New(size_t length) { return New(v8::Isolate::GetCurrent(), length); }) // public constructor from string -NODE_EXTERN v8::Local New(v8::Isolate* isolate, - v8::Handle string, - enum encoding enc = UTF8); +NODE_EXTERN v8::MaybeLocal New(v8::Isolate* isolate, + v8::Handle string, + enum encoding enc = UTF8); NODE_DEPRECATED("Use New(isolate, ...)", - inline v8::Local New(v8::Handle string, - enum encoding enc = UTF8) { + inline v8::MaybeLocal New( + v8::Handle string, + enum encoding enc = UTF8) { return New(v8::Isolate::GetCurrent(), string, enc); }) // public constructor - data is copied // TODO(trevnorris): should be something like Copy() -NODE_EXTERN v8::Local New(v8::Isolate* isolate, - const char* data, - size_t len); +NODE_EXTERN v8::MaybeLocal New(v8::Isolate* isolate, + const char* data, + size_t len); NODE_DEPRECATED("Use New(isolate, ...)", - inline v8::Local New(const char* data, size_t len) { + inline v8::MaybeLocal New(const char* data, + size_t len) { return New(v8::Isolate::GetCurrent(), data, len); }) // public constructor - data is used, callback is passed data on object gc -NODE_EXTERN v8::Local New(v8::Isolate* isolate, - char* data, - size_t length, - FreeCallback callback, - void* hint); +NODE_EXTERN v8::MaybeLocal New(v8::Isolate* isolate, + char* data, + size_t length, + FreeCallback callback, + void* hint); NODE_DEPRECATED("Use New(isolate, ...)", - inline v8::Local New(char* data, - size_t length, - FreeCallback callback, - void* hint) { + inline v8::MaybeLocal New(char* data, + size_t length, + FreeCallback callback, + void* hint) { return New(v8::Isolate::GetCurrent(), data, length, callback, hint); }) // public constructor - data is used. // TODO(trevnorris): should be New() for consistency -NODE_EXTERN v8::Local Use(v8::Isolate* isolate, - char* data, - size_t len); +NODE_EXTERN v8::MaybeLocal Use(v8::Isolate* isolate, + char* data, + size_t len); NODE_DEPRECATED("Use Use(isolate, ...)", - inline v8::Local Use(char* data, size_t len) { + inline v8::MaybeLocal Use(char* data, size_t len) { return Use(v8::Isolate::GetCurrent(), data, len); }) @@ -90,14 +92,14 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) { // src/node_internals.h due to a circular dependency issue with // the smalloc.h and node_internals.h headers. #if defined(NODE_WANT_INTERNALS) -v8::Local New(Environment* env, size_t size); -v8::Local New(Environment* env, const char* data, size_t len); -v8::Local New(Environment* env, - char* data, - size_t length, - FreeCallback callback, - void* hint); -v8::Local Use(Environment* env, char* data, size_t length); +v8::MaybeLocal New(Environment* env, size_t size); +v8::MaybeLocal New(Environment* env, const char* data, size_t len); +v8::MaybeLocal New(Environment* env, + char* data, + size_t length, + FreeCallback callback, + void* hint); +v8::MaybeLocal Use(Environment* env, char* data, size_t length); #endif // defined(NODE_WANT_INTERNALS) } // namespace Buffer diff --git a/src/node_crypto.cc b/src/node_crypto.cc index ba71eb73facfd0..2d64d9e1973266 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -942,7 +942,7 @@ void SecureContext::GetTicketKeys(const FunctionCallbackInfo& args) { SecureContext* wrap = Unwrap(args.Holder()); - Local buff = Buffer::New(wrap->env(), 48); + Local buff = Buffer::New(wrap->env(), 48).ToLocalChecked(); if (SSL_CTX_get_tlsext_ticket_keys(wrap->ctx_, Buffer::Data(buff), Buffer::Length(buff)) != 1) { @@ -1006,7 +1006,7 @@ void SecureContext::GetCertificate(const FunctionCallbackInfo& args) { return args.GetReturnValue().Set(Null(env->isolate())); int size = i2d_X509(cert, nullptr); - Local buff = Buffer::New(env, size); + Local buff = Buffer::New(env, size).ToLocalChecked(); unsigned char* serialized = reinterpret_cast( Buffer::Data(buff)); i2d_X509(cert, &serialized); @@ -1109,15 +1109,16 @@ int SSLWrap::NewSessionCallback(SSL* s, SSL_SESSION* sess) { return 0; // Serialize session - Local buff = Buffer::New(env, size); + Local buff = Buffer::New(env, size).ToLocalChecked(); unsigned char* serialized = reinterpret_cast( Buffer::Data(buff)); memset(serialized, 0, size); i2d_SSL_SESSION(sess, &serialized); - Local session = Buffer::New(env, - reinterpret_cast(sess->session_id), - sess->session_id_length); + Local session = Buffer::New( + env, + reinterpret_cast(sess->session_id), + sess->session_id_length).ToLocalChecked(); Local argv[] = { session, buff }; w->new_session_wait_ = true; w->MakeCallback(env->onnewsession_string(), ARRAY_SIZE(argv), argv); @@ -1138,7 +1139,7 @@ void SSLWrap::OnClientHello(void* arg, Local buff = Buffer::New( env, reinterpret_cast(hello.session_id()), - hello.session_size()); + hello.session_size()).ToLocalChecked(); hello_obj->Set(env->session_id_string(), buff); if (hello.servername() == nullptr) { hello_obj->Set(env->servername_string(), String::Empty(env->isolate())); @@ -1349,7 +1350,7 @@ static Local X509ToObject(Environment* env, X509* cert) { // Raw DER certificate int size = i2d_X509(cert, nullptr); - Local buff = Buffer::New(env, size); + Local buff = Buffer::New(env, size).ToLocalChecked(); unsigned char* serialized = reinterpret_cast( Buffer::Data(buff)); i2d_X509(cert, &serialized); @@ -1596,11 +1597,12 @@ void SSLWrap::GetTLSTicket(const FunctionCallbackInfo& args) { if (sess == nullptr || sess->tlsext_tick == nullptr) return; - Local buf = Buffer::New(env, - reinterpret_cast(sess->tlsext_tick), - sess->tlsext_ticklen); + Local buff = Buffer::New( + env, + reinterpret_cast(sess->tlsext_tick), + sess->tlsext_ticklen).ToLocalChecked(); - args.GetReturnValue().Set(buf); + args.GetReturnValue().Set(buff); } @@ -1880,7 +1882,7 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { arg = Buffer::New( env, reinterpret_cast(const_cast(resp)), - len); + len).ToLocalChecked(); } w->MakeCallback(env->onocspresponse_string(), 1, &arg); @@ -2897,7 +2899,7 @@ void CipherBase::GetAuthTag(const FunctionCallbackInfo& args) { unsigned int out_len = 0; if (cipher->GetAuthTag(&out, &out_len)) { - Local buf = Buffer::Use(env, out, out_len); + Local buf = Buffer::Use(env, out, out_len).ToLocalChecked(); args.GetReturnValue().Set(buf); } else { env->ThrowError("Attempting to get auth tag in unsupported state"); @@ -3014,7 +3016,8 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { "Trying to add data in unsupported state"); } - Local buf = Buffer::New(env, reinterpret_cast(out), out_len); + Local buf = + Buffer::New(env, reinterpret_cast(out), out_len).ToLocalChecked(); if (out) delete[] out; @@ -3089,8 +3092,11 @@ void CipherBase::Final(const FunctionCallbackInfo& args) { } } - args.GetReturnValue().Set( - Buffer::New(env, reinterpret_cast(out_value), out_len)); + Local buf = Buffer::New( + env, + reinterpret_cast(out_value), + out_len).ToLocalChecked(); + args.GetReturnValue().Set(buf); delete[] out_value; } @@ -3868,8 +3874,11 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { } } - args.GetReturnValue().Set( - Buffer::New(env, reinterpret_cast(out_value), out_len)); + Local vbuf = Buffer::New( + env, + reinterpret_cast(out_value), + out_len).ToLocalChecked(); + args.GetReturnValue().Set(vbuf); delete[] out_value; } @@ -4364,7 +4373,8 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to compute ECDH key"); } - args.GetReturnValue().Set(Buffer::Use(env, out, out_len)); + Local buf = Buffer::Use(env, out, out_len).ToLocalChecked(); + args.GetReturnValue().Set(buf); } @@ -4400,9 +4410,9 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get public key"); } - args.GetReturnValue().Set(Buffer::Use(env, - reinterpret_cast(out), - size)); + Local buf = + Buffer::Use(env, reinterpret_cast(out), size).ToLocalChecked(); + args.GetReturnValue().Set(buf); } @@ -4427,9 +4437,9 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to convert ECDH private key to Buffer"); } - args.GetReturnValue().Set(Buffer::Use(env, - reinterpret_cast(out), - size)); + Local buf = + Buffer::Use(env, reinterpret_cast(out), size).ToLocalChecked(); + args.GetReturnValue().Set(buf); } @@ -4831,7 +4841,7 @@ void RandomBytesCheck(RandomBytesRequest* req, Local argv[2]) { size_t size; req->return_memory(&data, &size); argv[0] = Null(req->env()->isolate()); - argv[1] = Buffer::Use(req->env()->isolate(), data, size); + argv[1] = Buffer::Use(req->env(), data, size).ToLocalChecked(); } } diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 79428fcd73ccc5..97bccfd4b789a6 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -167,7 +167,7 @@ void SyncProcessStdioPipe::Close() { Local SyncProcessStdioPipe::GetOutputAsBuffer(Environment* env) const { size_t length = OutputLength(); - Local js_buffer = Buffer::New(env, length); + Local js_buffer = Buffer::New(env, length).ToLocalChecked(); CopyOutput(Buffer::Data(js_buffer)); return js_buffer; } diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 540639d458050c..3097eac4859761 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -223,7 +223,8 @@ void StreamWrap::OnReadImpl(ssize_t nread, CHECK_EQ(pending, UV_UNKNOWN_HANDLE); } - wrap->EmitData(nread, Buffer::Use(env, base, nread), pending_obj); + Local obj = Buffer::Use(env, base, nread).ToLocalChecked(); + wrap->EmitData(nread, obj, pending_obj); } diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 0bdb8aad914d34..36525a73ac84c3 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -19,6 +19,7 @@ using v8::Handle; using v8::HandleScope; using v8::Isolate; using v8::Local; +using v8::Object; using v8::String; using v8::Value; @@ -702,7 +703,10 @@ Local StringBytes::Encode(Isolate* isolate, Local val; switch (encoding) { case BUFFER: - return scope.Escape(Buffer::New(isolate, buf, buflen)); + { + Local vbuf = Buffer::New(isolate, buf, buflen).ToLocalChecked(); + return scope.Escape(vbuf); + } case ASCII: if (contains_non_ascii(buf, buflen)) { diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index d4c7c9055d529d..fbb35fdb5e6173 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -660,7 +660,7 @@ void TLSWrap::OnReadSelf(ssize_t nread, TLSWrap* wrap = static_cast(ctx); Local buf_obj; if (buf != nullptr) - buf_obj = Buffer::Use(wrap->env(), buf->base, buf->len); + buf_obj = Buffer::Use(wrap->env(), buf->base, buf->len).ToLocalChecked(); wrap->EmitData(nread, buf_obj, Local()); } diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index fe0ed76dd5e68b..791ef76848cf43 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -408,7 +408,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, } char* base = static_cast(realloc(buf->base, nread)); - argv[2] = Buffer::Use(env, base, nread); + argv[2] = Buffer::Use(env, base, nread).ToLocalChecked(); argv[3] = AddressToJS(env, addr); wrap->MakeCallback(env->onmessage_string(), ARRAY_SIZE(argv), argv); }