src: throw DataCloneError on transfering untransferable objects

The HTML StructuredSerializeWithTransfer algorithm defines that when
an untransferable object is in the transfer list, a DataCloneError is
thrown.
An array buffer that is already transferred is also considered as
untransferable.

doc/api/worker_threads.md

@@ -128,10 +128,15 @@ if (isMainThread) {
 added:
   - v14.5.0
   - v12.19.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47604
+    description: An error is thrown when the untransferable object is in the
+                 transfer list.
 -->
 
 Mark an object as not transferable. If `object` occurs in the transfer list of
-a [`port.postMessage()`][] call, it is ignored.
+a [`port.postMessage()`][] call, an error is thrown.
 
 In particular, this makes sense for objects that can be cloned, rather than
 transferred, and which are used by other objects on the sending side.
@@ -568,6 +573,11 @@ are part of the channel.
 <!-- YAML
 added: v10.5.0
 changes:
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47604
+    description: An error is thrown when an untransferable object is in the
+                 transfer list.
   - version:
       - v15.14.0
       - v14.18.0

src/env_properties.h

@@ -66,7 +66,7 @@
   V(change_string, "change")                                                   \
   V(channel_string, "channel")                                                 \
   V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite")           \
-  V(clone_unsupported_type_str, "Cannot transfer object of unsupported type.") \
+  V(clone_unsupported_type_str, "Cannot clone object of unsupported type.")    \
   V(code_string, "code")                                                       \
   V(commonjs_string, "commonjs")                                               \
   V(config_string, "config")                                                   \
@@ -301,6 +301,8 @@
   V(time_to_first_header_string, "timeToFirstHeader")                          \
   V(tls_ticket_string, "tlsTicket")                                            \
   V(transfer_string, "transfer")                                               \
+  V(transfer_unsupported_type_str,                                             \
+    "Cannot transfer object of unsupported type.")                             \
   V(ttl_string, "ttl")                                                         \
   V(type_string, "type")                                                       \
   V(uid_string, "uid")                                                         \

src/node_messaging.cc

@@ -459,11 +459,14 @@ Maybe<bool> Message::Serialize(Environment* env,
               .To(&untransferable)) {
         return Nothing<bool>();
       }
-      if (untransferable) continue;
+      if (untransferable) {
+        ThrowDataCloneException(context, env->transfer_unsupported_type_str());
+        return Nothing<bool>();
+      }
     }
 
     // Currently, we support ArrayBuffers and BaseObjects for which
-    // GetTransferMode() does not return kUntransferable.
+    // GetTransferMode() returns kTransferable.
     if (entry->IsArrayBuffer()) {
       Local<ArrayBuffer> ab = entry.As<ArrayBuffer>();
       // If we cannot render the ArrayBuffer unusable in this Isolate,
@@ -474,7 +477,10 @@ Maybe<bool> Message::Serialize(Environment* env,
       // raw data *and* an Isolate with a non-default ArrayBuffer allocator
       // is always going to outlive any Workers it creates, and so will its
       // allocator along with it.
-      if (!ab->IsDetachable()) continue;
+      if (!ab->IsDetachable() || ab->WasDetached()) {
+        ThrowDataCloneException(context, env->transfer_unsupported_type_str());
+        return Nothing<bool>();
+      }
       if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
           array_buffers.end()) {
         ThrowDataCloneException(
@@ -524,8 +530,8 @@ Maybe<bool> Message::Serialize(Environment* env,
                 entry.As<Object>()->GetConstructorName()));
         return Nothing<bool>();
       }
-      if (host_object && host_object->GetTransferMode() !=
-              BaseObject::TransferMode::kUntransferable) {
+      if (host_object && host_object->GetTransferMode() ==
+                             BaseObject::TransferMode::kTransferable) {
         delegate.AddHostObject(host_object);
         continue;
       }

test/addons/worker-buffer-callback/test.js

@@ -9,7 +9,10 @@ const { buffer } = require(`./build/${common.buildType}/binding`);
 
 const { port1 } = new MessageChannel();
 const origByteLength = buffer.byteLength;
-port1.postMessage(buffer, [buffer.buffer]);
+assert.throws(() => port1.postMessage(buffer, [buffer.buffer]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 assert.strictEqual(buffer.byteLength, origByteLength);
 assert.notStrictEqual(buffer.byteLength, 0);

test/node-api/test_worker_buffer_callback/test.js

@@ -9,7 +9,10 @@ const { buffer } = require(`./build/${common.buildType}/binding`);
 
 const { port1 } = new MessageChannel();
 const origByteLength = buffer.byteLength;
-port1.postMessage(buffer, [buffer]);
+assert.throws(() => port1.postMessage(buffer, [buffer]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 assert.strictEqual(buffer.byteLength, origByteLength);
 assert.notStrictEqual(buffer.byteLength, 0);

test/parallel/test-buffer-pool-untransferable.js

@@ -13,7 +13,10 @@ assert.strictEqual(a.buffer, b.buffer);
 const length = a.length;
 
 const { port1 } = new MessageChannel();
-port1.postMessage(a, [ a.buffer ]);
+assert.throws(() => port1.postMessage(a, [ a.buffer ]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 // Verify that the pool ArrayBuffer has not actually been transferred:
 assert.strictEqual(a.buffer, b.buffer);

test/parallel/test-worker-message-port-arraybuffer.js

@@ -12,6 +12,13 @@ const { MessageChannel } = require('worker_threads');
   typedArray[0] = 0x12345678;
 
   port1.postMessage(typedArray, [ arrayBuffer ]);
+  assert.strictEqual(arrayBuffer.byteLength, 0);
+  // Transferring again should throw a DataCloneError.
+  assert.throws(() => port1.postMessage(typedArray, [ arrayBuffer ]), {
+    code: 25,
+    name: 'DataCloneError',
+  });
+
   port2.on('message', common.mustCall((received) => {
     assert.strictEqual(received[0], 0x12345678);
     port2.close(common.mustCall());

test/parallel/test-worker-message-port-transfer-native.js

@@ -31,7 +31,7 @@ const { internalBinding } = require('internal/test/binding');
     port1.postMessage(nativeObject);
   }, {
     name: 'DataCloneError',
-    message: /Cannot transfer object of unsupported type\.$/
+    message: /Cannot clone object of unsupported type\.$/
   });
   port1.close();
 }

test/parallel/test-worker-message-transfer-port-mark-as-untransferable.js

@@ -9,11 +9,13 @@ const { MessageChannel, markAsUntransferable } = require('worker_threads');
   markAsUntransferable(ab);
   assert.strictEqual(ab.byteLength, 8);
 
-  const { port1, port2 } = new MessageChannel();
-  port1.postMessage(ab, [ ab ]);
+  const { port1 } = new MessageChannel();
+  assert.throws(() => port1.postMessage(ab, [ ab ]), {
+    code: 25,
+    name: 'DataCloneError',
+  });
 
   assert.strictEqual(ab.byteLength, 8);  // The AB is not detached.
-  port2.once('message', common.mustCall());
 }
 
 {
@@ -24,7 +26,10 @@ const { MessageChannel, markAsUntransferable } = require('worker_threads');
 
   assert.throws(() => {
     channel1.port1.postMessage(channel2.port1, [ channel2.port1 ]);
-  }, /was found in message but not listed in transferList/);
+  }, {
+    code: 25,
+    name: 'DataCloneError',
+  });
 
   channel2.port1.postMessage('still works, not closed/transferred');
   channel2.port2.once('message', common.mustCall());