agnat / node_wake_on_lan

Wake-on-LAN utilities for node.js
MIT License
223 stars 24 forks source link

add option for srcAddress and srcPort #3

Closed Links2004 closed 6 years ago

Links2004 commented 9 years ago

add option for srcAddress and srcPort. needed if you have an server with multiple interfaces for example LAN + WAN then you dont want to send the WOL msg to over the WAN interface

agnat commented 9 years ago

Setting a source address makes sense.

However, IMO setting a port doesn't. On the contrary. By letting the OS pick a port the old code avoided all kinds of trouble (port busy, &c.). The new code randomly fails whenever your port randomization has bad luck.

Also, why the odd port range?

Links2004 commented 9 years ago

you are right the port selection can fail.

The port range has no rail meaning, i only wanted to avoid the lower ranges, in use here(10000 - 60000). all < 1024 is root only. and < 10000 has an higher change to be already in use.

for socket.bind port is required, so i have no possibility to only set the address.

socket.bind(port[, address][, callback]) https://nodejs.org/api/dgram.html#dgram_socket_bind_port_address_callback

is there a better method to bind the udp socked to an specific interface?

agnat commented 9 years ago

IIRC using port 0 is what you want. But this is your contribution. So, look it up... ;)

Links2004 commented 9 years ago

0 as port is working fin. i have verified with Wireshark.

agnat commented 9 years ago

Yeah, well. You verified it on one platform. However, Unix Network Programming says I'm right... which is good enough for me.

Now, why keep the port option at all? As I said: I can not see any use case. It only serves to shoot your own foot by catching a busy port. Please remove it.

Links2004 commented 9 years ago

is removed