From d7858f2f91806e3f212cbd97250c38bb009a9b23 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Wed, 19 Jun 2024 10:10:57 -0400 Subject: [PATCH] fix: MessagePort closing unexpectedly with non-cloneable objects (#42581) * fix: MessagePort closing unexpectedly with non-cloneable objects Co-authored-by: Shelley Vohr * fix: handle serialization failure in parentPort Co-authored-by: Shelley Vohr --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- shell/browser/api/message_port.cc | 7 +++++-- shell/services/node/parent_port.cc | 8 +++++++- spec/api-utility-process-spec.ts | 11 +++++++++++ spec/fixtures/api/utility-process/non-cloneable.js | 11 +++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/api/utility-process/non-cloneable.js diff --git a/shell/browser/api/message_port.cc b/shell/browser/api/message_port.cc index 6afeaa0c6954..a81360b0f1da 100644 --- a/shell/browser/api/message_port.cc +++ b/shell/browser/api/message_port.cc @@ -76,8 +76,11 @@ void MessagePort::PostMessage(gin::Arguments* args) { return; } - electron::SerializeV8Value(args->isolate(), message_value, - &transferable_message); + if (!electron::SerializeV8Value(args->isolate(), message_value, + &transferable_message)) { + // SerializeV8Value sets an exception. + return; + } v8::Local transferables; std::vector> wrapped_ports; diff --git a/shell/services/node/parent_port.cc b/shell/services/node/parent_port.cc index 89cf18dcb59d..13d9433f30dd 100644 --- a/shell/services/node/parent_port.cc +++ b/shell/services/node/parent_port.cc @@ -44,7 +44,13 @@ void ParentPort::PostMessage(v8::Local message_value) { if (!connector_closed_ && connector_ && connector_->is_valid()) { v8::Isolate* isolate = JavascriptEnvironment::GetIsolate(); blink::TransferableMessage transferable_message; - electron::SerializeV8Value(isolate, message_value, &transferable_message); + + if (!electron::SerializeV8Value(isolate, message_value, + &transferable_message)) { + // SerializeV8Value sets an exception. + return; + } + mojo::Message mojo_message = blink::mojom::TransferableMessage::WrapAsMessage( std::move(transferable_message)); diff --git a/spec/api-utility-process-spec.ts b/spec/api-utility-process-spec.ts index ccc551bbac95..3694e2c1adf9 100644 --- a/spec/api-utility-process-spec.ts +++ b/spec/api-utility-process-spec.ts @@ -297,6 +297,17 @@ describe('utilityProcess module', () => { expect(child.kill()).to.be.true(); await exit; }); + + it('handles the parent port trying to send an non-clonable object', async () => { + const child = utilityProcess.fork(path.join(fixturesPath, 'non-cloneable.js')); + await once(child, 'spawn'); + child.postMessage('non-cloneable'); + const [data] = await once(child, 'message'); + expect(data).to.equal('caught-non-cloneable'); + const exit = once(child, 'exit'); + expect(child.kill()).to.be.true(); + await exit; + }); }); describe('behavior', () => { diff --git a/spec/fixtures/api/utility-process/non-cloneable.js b/spec/fixtures/api/utility-process/non-cloneable.js new file mode 100644 index 000000000000..fd416ee2bec1 --- /dev/null +++ b/spec/fixtures/api/utility-process/non-cloneable.js @@ -0,0 +1,11 @@ +const nonClonableObject = () => {}; + +process.parentPort.on('message', () => { + try { + process.parentPort.postMessage(nonClonableObject); + } catch (error) { + if (/An object could not be cloned/.test(error.message)) { + process.parentPort.postMessage('caught-non-cloneable'); + } + } +});