diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-08-14 01:04:18 +0300 |
---|---|---|
committer | James M Snell <jasnell@gmail.com> | 2020-08-17 21:42:50 +0300 |
commit | 81df6684807072824ac51b4d14548bbf357c24b8 (patch) | |
tree | 042dd0c4688748cf5ae7773c58083ef3a60677b7 | |
parent | 5835367df4961bb2d71b0700b430b11f9ad32022 (diff) |
worker: do not crash when JSTransferable lists untransferable value
This can currently be triggered when posting a closing FileHandle.
Refs: https://github.com/nodejs/node/pull/34746#issuecomment-673675333
PR-URL: https://github.com/nodejs/node/pull/34766
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
-rw-r--r-- | src/node_messaging.cc | 12 | ||||
-rw-r--r-- | test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js | 38 |
2 files changed, 45 insertions, 5 deletions
diff --git a/src/node_messaging.cc b/src/node_messaging.cc index c6152935704..a072176523e 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -343,6 +343,12 @@ class SerializerDelegate : public ValueSerializer::Delegate { private: Maybe<bool> WriteHostObject(BaseObjectPtr<BaseObject> host_object) { + BaseObject::TransferMode mode = host_object->GetTransferMode(); + if (mode == BaseObject::TransferMode::kUntransferable) { + ThrowDataCloneError(env_->clone_unsupported_type_str()); + return Nothing<bool>(); + } + for (uint32_t i = 0; i < host_objects_.size(); i++) { if (host_objects_[i] == host_object) { serializer->WriteUint32(i); @@ -350,11 +356,7 @@ class SerializerDelegate : public ValueSerializer::Delegate { } } - BaseObject::TransferMode mode = host_object->GetTransferMode(); - if (mode == BaseObject::TransferMode::kUntransferable) { - ThrowDataCloneError(env_->clone_unsupported_type_str()); - return Nothing<bool>(); - } else if (mode == BaseObject::TransferMode::kTransferable) { + if (mode == BaseObject::TransferMode::kTransferable) { THROW_ERR_MISSING_TRANSFERABLE_IN_TRANSFER_LIST(env_); return Nothing<bool>(); } diff --git a/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js b/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js new file mode 100644 index 00000000000..621c42be514 --- /dev/null +++ b/test/parallel/test-worker-message-port-jstransferable-nested-untransferable.js @@ -0,0 +1,38 @@ +// Flags: --expose-internals --no-warnings +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { + JSTransferable, kTransfer, kTransferList +} = require('internal/worker/js_transferable'); +const { MessageChannel } = require('worker_threads'); + +// Transferring a JSTransferable that refers to another, untransferable, value +// in its transfer list should not crash hard. + +class OuterTransferable extends JSTransferable { + constructor() { + super(); + // Create a detached MessagePort at this.inner + const c = new MessageChannel(); + this.inner = c.port1; + c.port2.postMessage(this.inner, [ this.inner ]); + } + + [kTransferList] = common.mustCall(() => { + return [ this.inner ]; + }); + + [kTransfer] = common.mustCall(() => { + return { + data: { inner: this.inner }, + deserializeInfo: 'does-not:matter' + }; + }); +} + +const { port1 } = new MessageChannel(); +const ot = new OuterTransferable(); +assert.throws(() => { + port1.postMessage(ot, [ot]); +}, { name: 'DataCloneError' }); |