From 799da2419dc6ae6fcdf95fe9517086a517b2a9d0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 16 May 2016 13:22:17 -0700 Subject: [PATCH 1/2] doc: add `added` info for `dgram.setBroadcast()` Since I was doing the necessary git spelunking anyway, I took the time to add the YAML information into the docs about when `setBroadcast()` first appeared in its current form. --- doc/api/dgram.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/api/dgram.md b/doc/api/dgram.md index 53eeb818986fec..953d4bf8c9c0b3 100644 --- a/doc/api/dgram.md +++ b/doc/api/dgram.md @@ -283,6 +283,9 @@ not work because the packet will get silently dropped without informing the source that the data did not reach its intended recipient. ### socket.setBroadcast(flag) + * `flag` {Boolean} From 0f5960bd4181db78bcbffd8717254ab6a922cebe Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 16 May 2016 13:24:08 -0700 Subject: [PATCH 2/2] test,dgram: add tests for setBroadcast() The only tests for `setBroadcast()` (from the `dgram` module) were in `test/internet` which means they almost never get run. This adds a minimal test that can check JS-land functionality in `test/parallel`. I also expanded a comment and did some minor formatting on the existing `test/internet` test. If there were an easy and reliable way to check for the BROADCAST flag on an interface, it's possible that a version of the test could be moved to `test/sequential` or `test/parallel` once it was modified to only use internal networks. --- .../test-dgram-broadcast-multi-process.js | 8 ++-- test/parallel/test-dgram-setBroadcast.js | 39 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-dgram-setBroadcast.js diff --git a/test/internet/test-dgram-broadcast-multi-process.js b/test/internet/test-dgram-broadcast-multi-process.js index 21133f2956da16..0925432571f639 100644 --- a/test/internet/test-dgram-broadcast-multi-process.js +++ b/test/internet/test-dgram-broadcast-multi-process.js @@ -20,7 +20,9 @@ if (common.inFreeBSDJail) { return; } -// take the first non-internal interface as the address for binding +// Take the first non-internal interface as the address for binding. +// Ideally, this should check for whether or not an interface is set up for +// BROADCAST and favor internal/private interfaces. get_bindAddress: for (var name in networkInterfaces) { var interfaces = networkInterfaces[name]; for (var i = 0; i < interfaces.length; i++) { @@ -209,7 +211,7 @@ if (process.argv[2] === 'child') { receivedMessages.push(buf); - process.send({ message: buf.toString() }); + process.send({message: buf.toString()}); if (receivedMessages.length == messages.length) { process.nextTick(function() { @@ -228,7 +230,7 @@ if (process.argv[2] === 'child') { }); listenSocket.on('listening', function() { - process.send({ listening: true }); + process.send({listening: true}); }); listenSocket.bind(common.PORT); diff --git a/test/parallel/test-dgram-setBroadcast.js b/test/parallel/test-dgram-setBroadcast.js new file mode 100644 index 00000000000000..2d78a133ad3f17 --- /dev/null +++ b/test/parallel/test-dgram-setBroadcast.js @@ -0,0 +1,39 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const dgram = require('dgram'); + +const setup = () => { + return dgram.createSocket({type: 'udp4', reuseAddr: true}); +}; + +const teardown = (socket) => { + if (socket.close) + socket.close(); +}; + +const runTest = (testCode, expectError) => { + const socket = setup(); + const assertion = expectError ? assert.throws : assert.doesNotThrow; + const wrapped = () => { testCode(socket); }; + assertion(wrapped, expectError); + teardown(socket); +}; + +// Should throw EBADF if socket is never bound. +runTest((socket) => { socket.setBroadcast(true); }, /EBADF/); + +// Should not throw if broadcast set to false after binding. +runTest((socket) => { + socket.bind(common.PORT, common.localhostIPv4, () => { + socket.setBroadcast(false); + }); +}); + +// Should not throw if broadcast set to true after binding. +runTest((socket) => { + socket.bind(common.PORT, common.localhostIPv4, () => { + socket.setBroadcast(true); + }); +});