dgram: allow typed arrays in .send()

PR-URL: #22413 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
nodejs · Jun 30, 2020 · 6fca493 · 6fca493
1 parent 50cc0ed
commit 6fca493
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 35 deletions.
diff --git a/doc/api/dgram.md b/doc/api/dgram.md
@@ -384,6 +384,9 @@ socket is not connected.
 <!-- YAML
 added: v0.1.99
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/22413
+    description: The `msg` parameter can now be any `TypedArray` or `DataView`.
   - version: v8.0.0
     pr-url: https://github.com/nodejs/node/pull/11985
     description: The `msg` parameter can be an `Uint8Array` now.
@@ -403,7 +406,7 @@ changes:
     description: Added support for sending data on connected sockets.
 -->
 
-* `msg` {Buffer|Uint8Array|string|Array} Message to be sent.
+* `msg` {Buffer|TypedArray|DataView|string|Array} Message to be sent.
 * `offset` {integer} Offset in the buffer where the message starts.
 * `length` {integer} Number of bytes in the message.
 * `port` {integer} Destination port.
@@ -416,8 +419,8 @@ specified. Connected sockets, on the other hand, will use their associated
 remote endpoint, so the `port` and `address` arguments must not be set.
 
 The `msg` argument contains the message to be sent.
-Depending on its type, different behavior can apply. If `msg` is a `Buffer`
-or `Uint8Array`,
+Depending on its type, different behavior can apply. If `msg` is a `Buffer`,
+any `TypedArray` or a `DataView`,
 the `offset` and `length` specify the offset within the `Buffer` where the
 message begins and the number of bytes in the message, respectively.
 If `msg` is a `String`, then it is automatically converted to a `Buffer`
@@ -446,7 +449,8 @@ passed as the first argument to the `callback`. If a `callback` is not given,
 the error is emitted as an `'error'` event on the `socket` object.
 
 Offset and length are optional but both *must* be set if either are used.
-They are supported only when the first argument is a `Buffer` or `Uint8Array`.
+They are supported only when the first argument is a `Buffer`, a `TypedArray`,
+or a `DataView`.
 
 Example of sending a UDP packet to a port on `localhost`;
 

diff --git a/lib/dgram.js b/lib/dgram.js
@@ -54,7 +54,7 @@ const {
 } = require('internal/validators');
 const { Buffer } = require('buffer');
 const { deprecate } = require('internal/util');
-const { isUint8Array } = require('internal/util/types');
+const { isArrayBufferView } = require('internal/util/types');
 const EventEmitter = require('events');
 const {
   defaultTriggerAsyncIdScope,
@@ -455,15 +455,19 @@ Socket.prototype.sendto = function(buffer,
 function sliceBuffer(buffer, offset, length) {
   if (typeof buffer === 'string') {
     buffer = Buffer.from(buffer);
-  } else if (!isUint8Array(buffer)) {
+  } else if (!isArrayBufferView(buffer)) {
     throw new ERR_INVALID_ARG_TYPE('buffer',
-                                   ['Buffer', 'Uint8Array', 'string'], buffer);
+                                   ['Buffer',
+                                    'TypedArray',
+                                    'DataView',
+                                    'string'],
+                                   buffer);
   }
 
   offset = offset >>> 0;
   length = length >>> 0;
 
-  return buffer.slice(offset, offset + length);
+  return Buffer.from(buffer.buffer, buffer.byteOffset + offset, length);
 }
 
 
@@ -474,10 +478,10 @@ function fixBufferList(list) {
     const buf = list[i];
     if (typeof buf === 'string')
       newlist[i] = Buffer.from(buf);
-    else if (!isUint8Array(buf))
+    else if (!isArrayBufferView(buf))
       return null;
     else
-      newlist[i] = buf;
+      newlist[i] = Buffer.from(buf.buffer, buf.byteOffset, buf.byteLength);
   }
 
   return newlist;
@@ -581,16 +585,23 @@ Socket.prototype.send = function(buffer,
   if (!ArrayIsArray(buffer)) {
     if (typeof buffer === 'string') {
       list = [ Buffer.from(buffer) ];
-    } else if (!isUint8Array(buffer)) {
+    } else if (!isArrayBufferView(buffer)) {
       throw new ERR_INVALID_ARG_TYPE('buffer',
-                                     ['Buffer', 'Uint8Array', 'string'],
+                                     ['Buffer',
+                                      'TypedArray',
+                                      'DataView',
+                                      'string'],
                                      buffer);
     } else {
       list = [ buffer ];
     }
   } else if (!(list = fixBufferList(buffer))) {
     throw new ERR_INVALID_ARG_TYPE('buffer list arguments',
-                                   ['Buffer', 'string'], buffer);
+                                   ['Buffer',
+                                    'TypedArray',
+                                    'DataView',
+                                    'string'],
+                                   buffer);
   }
 
   if (!connected)

diff --git a/test/parallel/test-dgram-send-bad-arguments.js b/test/parallel/test-dgram-send-bad-arguments.js
@@ -36,7 +36,7 @@ function checkArgs(connected) {
       code: 'ERR_INVALID_ARG_TYPE',
       name: 'TypeError',
       message: 'The "buffer" argument must be of type string or an instance ' +
-      'of Buffer or Uint8Array. Received undefined'
+      'of Buffer, TypedArray, or DataView. Received undefined'
     }
   );
 
