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

os.networkInterfaces() does not expose interface's broadcast address #23437

Closed
pprindeville opened this issue Oct 12, 2018 · 12 comments
Closed
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. stale

Comments

@pprindeville
Copy link

  • Version: v11.0.0-pre
  • Platform: Linux (Ubuntu 16.04.4-LTS)
  • Subsystem: os

The overwhelming majority of network interfaces are Ethernet or Wifi (on servers, almost always Ethernet) which both support broadcasting. The broadcast address is typically the host address bit-wise ANDed with the net mask, and then bit-wise ORed with the bit-wise complement of the net mask.

But not always. Some arcane network configurations still use zero as the host id to signify the broadcast address (i.e. just the host address bit-wise ANDed with the net mask).

That is, for an address such as 192.168.1.12/24 (i.e. net mask of 255.255.255.0), the broadcast address would typically be 192.168.1.255.

That said, the correct behavior is always to take the broadcast address directly from the interface's configuration state, rather than guessing at it.

A mechanism to do exactly that is needed.

@richardlau richardlau added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Oct 12, 2018
@richardlau
Copy link
Member

See discussions in libuv/libuv#158 and libuv/libuv#1371 about extending libuv's support for network interfaces.

@pprindeville
Copy link
Author

pprindeville commented Oct 13, 2018

Please also see Host Requirements: Broadcasts.

@richardlau richardlau added the feature request Issues that request new features to be added to Node.js. label Oct 13, 2018
@silverwind
Copy link
Contributor

This is a IPv4-only feature right? To my knowledge, there is no such concept as a broadcast on IPv6. Do the underlying OS APIs still expose a broadcast address on IPv6?

@pprindeville
Copy link
Author

@silverwind That's correct. Is there a preferred way to export constants in JS? See:

https://github.com/libuv/libuv/pull/2033/files#diff-56343bd7ad8bf449c2693082fb3e4081R1097

how do I export this (IN6ADDR_ALLHOSTS_GROUP) into the JS space from C as a const? I'm a C programmer supporting a JS team...

@addaleax
Copy link
Member

@pprindeville Using NODE_DEFINE_CONSTANT(target, IN6ADDR_ALLHOSTS_GROUP); in the Initialize function of node_os.cc should work?

@pprindeville
Copy link
Author

pprindeville commented Oct 17, 2018

Even though the argument is of type const struct in6_addr? This looks problematic:

    v8::Local<v8::Number> constant_value =                                    \
        v8::Number::New(isolate, static_cast<double>(constant));

pprindeville added a commit to pprindeville/node that referenced this issue Oct 19, 2018
Not using the correct broadcast address in applications can cause
disasterous results. Rather than letting the programmer guess at
the broadcast address and try to derive it correctly, allow him to
query the system instead for the correctly configured state.

Fixes: nodejs#23437
Depends-on: libuv/libuv#2033
@pprindeville
Copy link
Author

@addaleax Okay, got that resolved so it’s exporting in JS properly.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 4, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Mar 4, 2022
@pprindeville
Copy link
Author

There's been progress in libuv, but I'm trying to add test coverage...

@bnoordhuis
Copy link
Member

@pprindeville if you're referring to libuv/libuv#1371, that never got merged. You're of course welcome to dust it off and re-submit it for review. I think it was close.

@github-actions github-actions bot removed the stale label Mar 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2022

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Sep 5, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. stale
Projects
None yet
Development

No branches or pull requests

5 participants