dgram: setMulticastInterface() outgoing interface selection by lostnet · Pull Request #7855 · nodejs/node · GitHub
Skip to content
Closed
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
81 changes: 81 additions & 0 deletions doc/api/dgram.md
15 changes: 15 additions & 0 deletions lib/dgram.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,21 @@ Socket.prototype.setMulticastLoopback = function(arg) {
};


Socket.prototype.setMulticastInterface = function(interfaceAddress) {
this._healthCheck();

if (typeof interfaceAddress !== 'string') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we be able to get away with checking for IP addresses only here (net.isIP())?

@lostnet lostnet Aug 5, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

net.isIP() makes it look like no checks are necessary either in JS or the wrapper:

Its existing tests don't cover Scope, so I added some:
assert.equal(net.isIP('::%eth1'), 6);
assert.equal(net.isIP('::1%1'), 6);
These passed leading me to verify it is using a C implementation to get to libuv:

static void IsIP(const FunctionCallbackInfo<Value>& args) {

yet, this doesn't need any wrapper:

node/lib/net.js

Line 1581 in 863952e

exports.isIP = cares.isIP;

and still passes:
assert.equal(net.isIP(), 0);

If net.isIP() is correct then no check on interfaceAddress in JS and removing the argument checks should be fine?

**I've run the tests with gdb and determined that net.isIP() passes since *ip ends up as the string "undefined" when uv_inet_pton is called. So the wrapper is more conservative than net.isIP() but in a way we already discussed and throwing separate missing/non string typeerrors independently EINVALs provides better debugging while calling net.isIP() would duplicate code just to rename EINVAL something else.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lostnet would you please cover the missing parts for net.isIP() as you described in another patch?

throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
'interfaceAddress',
'string');
}

const err = this._handle.setMulticastInterface(interfaceAddress);
if (err) {
throw errnoException(err, 'setMulticastInterface');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should use the new error system

}
};

Socket.prototype.addMembership = function(multicastAddress,
interfaceAddress) {
this._healthCheck();
Expand Down
17 changes: 17 additions & 0 deletions src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ void UDPWrap::Initialize(Local<Object> target,
GetSockOrPeerName<UDPWrap, uv_udp_getsockname>);
env->SetProtoMethod(t, "addMembership", AddMembership);
env->SetProtoMethod(t, "dropMembership", DropMembership);
env->SetProtoMethod(t, "setMulticastInterface", SetMulticastInterface);
env->SetProtoMethod(t, "setMulticastTTL", SetMulticastTTL);
env->SetProtoMethod(t, "setMulticastLoopback", SetMulticastLoopback);
env->SetProtoMethod(t, "setBroadcast", SetBroadcast);
Expand Down Expand Up @@ -238,6 +239,22 @@ X(SetMulticastLoopback, uv_udp_set_multicast_loop)

#undef X

void UDPWrap::SetMulticastInterface(const FunctionCallbackInfo<Value>& args) {
UDPWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap,
args.Holder(),
args.GetReturnValue().Set(UV_EBADF));

CHECK_EQ(args.Length(), 1);
CHECK(args[0]->IsString());

Utf8Value iface(args.GetIsolate(), args[0]);

const char* iface_cstr = *iface;

int err = uv_udp_set_multicast_interface(&wrap->handle_, iface_cstr);
args.GetReturnValue().Set(err);
}

void UDPWrap::SetMembership(const FunctionCallbackInfo<Value>& args,
uv_membership membership) {
Expand Down
2 changes: 2 additions & 0 deletions src/udp_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class UDPWrap: public HandleWrap {
static void RecvStop(const v8::FunctionCallbackInfo<v8::Value>& args);
static void AddMembership(const v8::FunctionCallbackInfo<v8::Value>& args);
static void DropMembership(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetMulticastInterface(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetMulticastTTL(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetMulticastLoopback(
const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
4 changes: 3 additions & 1 deletion test/internet/test-dgram-multicast-multi-process.js
Loading