fixup! src: throw DataCloneError on transfering untransferable objects

nodejs · nodejs-github-bot · May 5, 2023 · Apr 18, 2023 · Apr 27, 2023 · May 3, 2023
commit 78e022a9f93f121f1dbeb07a3872ff1aa550ea4a
diff --git a/doc/api/worker_threads.md b/doc/api/worker_threads.md
@@ -128,11 +128,6 @@ 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.
 -->
 
 * `object` {any} Any arbitrary JavaScript value.
@@ -167,7 +162,8 @@ try {
 
 // The following line prints the contents of typedArray1 -- it still owns
 // its memory and has not been transferred. Without
-// `markAsUntransferable()`, this would print an empty Uint8Array.
+// `markAsUntransferable()`, this would print an empty Uint8Array and the
+// postMessage call would have succeeded.
 // typedArray2 is intact as well.
 console.log(typedArray1);
 console.log(typedArray2);
@@ -181,7 +177,7 @@ There is no equivalent to this API in browsers.
 added: REPLACEME
 -->
 
-* `object` {any} Any arbitrary JavaScript value.
+* `object` {any} Any JavaScript value.
 * Returns: {boolean}
 
 Check is an object is marked as not transferable with

diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -6,6 +6,7 @@ const {
   Float64Array,
   MathFloor,
   Number,
+  ObjectPrototypeHasOwnProperty,
   Uint8Array,
 } = primordials;
 
@@ -1056,8 +1057,12 @@ function markAsUntransferable(obj) {
 // This simply checks if the object is marked as untransferable and doesn't
 // check the ability to be transferred.
 function isMarkedAsUntransferable(obj) {
-  if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
+  if (obj == null)
+    return false;
+  // v8::Object::HasPrivate checks own properties only.
+  if (!ObjectPrototypeHasOwnProperty(obj, untransferable_object_private_symbol)) {
     return false;
+  }
   return obj[untransferable_object_private_symbol] === true;
 }
 

diff --git a/test/parallel/test-worker-message-transfer-port-mark-as-untransferable.js b/test/parallel/test-worker-message-transfer-port-mark-as-untransferable.js
@@ -1,12 +1,13 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const { MessageChannel, markAsUntransferable } = require('worker_threads');
+const { MessageChannel, markAsUntransferable, isMarkedAsUntransferable } = require('worker_threads');
 
 {
   const ab = new ArrayBuffer(8);
 
   markAsUntransferable(ab);
+  assert.ok(isMarkedAsUntransferable(ab));
   assert.strictEqual(ab.byteLength, 8);
 
   const { port1 } = new MessageChannel();
@@ -23,6 +24,7 @@ const { MessageChannel, markAsUntransferable } = require('worker_threads');
   const channel2 = new MessageChannel();
 
   markAsUntransferable(channel2.port1);
+  assert.ok(isMarkedAsUntransferable(channel2.port1));
 
   assert.throws(() => {
     channel1.port1.postMessage(channel2.port1, [ channel2.port1 ]);
@@ -36,7 +38,22 @@ const { MessageChannel, markAsUntransferable } = require('worker_threads');
 }
 
 {
-  for (const value of [0, null, false, true, undefined, [], {}]) {
+  for (const value of [0, null, false, true, undefined]) {
     markAsUntransferable(value);  // Has no visible effect.
+    assert.ok(!isMarkedAsUntransferable(value));
   }
+  for (const value of [[], {}]) {
+    markAsUntransferable(value);
+    assert.ok(isMarkedAsUntransferable(value));
+  }
+}
+
+{
+  // Verifies that the mark is not inherited.
+  class Foo {}
+  markAsUntransferable(Foo.prototype);
+  assert.ok(isMarkedAsUntransferable(Foo.prototype));
+
+  const foo = new Foo();
+  assert.ok(!isMarkedAsUntransferable(foo));
 }