src: properly report exceptions from AddressToJS()

Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #42054 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · Apr 4, 2022 · 8e8f129 · 8e8f129
1 parent 3b810ca
commit 8e8f129
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 16 deletions.
diff --git 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
@@ -5,6 +5,9 @@
 
 #include <algorithm>
 
+// 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> 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> 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> value;
-  int64_t value_int = UV_EPROTO;
+  int64_t value_int = JS_EXCEPTION_PENDING;
   size_t total_len = 0;
 
   MaybeStackBuffer<Local<Value>, 16> buffers(nbufs);
@@ -100,10 +103,13 @@ ssize_t JSUDPWrap::Send(uv_buf_t* bufs,
     total_len += bufs[i].len;
   }
 
+  Local<Object> address;
+  if (!AddressToJS(env(), addr).ToLocal(&address)) return value_int;
+
   Local<Value> 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
@@ -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<v8::Object> AddressToJS(
+v8::MaybeLocal<v8::Object> AddressToJS(
     Environment* env,
     const sockaddr* addr,
     v8::Local<v8::Object> info = v8::Local<v8::Object>());

diff --git 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<v8::Object> SocketAddress::ToJS(
+v8::MaybeLocal<v8::Object> SocketAddress::ToJS(
     Environment* env,
     v8::Local<v8::Object> info) const {
   return AddressToJS(env, data(), info);

diff --git a/src/node_sockaddr.cc b/src/node_sockaddr.cc
@@ -847,7 +847,9 @@ void SocketAddressBase::LegacyDetail(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   SocketAddressBase* base;
   ASSIGN_OR_RETURN_UNWRAP(&base, args.Holder());
-  args.GetReturnValue().Set(base->address_->ToJS(env));
+  Local<Object> 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
@@ -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<v8::Object> ToJS(
+  inline v8::MaybeLocal<v8::Object> ToJS(
       Environment* env,
       v8::Local<v8::Object> obj = v8::Local<v8::Object>()) const;
 

diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc
@@ -342,9 +342,9 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args,
 
 
 // also used by udp_wrap.cc
-Local<Object> AddressToJS(Environment* env,
-                          const sockaddr* addr,
-                          Local<Object> info) {
+MaybeLocal<Object> AddressToJS(Environment* env,
+                               const sockaddr* addr,
+                               Local<Object> info) {
   EscapableHandleScope scope(env->isolate());
   char ip[INET6_ADDRSTRLEN + UV_IF_NAMESIZE];
   const sockaddr_in* a4;
@@ -371,8 +371,7 @@ Local<Object> 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
@@ -22,13 +22,15 @@
 #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"
 #include "util-inl.h"
 
 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<Object> 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<ArrayBuffer> 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);
 }