Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve issue with default broadcast addresses #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

my2b
Copy link

@my2b my2b commented Oct 14, 2016

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
Copy link
Owner

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
Copy link
Author

my2b commented Oct 15, 2016

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
Copy link
Owner

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
Copy link
Author

my2b commented Oct 15, 2016

Done, @wankdanker
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants