Skip to content

Commit

Permalink
worker: do not crash when JSTransferable lists untransferable value
Browse files Browse the repository at this point in the history
This can currently be triggered when posting a closing FileHandle.

Refs: #34746 (comment)

PR-URL: #34766
Backport-PR-URL: #34814
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax committed Sep 27, 2020
1 parent f9420c7 commit 304fa0c
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -367,18 +367,20 @@ 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);
return Just(true);
}
}

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) {
// TODO(addaleax): This message code is too specific. Fix that in a
// semver-major follow-up.
THROW_ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST(env_);
Expand Down
Original file line number Diff line number Diff line change
@@ -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' });

0 comments on commit 304fa0c

Please sign in to comment.