Skip to content

Maintain backwards compatibility with wire format from 4.3.0 #629

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Maintain backwards compatibility with wire format from 4.3.0 #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3df8b5c
Maintain backwards compatibility with wire format from 4.3.0
Nemikolh Feb 26, 2023
191bb15
Support multiple format to allow communication with 4.3.0 or older
Nemikolh Mar 7, 2023
0d49cdc
Isolate 4.3.0 test from attack iframe one
Nemikolh Mar 7, 2023
1d8bac0
Revert the addition of BaseMessage, it's not really needed anymore
Nemikolh Mar 7, 2023
5c7458b
Refactor V430* to Legacy* and isV430 to isLegacy
Nemikolh Mar 8, 2023
8ba107a
Simplify markLegacyPort logic and only mark them when needed
Nemikolh Mar 9, 2023
fee83e6
Add traversal as it's more useful than anticipated
Nemikolh Mar 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions karma.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ module.exports = function (config) {
pattern: "dist/**/*.@(mjs|js)",
included: false,
},
{
pattern: "node_modules/comlink/dist/esm/**/*.@(mjs|js)",
type: "module",
},
{
pattern: "tests/*.test.js",
type: "module",
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@rollup/plugin-typescript": "11.0.0",
"chai": "^4.3.7",
"conditional-type-checks": "1.0.6",
"comlink": "4.3.0",
"husky": "8.0.3",
"karma": "6.4.1",
"karma-chai": "0.1.0",
Expand Down
140 changes: 115 additions & 25 deletions src/comlink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import {
Endpoint,
EventSource,
Message,
LegacyMessageType,
MessageType,
PostMessageWithOrigin,
WireValue,
LegacyWireValueType,
WireValueType,
MessageTypeMap,
} from "./protocol";
export type { Endpoint };

Expand Down Expand Up @@ -305,43 +308,53 @@ export function expose(
}
const { id, type, path } = {
path: [] as string[],
...(ev.data as Message),
...(ev.data as Message | WireValue),
};

const isLegacy = typeof type === "number";
const argumentList = (ev.data.argumentList || []).map(fromWireValue);

let returnValue;
try {
const parent = path.slice(0, -1).reduce((obj, prop) => obj[prop], obj);
const rawValue = path.reduce((obj, prop) => obj[prop], obj);
switch (type) {
case LegacyMessageType.GET:
case MessageType.GET:
{
returnValue = rawValue;
}
break;
case LegacyMessageType.SET:
case MessageType.SET:
{
parent[path.slice(-1)[0]] = fromWireValue(ev.data.value);
const value = fromWireValue(ev.data.value);
parent[path.slice(-1)[0]] = value;
returnValue = true;
}
break;
case LegacyMessageType.APPLY:
case MessageType.APPLY:
{
returnValue = rawValue.apply(parent, argumentList);
}
break;
case LegacyMessageType.CONSTRUCT:
case MessageType.CONSTRUCT:
{
const value = new rawValue(...argumentList);
returnValue = proxy(value);
}
break;
case LegacyMessageType.ENDPOINT:
case MessageType.ENDPOINT:
{
const { port1, port2 } = new MessageChannel();
expose(obj, port2);
returnValue = transfer(port1, [port1]);
}
break;
case LegacyMessageType.RELEASE:
case MessageType.RELEASE:
{
returnValue = undefined;
Expand All @@ -358,7 +371,7 @@ export function expose(
return { value, [throwMarker]: 0 };
})
.then((returnValue) => {
const [wireValue, transferables] = toWireValue(returnValue);
const [wireValue, transferables] = toWireValue(returnValue, isLegacy);
ep.postMessage({ ...wireValue, id }, transferables);
if (type === MessageType.RELEASE) {
// detach and deactive after sending release response above.
Expand All @@ -371,10 +384,13 @@ export function expose(
})
.catch((error) => {
// Send Serialization Error To Caller
const [wireValue, transferables] = toWireValue({
value: new TypeError("Unserializable return value"),
[throwMarker]: 0,
});
const [wireValue, transferables] = toWireValue(
{
value: new TypeError("Unserializable return value"),
[throwMarker]: 0,
},
isLegacy
);
ep.postMessage({ ...wireValue, id }, transferables);
});
} as any);
Expand All @@ -387,11 +403,38 @@ function isMessagePort(endpoint: Endpoint): endpoint is MessagePort {
return endpoint.constructor.name === "MessagePort";
}

function isEndpoint(endpoint: object): endpoint is Endpoint {
return (
"postMessage" in endpoint &&
"addEventListener" in endpoint &&
"removeEventListener" in endpoint
);
}

function markLegacyPort(maybePort: unknown) {
if (isObject(maybePort)) {
if (isEndpoint(maybePort) && isMessagePort(maybePort)) {
legacyEndpoints.add(maybePort);
} else {
for (const prop in maybePort) {
markLegacyPort(maybePort[prop as keyof typeof maybePort]);
}
}
}
}

function closeEndPoint(endpoint: Endpoint) {
if (isMessagePort(endpoint)) endpoint.close();
}

export function wrap<T>(ep: Endpoint, target?: any): Remote<T> {
export function wrap<T>(
ep: Endpoint,
target?: any,
legacy: boolean = false
Copy link
Author

@Nemikolh Nemikolh Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is provided for cases where prior knowledge has not been obtained. For instance if instead of doing this:

a.foo(Comlink.transfer(port, [port])

The caller does:

a.foo(Comlink.transfer({ prop: port }, [port]))

In this case if inside foo we do:

foo({ prop: port }) {
  const remote = Comlink.wrap(port);
  // Oopsie this will break! It won't use the legacy format.
  remote.oopsie();
}

In this scenario because we don't traverse the object { prop: port } we don't mark port as legacy. We could traverse them but this might have a huge overhead and so I opted for not doing it.

Instead the implementer can do:

foo({ prop: port }) {
  const remote = Comlink.wrap(port, undefined, true);
  // Ok fine this works :)
  remote.oopsie();
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to add the traversal then only markLegacyPort needs to be updated

): Remote<T> {
if (legacy) {
legacyEndpoints.add(ep);
}
return createProxy<T>(ep, [], target) as any;
}

Expand All @@ -401,9 +444,9 @@ function throwIfProxyReleased(isReleased: boolean) {
}
}

function releaseEndpoint(ep: Endpoint) {
function releaseEndpoint(ep: Endpoint, legacy: boolean) {
return requestResponseMessage(ep, {
type: MessageType.RELEASE,
type: msgType(MessageType.RELEASE, legacy),
}).then(() => {
closeEndPoint(ep);
});
Expand All @@ -420,14 +463,16 @@ interface FinalizationRegistry<T> {
}
declare var FinalizationRegistry: FinalizationRegistry<Endpoint>;

const legacyEndpoints = new WeakSet<Endpoint>();
const proxyCounter = new WeakMap<Endpoint, number>();
const proxyFinalizers =
"FinalizationRegistry" in globalThis &&
new FinalizationRegistry((ep: Endpoint) => {
const newCount = (proxyCounter.get(ep) || 0) - 1;
proxyCounter.set(ep, newCount);
if (newCount === 0) {
releaseEndpoint(ep);
releaseEndpoint(ep, legacyEndpoints.has(ep));
legacyEndpoints.delete(ep);
}
});

Expand All @@ -451,13 +496,14 @@ function createProxy<T>(
target: object = function () {}
): Remote<T> {
let isProxyReleased = false;
const isLegacy = legacyEndpoints.has(ep);
const proxy = new Proxy(target, {
get(_target, prop) {
throwIfProxyReleased(isProxyReleased);
if (prop === releaseProxy) {
return () => {
unregisterProxy(proxy);
releaseEndpoint(ep);
releaseEndpoint(ep, isLegacy);
isProxyReleased = true;
};
}
Expand All @@ -466,7 +512,7 @@ function createProxy<T>(
return { then: () => proxy };
}
const r = requestResponseMessage(ep, {
type: MessageType.GET,
type: msgType(MessageType.GET, isLegacy),
path: path.map((p) => p.toString()),
}).then(fromWireValue);
return r.then.bind(r);
Expand All @@ -477,11 +523,11 @@ function createProxy<T>(
throwIfProxyReleased(isProxyReleased);
// FIXME: ES6 Proxy Handler `set` methods are supposed to return a
// boolean. To show good will, we return true asynchronously ¯\_(ツ)_/¯
const [value, transferables] = toWireValue(rawValue);
const [value, transferables] = toWireValue(rawValue, isLegacy);
return requestResponseMessage(
ep,
{
type: MessageType.SET,
type: msgType(MessageType.SET, isLegacy),
path: [...path, prop].map((p) => p.toString()),
value,
},
Expand All @@ -493,18 +539,21 @@ function createProxy<T>(
const last = path[path.length - 1];
if ((last as any) === createEndpoint) {
return requestResponseMessage(ep, {
type: MessageType.ENDPOINT,
type: msgType(MessageType.ENDPOINT, isLegacy),
}).then(fromWireValue);
}
// We just pretend that `bind()` didn’t happen.
if (last === "bind") {
return createProxy(ep, path.slice(0, -1));
}
const [argumentList, transferables] = processArguments(rawArgumentList);
const [argumentList, transferables] = processArguments(
rawArgumentList,
isLegacy
);
return requestResponseMessage(
ep,
{
type: MessageType.APPLY,
type: msgType(MessageType.APPLY, isLegacy),
path: path.map((p) => p.toString()),
argumentList,
},
Expand All @@ -513,11 +562,14 @@ function createProxy<T>(
},
construct(_target, rawArgumentList) {
throwIfProxyReleased(isProxyReleased);
const [argumentList, transferables] = processArguments(rawArgumentList);
const [argumentList, transferables] = processArguments(
rawArgumentList,
isLegacy
);
return requestResponseMessage(
ep,
{
type: MessageType.CONSTRUCT,
type: msgType(MessageType.CONSTRUCT, isLegacy),
path: path.map((p) => p.toString()),
argumentList,
},
Expand All @@ -533,8 +585,11 @@ function myFlat<T>(arr: (T | T[])[]): T[] {
return Array.prototype.concat.apply([], arr);
}

function processArguments(argumentList: any[]): [WireValue[], Transferable[]] {
const processed = argumentList.map(toWireValue);
function processArguments(
argumentList: any[],
isLegacy: boolean
): [WireValue[], Transferable[]] {
const processed = argumentList.map((arg) => toWireValue(arg, isLegacy));
return [processed.map((v) => v[0]), myFlat(processed.map((v) => v[1]))];
}

Expand All @@ -561,13 +616,16 @@ export function windowEndpoint(
};
}

function toWireValue(value: any): [WireValue, Transferable[]] {
function toWireValue(
value: any,
isLegacy: boolean
): [WireValue, Transferable[]] {
for (const [name, handler] of transferHandlers) {
if (handler.canHandle(value)) {
const [serializedValue, transferables] = handler.serialize(value);
return [
{
type: WireValueType.HANDLER,
type: isLegacy ? LegacyWireValueType.HANDLER : WireValueType.HANDLER,
name,
value: serializedValue,
},
Expand All @@ -577,7 +635,7 @@ function toWireValue(value: any): [WireValue, Transferable[]] {
}
return [
{
type: WireValueType.RAW,
type: isLegacy ? LegacyWireValueType.RAW : WireValueType.RAW,
value,
},
transferCache.get(value) || [],
Expand All @@ -586,8 +644,12 @@ function toWireValue(value: any): [WireValue, Transferable[]] {

function fromWireValue(value: WireValue): any {
switch (value.type) {
case LegacyWireValueType.HANDLER:
markLegacyPort(value.value);
case WireValueType.HANDLER:
return transferHandlers.get(value.name)!.deserialize(value.value);
case LegacyWireValueType.RAW:
markLegacyPort(value.value);
case WireValueType.RAW:
return value.value;
}
Expand All @@ -614,6 +676,34 @@ function requestResponseMessage(
});
}

function msgType<T extends MessageType>(
type: T,
isLegacy: boolean
): MessageTypeMap[T] | T {
if (!isLegacy) {
return type;
}

function mapMessageTypeToLegacyType(type: MessageType): LegacyMessageType {
switch (type) {
case MessageType.GET:
return LegacyMessageType.GET;
case MessageType.SET:
return LegacyMessageType.SET;
case MessageType.APPLY:
return LegacyMessageType.APPLY;
case MessageType.CONSTRUCT:
return LegacyMessageType.CONSTRUCT;
case MessageType.ENDPOINT:
return LegacyMessageType.ENDPOINT;
case MessageType.RELEASE:
return LegacyMessageType.RELEASE;
}
}

return mapMessageTypeToLegacyType(type) as MessageTypeMap[T];
}

function generateUUID(): string {
return new Array(4)
.fill(0)
Expand Down
Loading