wankdanker / node-discover

Automatic and decentralized discovery and monitoring of nodejs instances with built in support for a variable number of master processes, service advertising and channel messaging.
229 stars 59 forks source link

Resolve issue with default broadcast addresses #25

Open my2b opened 7 years ago

my2b commented 7 years ago

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?

wankdanker commented 7 years ago

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?

my2b commented 7 years ago

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!

wankdanker commented 7 years ago

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

my2b commented 7 years ago

Done, @wankdanker Thank you!