Skip to content

Commit

Permalink
src: properly report exceptions from AddressToJS()
Browse files Browse the repository at this point in the history
Signed-off-by: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#42054
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
RaisinTen authored and guangwong committed Oct 10, 2022
1 parent 83e3128 commit 83c1509
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 16 deletions.
8 changes: 7 additions & 1 deletion lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
14 changes: 10 additions & 4 deletions src/js_udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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);
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>());
Expand Down
2 changes: 1 addition & 1 deletion src/node_sockaddr-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/node_sockaddr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/node_sockaddr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
9 changes: 4 additions & 5 deletions src/tcp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
42 changes: 40 additions & 2 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit 83c1509

Please sign in to comment.