src: throw DOMException on cloning non-serializable objects

Instead of TypeError, throwing DOMException in accordance to the HTML structured serialize algorithms. PR-URL: #47839 Fixes: #40841 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
nodejs · Oct 8, 2023 · d920b7c · d920b7c
1 parent e43bf4c
commit d920b7c
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 38 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -2084,12 +2084,6 @@ urlSearchParams.has.call(buf, 'foo');
 // Throws a TypeError with code 'ERR_INVALID_THIS'
 ```
 
-<a id="ERR_INVALID_TRANSFER_OBJECT"></a>
-
-### `ERR_INVALID_TRANSFER_OBJECT`
-
-An invalid transfer object was passed to `postMessage()`.
-
 <a id="ERR_INVALID_TUPLE"></a>
 
 ### `ERR_INVALID_TUPLE`
@@ -2287,23 +2281,6 @@ The V8 platform used by this instance of Node.js does not support creating
 Workers. This is caused by lack of embedder support for Workers. In particular,
 this error will not occur with standard builds of Node.js.
 
-<a id="ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST"></a>
-
-### `ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`
-
-<!-- YAML
-added: v15.0.0
--->
-
-An object that needs to be explicitly listed in the `transferList` argument
-is in the object passed to a [`postMessage()`][] call, but is not provided
-in the `transferList` for that call. Usually, this is a `MessagePort`.
-
-In Node.js versions prior to v15.0.0, the error code being used here was
-[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`][]. However, the set of
-transferable object types has been expanded to cover more types than
-`MessagePort`.
-
 <a id="ERR_MODULE_NOT_FOUND"></a>
 
 ### `ERR_MODULE_NOT_FOUND`
@@ -3300,6 +3277,20 @@ removed: v15.0.0
 
 An invalid or unknown file encoding was passed.
 
+<a id="ERR_INVALID_TRANSFER_OBJECT"></a>
+
+### `ERR_INVALID_TRANSFER_OBJECT`
+
+<!-- YAML
+removed: REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47839
+    description: A `DOMException` is thrown instead.
+-->
+
+An invalid transfer object was passed to `postMessage()`.
+
 <a id="ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST"></a>
 
 ### `ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`
@@ -3312,6 +3303,28 @@ This error code was replaced by [`ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`][]
 in Node.js v15.0.0, because it is no longer accurate as other types of
 transferable objects also exist now.
 
