w3c / webextensions

Charter and administrivia for the WebExtensions Community Group (WECG)
Other
605 stars 56 forks source link

Proposal: dom.createPort() #679

Open rdcronin opened 3 months ago

rdcronin commented 3 months ago

This adds a proposal for a new dom.createPort() method, which will allow communication between different JavaScript "worlds". It leverages the proposed dom.execute() method (#678).

tophf commented 2 months ago

A message port can be passed to another world [...] by leveraging the new dom.execute() API

But this won't support userscripts registered via userScripts.register() API in the target world (e.g. "main"), so we will have register them in the primary userscript world and then run them via dom.execute(), which is pretty wasteful due to the additional serialization/deserialization and various other JS checks.

Possible solutions:

tophf commented 2 months ago

It's also imperative to have the ability to pass DOM nodes (live and detached, from either a shadow or the main tree) and a window object. The recipient receives its own world's version of the object, of course. Currently ***monkey extensions are doing it by dispatching two messages: CustomEvent with the data and a MouseEvent with relatedTarget set to the node/window (the actual supported type is EventTarget).

❌ It means that a MessagePort cannot be used.

tophf commented 2 months ago

Today, to communicate between JS worlds, extensions can use custom events [...] This is fragile and hacky, and can lean to leaking more data to the embedding page.

To clarify, it's not that using CustomEvent per se is fragile and hacky, but using any built-in global or prototype is unsafe (e.g. MessagePort) due to a bug in all browsers that allows the embedder to poison the prototypes of a same-origin iframe before document_start content script runs inside. There's also the problem of passing the random event id between worlds to avoid eavesdropping by other extensions or the web page scripts, although it's trivially solved by the upcoming dom.execute() in #678, which only leaves the problem of poisoning.

There is a way to use CustomEvent mechanism safely for a communication channel - just don't depend on current prototypes, i.e. addEventListener, dispatchEvent, CustomEvent constructor, CustomEvent.prototype.detail getter all should be used from a safe source, not from the current JS environment.

Current workaround for extensions is to extract these safe original functions from a temporary iframe that they create in their secure world using a function extractor that runs synchronously in the target world ("main").

Browsers should be able to do it without creating an iframe:

In ether case the object produced by dom.createPort() will use these functions internally.


One inconvenience of CustomEvent is the need to have a target such as window, which means that the event listener can be seen in devtools inspector's event listeners panel. While not a safety concern, but an ideal solution would be to use the CustomEvent mechanism directly without attaching a visible event or mark this event listener as invisible for devtools and debugger protocol.

tophf commented 2 months ago

Compiling the reasons why MessagePort cannot be used as the internal mechanism:

Rob--W commented 2 months ago

Note: "MessagePort" in this proposal refers to a new type. It is not the MessagePort from the web platform. I explicitly called out before that this concept already exists and that it cannot be used to serve the use case of extensions, at https://github.com/w3c/webextensions/pull/679#discussion_r1754969359

Regarding the prototypes: I'd like the proposed API to be safe against prototype tampering. This was called out at https://github.com/w3c/webextensions/pull/679#discussion_r1754990101 . There is also a special note on (un)safety of received values, at https://github.com/w3c/webextensions/pull/679#discussion_r1755098971

tophf commented 2 months ago

"MessagePort" in this proposal refers to a new type

Then it must be renamed, otherwise it'll be the "Local Storage" disaster all over again, when new developers confuse chrome.storage.local with localStorage en masse and mix up their usage patterns, limitations, availability, permissions, and inspectability in devtools.

It's unclear to me that it should contain "port" in its name.

What is clear to me is that it should be based on the existing robust synchronous mechanisms such as CustomEvent (for objects) and MouseEvent (for DOM nodes), which aren't "fragile or hacky" per se as I explained in my comments above in detail.

tophf commented 2 months ago

A simpler solution might be to incorporate it all in dom.execute.

Example 1, port parameter and result ```js const {result, error, port} = dom.execute({func, worldId, args, port: true}); port.onmessage = msg => console.log('from', worldId, msg); port.send(1); ``` `port: true` adds an implicit argument to `func`: ```js function (arg1, ..., argN, port) { port.onmessage = msg => { console.log(msg); if (msg === 1) port.send(2); }; } ``` OTOH, it may frustrate new developers because `port.send` cannot be used during the initial execution of the function.
Example 2, onmessage parameter ```js const {result, error, send} = dom.execute({ func, worldId, args, onmessage: (msg, respond) => { console.log('from', worldId, msg); respond('ok'); }, }); send(123); ``` `onmessage` adds an implicit argument to `func`: ```js function (arg1, ..., argN, port) { port.onmessage = msg => console.log(msg); port.send(1); } ```
Example 3, onmessage parameter + port result ```js const {result, error, port} = dom.execute({ func, worldId, args, onmessage: (msg, port) => { console.log('from', worldId, msg); port.send('ok'); }, }); port.send(123); port.onmessage = null; ``` `onmessage` adds an implicit argument to `func`: ```js function (arg1, ..., argN, port) { port.onmessage = (msg, port) => console.log(msg); port.send(1); } ```