Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: properly report exceptions from AddressToJS() #42054

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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