+<a id="ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST"></a>
+
+### `ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST`
+
+<!-- YAML
+added: v15.0.0
+removed: REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47839
+    description: A `DOMException` is thrown instead.
+-->
+
+An object that needs to be explicitly listed in the `transferList` argument
+is in the object passed to a [`postMessage()`][] call, but is not provided
+in the `transferList` for that call. Usually, this is a `MessagePort`.
+
+In Node.js versions prior to v15.0.0, the error code being used here was
+[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`][]. However, the set of
+transferable object types has been expanded to cover more types than
+`MessagePort`.
+
 <a id="ERR_NAPI_CONS_PROTOTYPE_OBJECT"></a>
 
 ### `ERR_NAPI_CONS_PROTOTYPE_OBJECT`

diff --git a/src/env_properties.h b/src/env_properties.h
@@ -74,6 +74,10 @@
   V(channel_string, "channel")                                                 \
   V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite")           \
   V(clone_unsupported_type_str, "Cannot clone object of unsupported type.")    \
+  V(clone_transfer_needed_str,                                                 \
+    "Object that needs transfer was found in message but not listed in "       \
+    "transferList")                                                            \
+  V(clone_untransferable_str, "Found invalid value in transferList.")          \
   V(code_string, "code")                                                       \
   V(commonjs_string, "commonjs")                                               \
   V(config_string, "config")                                                   \

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -75,13 +75,11 @@ void AppendExceptionLine(Environment* env,
   V(ERR_INVALID_MODULE, Error)                                                 \
   V(ERR_INVALID_STATE, Error)                                                  \
   V(ERR_INVALID_THIS, TypeError)                                               \
-  V(ERR_INVALID_TRANSFER_OBJECT, TypeError)                                    \
   V(ERR_INVALID_URL, TypeError)                                                \
   V(ERR_INVALID_URL_SCHEME, TypeError)                                         \
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                       \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE, Error)                             \
   V(ERR_MISSING_ARGS, TypeError)                                               \
-  V(ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST, TypeError)                      \
   V(ERR_MISSING_PASSPHRASE, TypeError)                                         \
   V(ERR_MISSING_PLATFORM_FOR_WORKER, Error)                                    \
   V(ERR_MODULE_NOT_FOUND, Error)                                               \
@@ -168,16 +166,12 @@ ERRORS_WITH_CODE(V)
   V(ERR_INVALID_MODULE, "No such module")                                      \
   V(ERR_INVALID_STATE, "Invalid state")                                        \
   V(ERR_INVALID_THIS, "Value of \"this\" is the wrong type")                   \
-  V(ERR_INVALID_TRANSFER_OBJECT, "Found invalid object in transferList")       \
   V(ERR_INVALID_URL_SCHEME, "The URL must be of scheme file:")                 \
   V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory")                 \
   V(ERR_OSSL_EVP_INVALID_DIGEST, "Invalid digest used")                        \
   V(ERR_MESSAGE_TARGET_CONTEXT_UNAVAILABLE,                                    \
     "A message object could not be deserialized successfully in the target "   \
     "vm.Context")                                                              \
-  V(ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST,                                 \
-    "Object that needs transfer was found in message but not listed "          \
-    "in transferList")                                                         \
   V(ERR_MISSING_PLATFORM_FOR_WORKER,                                           \
     "The V8 platform used by this instance of Node does not support "          \
     "creating Workers")                                                        \

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -431,7 +431,7 @@ class SerializerDelegate : public ValueSerializer::Delegate {
           return Just(true);
         }
       }
-      THROW_ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST(env_);
+      ThrowDataCloneError(env_->clone_transfer_needed_str());
       return Nothing<bool>();
     }
 
@@ -475,7 +475,7 @@ Maybe<bool> Message::Serialize(Environment* env,
     Local<Value> entry_val = transfer_list_v[i];
     if (!entry_val->IsObject()) {
       // Only object can be transferred.
-      THROW_ERR_INVALID_TRANSFER_OBJECT(env);
+      ThrowDataCloneException(context, env->clone_untransferable_str());
       return Nothing<bool>();
     }
     Local<Object> entry = entry_val.As<Object>();
@@ -533,7 +533,7 @@ Maybe<bool> Message::Serialize(Environment* env,
       host_object = BaseObjectPtr<BaseObject>{Unwrap<BaseObject>(entry)};
     } else {
       if (!JSTransferable::IsJSTransferable(env, context, entry)) {
-        THROW_ERR_INVALID_TRANSFER_OBJECT(env);
+        ThrowDataCloneException(context, env->clone_untransferable_str());
         return Nothing<bool>();
       }
       JSTransferable* js_transferable = JSTransferable::Wrap(env, entry);

diff --git a/test/parallel/test-whatwg-webstreams-transfer.js b/test/parallel/test-whatwg-webstreams-transfer.js
@@ -105,7 +105,9 @@ const theData = 'hello';
   });
 
   assert.throws(() => port2.postMessage(readable), {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
   });
 
   port2.postMessage(readable, [readable]);
@@ -155,7 +157,9 @@ const theData = 'hello';
   });
 
   assert.throws(() => port2.postMessage(readable), {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
   });
 
   port2.postMessage(readable, [readable]);
@@ -206,7 +210,9 @@ const theData = 'hello';
   });
 
   assert.throws(() => port2.postMessage(writable), {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
   });
 
   port2.postMessage(writable, [writable]);
@@ -292,7 +298,9 @@ const theData = 'hello';
   });
 
   assert.throws(() => port2.postMessage(transform), {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
   });
 
   port2.postMessage(transform, [transform]);

diff --git a/test/parallel/test-worker-message-port-transfer-filehandle.js b/test/parallel/test-worker-message-port-transfer-filehandle.js
@@ -14,7 +14,9 @@ const { once } = require('events');
   assert.throws(() => {
     port1.postMessage(fh);
   }, {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST'
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
   });
 
   // Check that transferring FileHandle instances works.

diff --git a/test/parallel/test-worker-workerdata-messageport.js b/test/parallel/test-worker-workerdata-messageport.js
@@ -54,7 +54,9 @@ const meowScript = () => 'meow';
     workerData,
     transferList: []
   }), {
-    code: 'ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST',
+    constructor: DOMException,
+    name: 'DataCloneError',
+    code: 25,
     message: 'Object that needs transfer was found in message but not ' +
              'listed in transferList'
   });