node-discover icon indicating copy to clipboard operation
node-discover copied to clipboard

Resolve issue with default broadcast addresses

Open my2b opened this issue 9 years ago • 4 comments

Use list of network interfaces in order to find broadcast addresses for each interface and use them instead 255.255.255.255 because of issues on Windows: see thread How to fix the global broadcast address (255.255.255.255) behavior on Windows?

my2b avatar Oct 14 '16 18:10 my2b

Hey @my2b. Thank you for your contribution. This looks pretty good.

The only difference that I would like to see is to only execute the block of code that determines the broadcast addresses by interface when broadcast is not specified. Like:

if (self.broadcast) {
    self.destination = self.broadcast
}
else {
          /**
             * Get broadcast addresses for all network interfaces instead 255.255.255.255
             * because of issues with broadcast using this method on Windows
             */
            var networkInterfaces = os.networkInterfaces(),
                broadcastAddresses = [];

            Object.keys(networkInterfaces).forEach(function (ifname) {
                networkInterfaces[ifname].forEach(function (iface) {
                    if ('IPv4' !== iface.family || iface.internal !== false) {
                        // skip over internal (i.e. 127.0.0.1) and non-ipv4 addresses
                        return;
                    }

                    var tabytes = (iface.address).split("."),
                        tsbytes = (iface.netmask).split(".");

                    // Calculate Broadcast address
                    var tbaddr = ((tabytes[0] & tsbytes[0]) | (255 ^ tsbytes[0])) + "."
                        + ((tabytes[1] & tsbytes[1]) | (255 ^ tsbytes[1])) + "."
                        + ((tabytes[2] & tsbytes[2]) | (255 ^ tsbytes[2])) + "."
                        + ((tabytes[3] & tsbytes[3]) | (255 ^ tsbytes[3]));

                    broadcastAddresses.push(tbaddr);
                });
            });

            self.destination = broadcastAddresses;
}

That way if for some odd reason os.networkInterfaces() doesn't work well across platforms it won't be executed at all if broadcast is manually specified.

What do you think about that?

wankdanker avatar Oct 14 '16 19:10 wankdanker

Thanks for fast answer! I absolutely agree with your additional changes, in my opinion this is a good way how to protect your code and I like it.

If there is nothing else to change/add - in which way can we add this changes? I am new in GitHub so let me know please how can I contribute new changes: I need to create another pull request or change this one somehow or you may do it by yourself?

Thanks!

my2b avatar Oct 15 '16 15:10 my2b

Hi @my2b! Just make the changes in your local git repo, commit them and then push them to your master branch on github. That will update this pull request. No need for a separate pull request. Once this pull request is looking good, I'll merge it and we'll close it up!

Thank you for your contribution.

Dan

wankdanker avatar Oct 15 '16 17:10 wankdanker

Done, @wankdanker Thank you!

my2b avatar Oct 15 '16 18:10 my2b