diversario / node-ssdp

node.js SSDP client/server.
MIT License
274 stars 117 forks source link

Default behaviour with multiple NICs is not good #109

Open djcsdy opened 3 years ago

djcsdy commented 3 years ago

Summary:

Judging by some comments in the code and documentation, I think you already know that node-ssdp doesn't behave very well when there are multiple NICs on the system. Unfortunately this is a more common scenario than you might think because many systems, especially developer systems, have virtual NICs for VMs and the like. For example my Windows system has a virtual NIC that is used for WSL and Hyper-V.

The problem is that, although node-ssdp creates a socket for each interface, by default the sockets are not bound to any particular interface. Normally when you send a packet through such a socket, the OS decides which NIC to route the packet through based on the destination IP address. But multicast addresses are associated with every interface, so the OS can choose to send a multicast packet through any NIC essentially at random - the behaviour is undefined. Letting the OS choose here isn't really a viable option because the code will work as expected in some cases but not in others and there will be no logical reason for it.

On Windows, as far as I can tell, in the above circumstances the OS always sends multicast packets to the first available interface, which in my case is the WSL/Hyper-V virtual NIC, so the only SSDP services node-ssdp can find by default for me are the ones hosted on localhost.

I'm not sure what the behaviour of other OSes is. The best case is that the OS sends multicast packets over every interface, in which case the behaviour of node-ssdp is undesirable because it creates one socket per interface, so node-ssdp would send duplicate packets on every interface (I don't think any OS would actually do this, but who knows). The worst case is that the OS picks one interface - the same interface for every socket, and by Murphy's Law probably not the one you wanted - in which case node-ssdp will send duplicate packets on one interface, more or less chosen at random, and ignore the others.

The workaround appears to be to set the explicitSocketBind option. But this doesn't make sense either, because why would correct behaviour be an option, and why would it be off by default? Since node-ssdp creates a socket for every interface, it should be safe to bind that socket to its associated interface. Does this option cause some bad side effect I'm not aware of?

I notice that there is one place in the code where an interface is specified explicitly: https://github.com/diversario/node-ssdp/blob/v4.0.1/lib/index.js#L264. But the addMembership function only tells the socket to listen for multicast requests on that particular interface. It doesn't tell the socket where to send multicast requests.

There is an equivalent function setMulticastInterface, which tells the socket where to send requests, so if for some reason you don't want to bind the socket to a specific interface you might consider adding a call to setMulticastInterface(iface) above the call to addMembership(self._ssdpIp, iface). This does require node 8 though, and I notice that you appear to have deliberately maintained support for older node versions.

I was going to make a PR for this but I wasn't sure if there was some good reason for the existence of the explicitSocketBind option, or which of the above approaches you'd prefer. So I wrote this long rambling explanation instead. Let me know if you'd like me to create a PR. Either way it should be a one line change.