From 353bc96c85d6ba7b3c456f51702fe110bf6b213a Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sat, 26 Mar 2022 21:09:59 +0530 Subject: [PATCH] src: properly report exceptions from AddressToJS() Signed-off-by: Darshan Sen PR-URL: https://github.com/nodejs/node/pull/42054 Reviewed-By: Matteo Collina --- lib/dgram.js | 8 +++++++- src/js_udp_wrap.cc | 14 ++++++++++---- src/node_internals.h | 2 +- src/node_sockaddr-inl.h | 2 +- src/node_sockaddr.cc | 4 +++- src/node_sockaddr.h | 2 +- src/tcp_wrap.cc | 9 ++++----- src/udp_wrap.cc | 42 +++++++++++++++++++++++++++++++++++++++-- 8 files changed, 67 insertions(+), 16 deletions(-) diff --git a/lib/dgram.js b/lib/dgram.js index 221afcf5bb0c84..5dbc2f22dab658 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -159,7 +159,7 @@ function startListening(socket) { const state = socket[kStateSymbol]; state.handle.onmessage = onMessage; - // Todo: handle errors + state.handle.onerror = onError; state.handle.recvStart(); state.receiving = true; state.bindState = BIND_STATE_BOUND; @@ -923,6 +923,12 @@ function onMessage(nread, handle, buf, rinfo) { } +function onError(nread, handle, error) { + const self = handle[owner_symbol]; + return self.emit('error', error); +} + + Socket.prototype.ref = function() { const handle = this[kStateSymbol].handle; diff --git a/src/js_udp_wrap.cc b/src/js_udp_wrap.cc index c01289033e6764..3f02771ee1a907 100644 --- a/src/js_udp_wrap.cc +++ b/src/js_udp_wrap.cc @@ -5,6 +5,9 @@ #include +// TODO(RaisinTen): Replace all uses with empty `v8::Maybe`s. +#define JS_EXCEPTION_PENDING UV_EPROTO + namespace node { using errors::TryCatchScope; @@ -60,7 +63,7 @@ int JSUDPWrap::RecvStart() { Context::Scope context_scope(env()->context()); TryCatchScope try_catch(env()); Local value; - int32_t value_int = UV_EPROTO; + int32_t value_int = JS_EXCEPTION_PENDING; if (!MakeCallback(env()->onreadstart_string(), 0, nullptr).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { if (try_catch.HasCaught() && !try_catch.HasTerminated()) @@ -74,7 +77,7 @@ int JSUDPWrap::RecvStop() { Context::Scope context_scope(env()->context()); TryCatchScope try_catch(env()); Local value; - int32_t value_int = UV_EPROTO; + int32_t value_int = JS_EXCEPTION_PENDING; if (!MakeCallback(env()->onreadstop_string(), 0, nullptr).ToLocal(&value) || !value->Int32Value(env()->context()).To(&value_int)) { if (try_catch.HasCaught() && !try_catch.HasTerminated()) @@ -90,7 +93,7 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, Context::Scope context_scope(env()->context()); TryCatchScope try_catch(env()); Local value; - int64_t value_int = UV_EPROTO; + int64_t value_int = JS_EXCEPTION_PENDING; size_t total_len = 0; MaybeStackBuffer, 16> buffers(nbufs); @@ -100,10 +103,13 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs, total_len += bufs[i].len; } + Local address; + if (!AddressToJS(env(), addr).ToLocal(&address)) return value_int; + Local args[] = { listener()->CreateSendWrap(total_len)->object(), Array::New(env()->isolate(), buffers.out(), nbufs), - AddressToJS(env(), addr) + address, }; if (!MakeCallback(env()->onwrite_string(), arraysize(args), args) diff --git a/src/node_internals.h b/src/node_internals.h index fc82053cdbd262..c9d7cca47c8b2f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -58,7 +58,7 @@ class Environment; // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object. // Sets address and port properties on the info object and returns it. // If |info| is omitted, a new object is returned. -v8::Local AddressToJS( +v8::MaybeLocal AddressToJS( Environment* env, const sockaddr* addr, v8::Local info = v8::Local()); diff --git a/src/node_sockaddr-inl.h b/src/node_sockaddr-inl.h index 0b2361595f3db7..e16a09b04c7d6f 100644 --- a/src/node_sockaddr-inl.h +++ b/src/node_sockaddr-inl.h @@ -157,7 +157,7 @@ void SocketAddress::Update(const sockaddr* data, size_t len) { memcpy(&address_, data, len); } -v8::Local SocketAddress::ToJS( +v8::MaybeLocal SocketAddress::ToJS( Environment* env, v8::Local info) const { return AddressToJS(env, data(), info); diff --git a/src/node_sockaddr.cc b/src/node_sockaddr.cc index 09a74f302923f7..f6afaaac4f3d66 100644 --- a/src/node_sockaddr.cc +++ b/src/node_sockaddr.cc @@ -847,7 +847,9 @@ void SocketAddressBase::LegacyDetail(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); SocketAddressBase* base; ASSIGN_OR_RETURN_UNWRAP(&base, args.Holder()); - args.GetReturnValue().Set(base->address_->ToJS(env)); + Local address; + if (!base->address_->ToJS(env).ToLocal(&address)) return; + args.GetReturnValue().Set(address); } SocketAddressBase::SocketAddressBase( diff --git a/src/node_sockaddr.h b/src/node_sockaddr.h index 4cc5291ceefead..0a4633b9a33d7e 100644 --- a/src/node_sockaddr.h +++ b/src/node_sockaddr.h @@ -131,7 +131,7 @@ class SocketAddress : public MemoryRetainer { static SocketAddress FromPeerName(const uv_udp_t& handle); static SocketAddress FromPeerName(const uv_tcp_t& handle); - inline v8::Local ToJS( + inline v8::MaybeLocal ToJS( Environment* env, v8::Local obj = v8::Local()) const; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 669206fc6bf94b..538f0355491c4a 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -342,9 +342,9 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args, // also used by udp_wrap.cc -Local AddressToJS(Environment* env, - const sockaddr* addr, - Local info) { +MaybeLocal AddressToJS(Environment* env, + const sockaddr* addr, + Local info) { EscapableHandleScope scope(env->isolate()); char ip[INET6_ADDRSTRLEN + UV_IF_NAMESIZE]; const sockaddr_in* a4; @@ -371,8 +371,7 @@ Local AddressToJS(Environment* env, &scopeidlen); if (r) { env->ThrowUVException(r, "uv_if_indextoiid"); - // TODO(addaleax): Do proper MaybeLocal handling here - return scope.Escape(info); + return {}; } } port = ntohs(a6->sin6_port); diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 4a0c6aeaa940d2..127a1a6e5d8fe7 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -22,6 +22,7 @@ #include "udp_wrap.h" #include "env-inl.h" #include "node_buffer.h" +#include "node_errors.h" #include "node_sockaddr-inl.h" #include "handle_wrap.h" #include "req_wrap-inl.h" @@ -29,6 +30,7 @@ namespace node { +using errors::TryCatchScope; using v8::Array; using v8::ArrayBuffer; using v8::BackingStore; @@ -728,9 +730,45 @@ void UDPWrap::OnRecv(ssize_t nread, bs = BackingStore::Reallocate(isolate, std::move(bs), nread); } + Local address; + { + bool has_caught = false; + { + TryCatchScope try_catch(env); + if (!AddressToJS(env, addr).ToLocal(&address)) { + DCHECK(try_catch.HasCaught() && !try_catch.HasTerminated()); + argv[2] = try_catch.Exception(); + DCHECK(!argv[2].IsEmpty()); + has_caught = true; + } + } + if (has_caught) { + DCHECK(!argv[2].IsEmpty()); + MakeCallback(env->onerror_string(), arraysize(argv), argv); + return; + } + } + Local ab = ArrayBuffer::New(isolate, std::move(bs)); - argv[2] = Buffer::New(env, ab, 0, ab->ByteLength()).ToLocalChecked(); - argv[3] = AddressToJS(env, addr); + { + bool has_caught = false; + { + TryCatchScope try_catch(env); + if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&argv[2])) { + DCHECK(try_catch.HasCaught() && !try_catch.HasTerminated()); + argv[2] = try_catch.Exception(); + DCHECK(!argv[2].IsEmpty()); + has_caught = true; + } + } + if (has_caught) { + DCHECK(!argv[2].IsEmpty()); + MakeCallback(env->onerror_string(), arraysize(argv), argv); + return; + } + } + + argv[3] = address; MakeCallback(env->onmessage_string(), arraysize(argv), argv); }