@@ -90,7 +90,7 @@ function checkArgs(connected) {
       code: 'ERR_INVALID_ARG_TYPE',
       name: 'TypeError',
       message: 'The "buffer" argument must be of type string or an instance ' +
-      'of Buffer or Uint8Array. Received type number (23)'
+      'of Buffer, TypedArray, or DataView. Received type number (23)'
     }
   );
 
@@ -101,7 +101,8 @@ function checkArgs(connected) {
       code: 'ERR_INVALID_ARG_TYPE',
       name: 'TypeError',
       message: 'The "buffer list arguments" argument must be of type string ' +
-      'or an instance of Buffer. Received an instance of Array'
+      'or an instance of Buffer, TypedArray, or DataView. ' +
+      'Received an instance of Array'
     }
   );
 }

diff --git a/test/parallel/test-dgram-send-default-host.js b/test/parallel/test-dgram-send-default-host.js
@@ -12,6 +12,11 @@ const toSend = [Buffer.alloc(256, 'x'),
                 'hello'];
 
 const received = [];
+let totalBytesSent = 0;
+let totalBytesReceived = 0;
+const arrayBufferViewsCount = common.getArrayBufferViews(
+  Buffer.from('')
+).length;
 
 client.on('listening', common.mustCall(() => {
   const port = client.address().port;
@@ -21,24 +26,47 @@ client.on('listening', common.mustCall(() => {
   client.send([toSend[2]], port);
   client.send(toSend[3], 0, toSend[3].length, port);
 
-  client.send(new Uint8Array(toSend[0]), 0, toSend[0].length, port);
-  client.send(new Uint8Array(toSend[1]), port);
-  client.send([new Uint8Array(toSend[2])], port);
-  client.send(new Uint8Array(Buffer.from(toSend[3])),
-              0, toSend[3].length, port);
+  totalBytesSent += toSend.map((buf) => buf.length)
+                      .reduce((a, b) => a + b, 0);
+
+  for (const msgBuf of common.getArrayBufferViews(toSend[0])) {
+    client.send(msgBuf, 0, msgBuf.byteLength, port);
+    totalBytesSent += msgBuf.byteLength;
+  }
+  for (const msgBuf of common.getArrayBufferViews(toSend[1])) {
+    client.send(msgBuf, port);
+    totalBytesSent += msgBuf.byteLength;
+  }
+  for (const msgBuf of common.getArrayBufferViews(toSend[2])) {
+    client.send([msgBuf], port);
+    totalBytesSent += msgBuf.byteLength;
+  }
 }));
 
 client.on('message', common.mustCall((buf, info) => {
   received.push(buf.toString());
+  totalBytesReceived += info.size;
 
-  if (received.length === toSend.length * 2) {
-    // The replies may arrive out of order -> sort them before checking.
-    received.sort();
-
-    const expected = toSend.concat(toSend).map(String).sort();
-    assert.deepStrictEqual(received, expected);
+  if (totalBytesReceived === totalBytesSent) {
     client.close();
   }
-}, toSend.length * 2));
+  // For every buffer in `toSend`, we send the raw Buffer,
+  // as well as every TypedArray in getArrayBufferViews()
+}, toSend.length + (toSend.length - 1) * arrayBufferViewsCount));
+
+client.on('close', common.mustCall((buf, info) => {
+  // The replies may arrive out of order -> sort them before checking.
+  received.sort();
+
+  const repeated = [...toSend];
+  for (let i = 0; i < arrayBufferViewsCount; i++) {
+    repeated.push(...toSend.slice(0, 3));
+  }
+
+  assert.strictEqual(totalBytesSent, totalBytesReceived);
+
+  const expected = repeated.map(String).sort();
+  assert.deepStrictEqual(received, expected);
+}));
 
 client.bind(0);
diff --git a/test/parallel/test-dgram-udp6-send-default-host.js b/test/parallel/test-dgram-udp6-send-default-host.js
@@ -15,26 +15,62 @@ const toSend = [Buffer.alloc(256, 'x'),
                 'hello'];
 
 const received = [];
+let totalBytesSent = 0;
+let totalBytesReceived = 0;
+const arrayBufferViewLength = common.getArrayBufferViews(
+  Buffer.from('')
+).length;
 
 client.on('listening', common.mustCall(() => {
   const port = client.address().port;
+
   client.send(toSend[0], 0, toSend[0].length, port);
   client.send(toSend[1], port);
   client.send([toSend[2]], port);
   client.send(toSend[3], 0, toSend[3].length, port);
+
+  totalBytesSent += toSend.map((buf) => buf.length)
+                      .reduce((a, b) => a + b, 0);
+
+  for (const msgBuf of common.getArrayBufferViews(toSend[0])) {
+    client.send(msgBuf, 0, msgBuf.byteLength, port);
+    totalBytesSent += msgBuf.byteLength;
+  }
+  for (const msgBuf of common.getArrayBufferViews(toSend[1])) {
+    client.send(msgBuf, port);
+    totalBytesSent += msgBuf.byteLength;
+  }
+  for (const msgBuf of common.getArrayBufferViews(toSend[2])) {
+    client.send([msgBuf], port);
+    totalBytesSent += msgBuf.byteLength;
+  }
 }));
 
 client.on('message', common.mustCall((buf, info) => {
   received.push(buf.toString());
+  totalBytesReceived += info.size;
 
-  if (received.length === toSend.length) {
-    // The replies may arrive out of order -> sort them before checking.
-    received.sort();
-
-    const expected = toSend.map(String).sort();
-    assert.deepStrictEqual(received, expected);
+  if (totalBytesReceived === totalBytesSent) {
     client.close();
   }
-}, toSend.length));
+  // For every buffer in `toSend`, we send the raw Buffer,
+  // as well as every TypedArray in getArrayBufferViews()
+}, toSend.length + (toSend.length - 1) * arrayBufferViewLength));
+
+client.on('close', common.mustCall((buf, info) => {
+  // The replies may arrive out of order -> sort them before checking.
+  received.sort();
+
+  const repeated = [...toSend];
+  for (let i = 0; i < arrayBufferViewLength; i++) {
+    // We get arrayBufferViews only for toSend[0..2].
+    repeated.push(...toSend.slice(0, 3));
+  }
+
+  assert.strictEqual(totalBytesSent, totalBytesReceived);
+
+  const expected = repeated.map(String).sort();
+  assert.deepStrictEqual(received, expected);
+}));
 
 client.bind(0);