worker: add typechecking for postMessage transfer list

addaleax · addaleax · commit 269c1d69c957 · 2019-06-10T15:41:09.000+02:00
If the transfer list argument is present, it should be an array. This commit adds typechecking to that effect. This aligns behaviour with browsers. PR-URL: #28033 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -714,6 +714,15 @@ void MessagePort::PostMessage(const FunctionCallbackInfo<Value>& args) {
     return THROW_ERR_MISSING_ARGS(env, "Not enough arguments to "
                                        "MessagePort.postMessage");
   }
+  if (!args[1]->IsNullOrUndefined() && !args[1]->IsObject()) {
+    // Browsers ignore null or undefined, and otherwise accept an array or an
+    // options object.
+    // TODO(addaleax): Add support for an options object and generic sequence
+    // support.
+    // Refs: https://github.com/nodejs/node/pull/28033#discussion_r289964991
+    return THROW_ERR_INVALID_ARG_TYPE(env,
+        "Optional transferList argument must be an array");
+  }
 
   MessagePort* port = Unwrap<MessagePort>(args.This());
   // Even if the backing MessagePort object has already been deleted, we still
diff --git a/test/parallel/test-worker-message-port.js b/test/parallel/test-worker-message-port.js
@@ -70,6 +70,27 @@ const { MessageChannel, MessagePort } = require('worker_threads');
   });
 }
 
+{
+  const { port1, port2 } = new MessageChannel();
+  port2.on('message', common.mustCall(4));
+  port1.postMessage(1, null);
+  port1.postMessage(2, undefined);
+  port1.postMessage(3, []);
+  port1.postMessage(4, {});
+
+  const err = {
+    constructor: TypeError,
+    code: 'ERR_INVALID_ARG_TYPE',
+    message: 'Optional transferList argument must be an array'
+  };
+
+  assert.throws(() => port1.postMessage(5, 0), err);
+  assert.throws(() => port1.postMessage(5, false), err);
+  assert.throws(() => port1.postMessage(5, 'X'), err);
+  assert.throws(() => port1.postMessage(5, Symbol('X')), err);
+  port1.close();
+}
+
 {
   assert.deepStrictEqual(
     Object.getOwnPropertyNames(MessagePort.prototype).sort(),
