From 2c12778f9efdf174b0d491834cde828490c89e99 Mon Sep 17 00:00:00 2001 From: Sri Krishna <7254698+srikrsna@users.noreply.github.com> Date: Wed, 5 Aug 2026 15:29:45 +0530 Subject: [PATCH 1/6] Add support for `Set` --- README.md | 3 +- __tests__/index.test.ts | 113 ++++++++++++++++++ .../capnweb-validate/src/internal/core.ts | 1 + protocol.md | 4 + src/core.ts | 61 +++++++++- src/serialize.ts | 24 +++- src/types.d.ts | 1 + 7 files changed, 203 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 10aa4f40..098ad009 100644 --- a/README.md +++ b/README.md @@ -199,6 +199,7 @@ The following types can be passed over RPC (in arguments or return values), and * Arrays * `bigint` * `Date` +* `Set` * `ArrayBuffer`, `DataView`, and typed arrays * `Error` and its well-known subclasses * `Blob` @@ -206,7 +207,7 @@ The following types can be passed over RPC (in arguments or return values), and * `Headers`, `Request`, and `Response` from the Fetch API. The following types are not supported as of this writing, but may be added in the future: -* `Map` and `Set` +* `Map` * `RegExp` The following are intentionally NOT supported: diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 722a1eee..0b1f20ba 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -27,6 +27,8 @@ let SERIALIZE_TEST_CASES: Record = { '{"foo":[[123]]}': {foo: [123]}, '{"foo":[[123]],"bar":[[456,789]]}': {foo: [123], bar: [456, 789]}, + '["set",[1,2,"abc",[[123]]]]': new Set([1, 2, "abc", [123]]), + '["bigint","123"]': 123n, '["date",1234]': new Date(1234), '["bytes","aGVsbG8h"]': new TextEncoder().encode("hello!"), @@ -1573,6 +1575,117 @@ describe("promise pipelining", () => { }); }); +describe("promises inside a Set", () => { + // A Set has no addressable positions, but delivery resolves a located promise by assigning + // `parent[property] = value`. These tests pin the behavior of the temporary setter that makes + // that work, on both the wire path (Evaluator) and the local path (RpcPayload.deepCopy). + class SetTarget extends RpcTarget { + square(i: number) { + return i * i; + } + + // Reports what actually arrived. Deliberately does not await the elements: an unresolved + // promise is still thenable, so awaiting would hide a failure to substitute it. + inspect(container: Set) { + return { + isSet: container instanceof Set, + elements: [...container].map(element => { + // RPC stubs and promises are callable, so typeof reports "function", not "object". + let objectLike = element !== null && + (typeof element === "object" || typeof element === "function"); + return objectLike && typeof (element).then === "function" + ? "" : element; + }), + // Anything here is a resolved value that was written onto the Set as a property instead + // of replacing the element it belongs to. + strayProps: Object.getOwnPropertyNames(container), + }; + } + + // Blobs are always delivered through the promise machinery, so this exercises the same path + // in the returning direction without any pipelining on the caller's part. + makeBlobSet() { + return new Set([new Blob(["first"]), new Blob(["second"])]); + } + } + + it("substitutes a promise sent inside a Set", async () => { + await using harness = new TestHarness(new SetTarget()); + let stub = harness.stub; + using promise = stub.square(3); + + let result = await stub.inspect(new Set(["alpha", promise, "omega"])); + + expect(result.isSet).toBe(true); + expect(result.elements).toStrictEqual(["alpha", 9, "omega"]); + expect(result.strayProps).toStrictEqual([]); + }); + + it("substitutes multiple promises at their own positions", async () => { + await using harness = new TestHarness(new SetTarget()); + let stub = harness.stub; + using first = stub.square(3); + using second = stub.square(4); + + // Every element must get a distinct property. Sharing one would make both writes target the + // same slot, losing all but the last. + let result = await stub.inspect(new Set(["a", first, "b", second, "c"])); + + expect(result.elements).toStrictEqual(["a", 9, "b", 16, "c"]); + expect(result.strayProps).toStrictEqual([]); + }); + + it("leaves no residue when a promise is nested inside a Set element", async () => { + await using harness = new TestHarness(new SetTarget()); + let stub = harness.stub; + using promise = stub.square(4); + + // Here the promise's parent is the inner object, not the Set, so the Set needs no setter. + let result = await stub.inspect(new Set([{value: promise}])); + + expect(result.elements).toStrictEqual([{value: 16}]); + expect(result.strayProps).toStrictEqual([]); + }); + + it("collapses a resolution that equals an existing element", async () => { + await using harness = new TestHarness(new SetTarget()); + let stub = harness.stub; + using promise = stub.square(3); + + // Rebuilding the Set re-applies Set semantics, so the duplicate 9 drops out. + let result = await stub.inspect(new Set([9, promise, "tail"])); + + expect(result.elements).toStrictEqual([9, "tail"]); + expect(result.strayProps).toStrictEqual([]); + }); + + it("substitutes Blobs in a Set returned to the caller", async () => { + await using harness = new TestHarness(new SetTarget()); + + let received = await harness.stub.makeBlobSet(); + + expect(received).toBeInstanceOf(Set); + expect(Object.getOwnPropertyNames(received)).toStrictEqual([]); + expect(await Promise.all([...received].map(blob => blob.text()))) + .toStrictEqual(["first", "second"]); + }); + + it("substitutes a promise in a Set passed to a local stub", async () => { + using stub = new RpcStub(new SetTarget()); + using promise = stub.square(3); + let source = new Set(["alpha", promise, "omega"]); + + let result = await stub.inspect(source); + + expect(result.elements).toStrictEqual(["alpha", 9, "omega"]); + expect(result.strayProps).toStrictEqual([]); + + // deepCopy() must fix up its copy, not the caller's Set. + expect([...source][1]).toBe(promise); + expect(Object.getOwnPropertyNames(source)).toStrictEqual([]); + }); +}); + describe("map() over RPC", () => { it("supports map() on nulls", async () => { let counter = new RpcStub(new Counter(0)); diff --git a/packages/capnweb-validate/src/internal/core.ts b/packages/capnweb-validate/src/internal/core.ts index 8e4b8b69..8ef199b2 100644 --- a/packages/capnweb-validate/src/internal/core.ts +++ b/packages/capnweb-validate/src/internal/core.ts @@ -75,6 +75,7 @@ type BaseType = | bigint | string | Date + | Set | Error | RegExp | Blob diff --git a/protocol.md b/protocol.md index c6c8abd7..ed5ec369 100644 --- a/protocol.md +++ b/protocol.md @@ -191,6 +191,10 @@ bound parsing cost. A JavaScript `Date` value. The number represents milliseconds since the Unix epoch. +`["set", elements]` + +A JavaScript `Set` value. `elements` is an array of the set elements. + `["error", type, message, stack?, props?]` A JavaScript `Error` value. `type` is the name of the specific well-known `Error` subclass, e.g. "TypeError". `message` is a string containing the error message. `stack` may optionally contain the stack trace, though by default stacks will be redacted for security reasons. diff --git a/src/core.ts b/src/core.ts index f60750a7..81b88192 100644 --- a/src/core.ts +++ b/src/core.ts @@ -37,7 +37,7 @@ export let RpcTarget = workersModule ? workersModule.RpcTarget : class {}; export type PropertyPath = (string | number)[]; -type TypeForRpc = "unsupported" | "primitive" | "object" | "function" | "array" | "date" | +type TypeForRpc = "unsupported" | "primitive" | "object" | "function" | "array" | "date" | "set" | "bigint" | "bytes" | "blob" | "stub" | "rpc-promise" | "rpc-target" | "rpc-thenable" | "error" | "undefined" | "writable" | "readable" | "headers" | "request" | "response"; @@ -93,6 +93,9 @@ export function typeForRpc(value: unknown): TypeForRpc { case Date.prototype: return "date"; + case Set.prototype: + return "set"; + case Uint8Array.prototype: case BUFFER_PROTOTYPE: case ArrayBuffer.prototype: @@ -626,6 +629,28 @@ export function unwrapStubAndPath(stub: RpcStub): {hook: StubHook, pathIfPromise return stub[RAW_STUB]; } +// RpcPromise elements are set using property access on the parent. +// +// To make this work for `Set`, this function defines a one-time use setter that inserts the +// resolved value in the correct order and rebuilds the set. +// +// The `property` must be unique for each element. +export function defineSetPromiseSlot(set: Set, property: string, placeholder: unknown) { + if (!(placeholder instanceof RpcPromise)) return; + Object.defineProperty(set, property, { + configurable: true, enumerable: false, + set(resolved: unknown) { + let elms = [...set]; + let ri = elms.indexOf(placeholder); + delete (set as any)[property] + set.clear() + for (let i = 0; i < elms.length; i++) { + set.add(i === ri ? resolved : elms[i]); + } + } + }); +} + // Given a promise stub (still wrapped in a Proxy), pull the remote promise and deliver the // payload. This is a helper used to implement the then/catch/finally methods of RpcPromise. async function pullPromise(promise: RpcPromise): Promise { @@ -974,13 +999,28 @@ export class RpcPayload { // parent. let array = >value; let len = array.length; - let result = new Array(len); + let result = new Array(len); for (let i = 0; i < len; i++) { result[i] = this.deepCopy(array[i], array, i, result, dupStubs, owner); } return result; } + case "set": { + // We have to construct the new set first, then fill it in, so we can pass it as the + // parent. + let set = >value; + let result = new Set(); + let counter = 0; + for (let val of set) { + let key = `${counter++}`; + let copy = this.deepCopy(val, set, key, result, dupStubs, owner); + defineSetPromiseSlot(result, key, copy); + result.add(copy) + } + return result; + } + case "object": { // Plain object. Unfortunately there's no way to pre-allocate the right shape. let result: Record = {}; @@ -1357,6 +1397,14 @@ export class RpcPayload { return; } + case "set": { + let set = >value; + for (let element of >value) { + this.disposeImpl(element, set) + } + return; + } + case "object": { let object = >value; for (let i in object) { @@ -1503,6 +1551,14 @@ export class RpcPayload { return; } + case "set": { + let set = >value; + for (let element of set) { + this.ignoreUnhandledRejectionsImpl(element); + } + return; + } + case "object": { let object = >value; for (let i in object) { @@ -1641,6 +1697,7 @@ function followPath(value: unknown, parent: object | undefined, case "bytes": case "blob": case "date": + case "set": case "error": case "headers": case "request": diff --git a/src/serialize.ts b/src/serialize.ts index 9cd578c5..a8b5df4d 100644 --- a/src/serialize.ts +++ b/src/serialize.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license found in the LICENSE.txt file or at: // https://opensource.org/license/mit -import { StubHook, RpcPayload, typeForRpc, RpcStub, RpcPromise, LocatedPromise, RpcTarget, unwrapStubAndPath, streamImpl, PromiseStubHook, PayloadStubHook } from "./core.js"; +import { StubHook, RpcPayload, typeForRpc, RpcStub, RpcPromise, LocatedPromise, RpcTarget, unwrapStubAndPath, streamImpl, PromiseStubHook, PayloadStubHook, defineSetPromiseSlot } from "./core.js"; export type ImportId = number; export type ExportId = number; @@ -319,6 +319,15 @@ export class Devaluator { return [result]; } + case "set": { + let set = >value; + let elements: unknown[] = []; + for (let element of set) { + elements.push(this.devaluateImpl(element, set, depth + 1)) + } + return ["set", elements]; + } + case "bigint": // At structuredClonable level, keep BigInt as native value if (this.encodingLevel === "structuredClonable") { @@ -841,6 +850,19 @@ export class Evaluator { return new Date(value[1]); } break; + case "set": + if (value.length === 2 && value[1] instanceof Array) { + let set = new Set(); + let counter = 0; + for (let element of value[1]) { + let key = `${counter++}` + let copy = this.evaluateImpl(element, set, key, depth + 1) + defineSetPromiseSlot(set, key, copy) + set.add(copy); + } + return set; + } + break; case "bytes": { let bytes: Uint8Array; // At jsonCompatibleWithBytes/structuredClonable level, bytes may already be raw. diff --git a/src/types.d.ts b/src/types.d.ts index 2c043e30..996bcdef 100644 --- a/src/types.d.ts +++ b/src/types.d.ts @@ -85,6 +85,7 @@ type BaseType = | ArrayBuffer | DataView | Date + | Set | Error | RegExp | Blob From 10c2fc289d6194de3b530c405ffea6972e3b5676 Mon Sep 17 00:00:00 2001 From: Sri Krishna <7254698+srikrsna@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:31:01 +0530 Subject: [PATCH 2/6] Add changeset --- .changeset/serialize-set.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/serialize-set.md diff --git a/.changeset/serialize-set.md b/.changeset/serialize-set.md new file mode 100644 index 00000000..60401065 --- /dev/null +++ b/.changeset/serialize-set.md @@ -0,0 +1,5 @@ +--- +"capnweb": minor +--- + +Support serializing `Set` objects over RPC. \ No newline at end of file From 4d1960427458a148b81e8f10e9d413e0ea23e154 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala <7254698+srikrsna@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:12:46 +0530 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com> --- src/core.ts | 14 ++++++++------ src/serialize.ts | 15 +++++++++------ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/core.ts b/src/core.ts index 81b88192..790a0e2b 100644 --- a/src/core.ts +++ b/src/core.ts @@ -629,9 +629,9 @@ export function unwrapStubAndPath(stub: RpcStub): {hook: StubHook, pathIfPromise return stub[RAW_STUB]; } -// RpcPromise elements are set using property access on the parent. -// -// To make this work for `Set`, this function defines a one-time use setter that inserts the +// RpcPromise elements are set using property access on the parent. +// +// To make this work for `Set`, this function defines a one-time-use setter that inserts the // resolved value in the correct order and rebuilds the set. // // The `property` must be unique for each element. @@ -999,7 +999,7 @@ export class RpcPayload { // parent. let array = >value; let len = array.length; - let result = new Array(len); + let result = new Array(len); for (let i = 0; i < len; i++) { result[i] = this.deepCopy(array[i], array, i, result, dupStubs, owner); } @@ -1015,7 +1015,8 @@ export class RpcPayload { for (let val of set) { let key = `${counter++}`; let copy = this.deepCopy(val, set, key, result, dupStubs, owner); - defineSetPromiseSlot(result, key, copy); + defineSetPromiseSlot(result, key, copy); + result.add(copy); result.add(copy) } return result; @@ -1400,7 +1401,8 @@ export class RpcPayload { case "set": { let set = >value; for (let element of >value) { - this.disposeImpl(element, set) + for (let element of set) { + this.disposeImpl(element, set); } return; } diff --git a/src/serialize.ts b/src/serialize.ts index a8b5df4d..7878daf5 100644 --- a/src/serialize.ts +++ b/src/serialize.ts @@ -320,10 +320,10 @@ export class Devaluator { } case "set": { - let set = >value; + let set = >value; let elements: unknown[] = []; - for (let element of set) { - elements.push(this.devaluateImpl(element, set, depth + 1)) + for (let element of set) { + elements.push(this.devaluateImpl(element, set, depth + 1)); } return ["set", elements]; } @@ -855,13 +855,16 @@ export class Evaluator { let set = new Set(); let counter = 0; for (let element of value[1]) { - let key = `${counter++}` - let copy = this.evaluateImpl(element, set, key, depth + 1) - defineSetPromiseSlot(set, key, copy) + let key = `${counter++}`; + let copy = this.evaluateImpl(element, set, key, depth + 1); + defineSetPromiseSlot(set, key, copy); set.add(copy); } return set; } + break; + return set; + } break; case "bytes": { let bytes: Uint8Array; From 0942d18f337ccdcc67cdf338a36a674aa360b750 Mon Sep 17 00:00:00 2001 From: Sri Krishna <7254698+srikrsna@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:27:05 +0530 Subject: [PATCH 4/6] Fix suggestions --- src/core.ts | 2 -- src/serialize.ts | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/core.ts b/src/core.ts index 790a0e2b..990a3113 100644 --- a/src/core.ts +++ b/src/core.ts @@ -1017,7 +1017,6 @@ export class RpcPayload { let copy = this.deepCopy(val, set, key, result, dupStubs, owner); defineSetPromiseSlot(result, key, copy); result.add(copy); - result.add(copy) } return result; } @@ -1400,7 +1399,6 @@ export class RpcPayload { case "set": { let set = >value; - for (let element of >value) { for (let element of set) { this.disposeImpl(element, set); } diff --git a/src/serialize.ts b/src/serialize.ts index 7878daf5..d5a1e8ed 100644 --- a/src/serialize.ts +++ b/src/serialize.ts @@ -862,9 +862,6 @@ export class Evaluator { } return set; } - break; - return set; - } break; case "bytes": { let bytes: Uint8Array; From 94cba7735b47a546a35ecb7271f88c5cc3be6b93 Mon Sep 17 00:00:00 2001 From: Sri Krishna <7254698+srikrsna@users.noreply.github.com> Date: Thu, 6 Aug 2026 12:55:09 +0530 Subject: [PATCH 5/6] Fix format --- src/core.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core.ts b/src/core.ts index 990a3113..ed93636d 100644 --- a/src/core.ts +++ b/src/core.ts @@ -642,8 +642,8 @@ export function defineSetPromiseSlot(set: Set, property: string, placeh set(resolved: unknown) { let elms = [...set]; let ri = elms.indexOf(placeholder); - delete (set as any)[property] - set.clear() + delete (set as any)[property]; + set.clear(); for (let i = 0; i < elms.length; i++) { set.add(i === ri ? resolved : elms[i]); } From 2fe183d3f1863d8ba161a28c3e7b09c8e9249d87 Mon Sep 17 00:00:00 2001 From: Sri Krishna <7254698+srikrsna@users.noreply.github.com> Date: Thu, 13 Aug 2026 19:11:39 +0530 Subject: [PATCH 6/6] Reject promises, stubs, and Blobs as Set elements over the wire --- .changeset/serialize-set.md | 4 +- README.md | 3 +- __tests__/index.test.ts | 182 +++++++++++++++++++++++++++--------- protocol.md | 2 + src/core.ts | 33 ++----- src/serialize.ts | 48 ++++++++-- 6 files changed, 192 insertions(+), 80 deletions(-) diff --git a/.changeset/serialize-set.md b/.changeset/serialize-set.md index 60401065..dcce74f7 100644 --- a/.changeset/serialize-set.md +++ b/.changeset/serialize-set.md @@ -2,4 +2,6 @@ "capnweb": minor --- -Support serializing `Set` objects over RPC. \ No newline at end of file +Support serializing `Set` objects over RPC. + +A `Set` carries plain data only: promises, stubs, and `Blob`s are not allowed as elements, and sending one over a connection throws a `TypeError`. diff --git a/README.md b/README.md index 098ad009..30cdc23b 100644 --- a/README.md +++ b/README.md @@ -199,7 +199,7 @@ The following types can be passed over RPC (in arguments or return values), and * Arrays * `bigint` * `Date` -* `Set` +* `Set`, except that an element may not be a promise, stub, or `Blob` (see below) * `ArrayBuffer`, `DataView`, and typed arrays * `Error` and its well-known subclasses * `Blob` @@ -213,6 +213,7 @@ The following types are not supported as of this writing, but may be added in th The following are intentionally NOT supported: * Application-defined classes that do not extend `RpcTarget`. * Cyclic values. Messages are serialized strictly as trees (like JSON). +* Promises, stubs, and `Blob`s as elements of a `Set`. ### `RpcTarget` diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 0b1f20ba..6352c66e 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -1575,10 +1575,22 @@ describe("promise pipelining", () => { }); }); -describe("promises inside a Set", () => { - // A Set has no addressable positions, but delivery resolves a located promise by assigning - // `parent[property] = value`. These tests pin the behavior of the temporary setter that makes - // that work, on both the wire path (Evaluator) and the local path (RpcPayload.deepCopy). +describe("promises, stubs and Blobs inside a Set", () => { + // A Set carries plain data only: no promises, stubs or Blobs as elements. A promise or Blob is + // delivered by substituting the value into its parent, i.e. `parent[property] = value`, and a Set + // has no property that names an element. Rather than mutate the Set behind the application's + // back, we reject it. Stubs are excluded along with them so a Set behaves the same in both + // directions. These tests pin that down on the send path (Devaluator), the local path + // (RpcPayload.deepCopy), and the receive path (Evaluator). + // + // Each path converts the element first and then checks what came back, instead of inspecting the + // element's type up front. That's why a Blob or stub is rejected over a connection but allowed on + // a local call, where nothing is encoded. + const PROMISE_ERROR = "Cannot serialize a promise as an element of a Set"; + const BLOB_ERROR = "Cannot serialize a Blob as an element of a Set"; + const DESERIALIZE_ERROR = "Cannot deserialize a stub or promise as an element of a Set"; + const STUB_ERROR = "Cannot serialize a stub as an element of a Set"; + class SetTarget extends RpcTarget { square(i: number) { return i * i; @@ -1602,87 +1614,167 @@ describe("promises inside a Set", () => { }; } - // Blobs are always delivered through the promise machinery, so this exercises the same path - // in the returning direction without any pipelining on the caller's part. + // Sending a Blob over a connection means streaming it, so it arrives behind a promise and hits + // the same restriction, even though the application never created a promise. makeBlobSet() { - return new Set([new Blob(["first"]), new Blob(["second"])]); + return new Set([new Blob(["first"])]); + } + + // Hands the stub straight back inside a Set. Since the caller is the one that exported it, the + // caller's Evaluator sees the element as an ["import", id] expression. + bounce(stub: any) { + return new Set([stub.dup()]); + } + + makeCounter(i: number) { + return new Counter(i); + } + + // Stubs are passed by reference rather than substituted, so they remain legal elements. + async incrementAll(container: Set) { + let results: number[] = []; + for (let element of container) { + results.push(await (element).increment(1)); + } + return {isSet: container instanceof Set, results}; } } - it("substitutes a promise sent inside a Set", async () => { + it("rejects a promise sent inside a Set", async () => { await using harness = new TestHarness(new SetTarget()); let stub = harness.stub; using promise = stub.square(3); - let result = await stub.inspect(new Set(["alpha", promise, "omega"])); + // Thrown synchronously at the call site, like any other unserializable argument. + expect(() => stub.inspect(new Set(["alpha", promise, "omega"]))) + .toThrow(PROMISE_ERROR); - expect(result.isSet).toBe(true); - expect(result.elements).toStrictEqual(["alpha", 9, "omega"]); - expect(result.strayProps).toStrictEqual([]); + // The failed call must not have poisoned the session. + expect(await stub.inspect(new Set(["alpha", "omega"]))) + .toStrictEqual({isSet: true, elements: ["alpha", "omega"], strayProps: []}); }); - it("substitutes multiple promises at their own positions", async () => { + it("rejects a promise in a Set passed to a local stub", async () => { + using stub = new RpcStub(new SetTarget()); + using promise = stub.square(3); + let source = new Set(["alpha", promise, "omega"]); + + // The local path copies at delivery time, so this surfaces as a rejection. + await expect(() => stub.inspect(source)).rejects.toThrow(PROMISE_ERROR); + + // The caller's Set is left exactly as it was. + expect([...source]).toStrictEqual(["alpha", promise, "omega"]); + expect(Object.getOwnPropertyNames(source)).toStrictEqual([]); + }); + + it("rejects a Blob sent inside a Set", async () => { + await using harness = new TestHarness(new SetTarget()); + + // Caught while encoding, before the Blob's pipe is created. The harness checks at the end of + // the test that no import or export leaked, which is what creating the pipe and then throwing + // would do. + expect(() => harness.stub.inspect(new Set([new Blob(["hello"])]))).toThrow(BLOB_ERROR); + }); + + it("rejects a Blob in a Set returned to the caller", async () => { await using harness = new TestHarness(new SetTarget()); - let stub = harness.stub; - using first = stub.square(3); - using second = stub.square(4); - // Every element must get a distinct property. Sharing one would make both writes target the - // same slot, losing all but the last. - let result = await stub.inspect(new Set(["a", first, "b", second, "c"])); + // Caught by the server as it serializes its result. Note the arrow function: an RpcPromise is + // callable, so passing one to `expect(...).rejects` directly would make vitest invoke it. + await expect(() => harness.stub.makeBlobSet()).rejects.toThrow(BLOB_ERROR); + }); + + it("rejects a Set containing a Blob in plain serialize()", () => { + expect(() => serialize(new Set([new Blob(["hello"])]))).toThrow(BLOB_ERROR); + }); - expect(result.elements).toStrictEqual(["a", 9, "b", 16, "c"]); + it("accepts a Blob in a Set passed to a local stub", async () => { + using stub = new RpcStub(new SetTarget()); + let blob = new Blob(["hello"]); + + // A same-process call streams nothing. The app receives this very Blob, so no promise is + // involved and there is nothing to substitute. Only a Blob crossing a connection is a problem. + let result = await stub.inspect(new Set([blob])); + + expect(result.elements).toStrictEqual([blob]); expect(result.strayProps).toStrictEqual([]); }); - it("leaves no residue when a promise is nested inside a Set element", async () => { + it("accepts a promise nested inside a Set element", async () => { await using harness = new TestHarness(new SetTarget()); let stub = harness.stub; using promise = stub.square(4); - // Here the promise's parent is the inner object, not the Set, so the Set needs no setter. + // Only a promise that is *itself* an element is a problem. Here the promise's parent is the + // inner object, which has a property to write the resolution to. let result = await stub.inspect(new Set([{value: promise}])); expect(result.elements).toStrictEqual([{value: 16}]); expect(result.strayProps).toStrictEqual([]); }); - it("collapses a resolution that equals an existing element", async () => { + it("rejects a stub sent inside a Set", async () => { await using harness = new TestHarness(new SetTarget()); - let stub = harness.stub; - using promise = stub.square(3); - - // Rebuilding the Set re-applies Set semantics, so the duplicate 9 drops out. - let result = await stub.inspect(new Set([9, promise, "tail"])); + using counter = new RpcStub(new Counter(5)); - expect(result.elements).toStrictEqual([9, "tail"]); - expect(result.strayProps).toStrictEqual([]); + // A Set holds plain data only, so a stub the sender owns is out too. It encodes as ["export"]. + expect(() => harness.stub.incrementAll(new Set([counter]))).toThrow(STUB_ERROR); }); - it("substitutes Blobs in a Set returned to the caller", async () => { + it("rejects a stub pointing back at the peer inside a Set", async () => { await using harness = new TestHarness(new SetTarget()); - let received = await harness.stub.makeBlobSet(); + // The other encoding of a stub: this one is the peer's own capability, so it goes out as + // ["import"] rather than ["export"]. + using counter = await harness.stub.makeCounter(5); + + expect(() => harness.stub.incrementAll(new Set([counter]))).toThrow(STUB_ERROR); + }); + + it("rejects a stub in a Set returned to the caller", async () => { + await using harness = new TestHarness(new SetTarget()); + using counter = new RpcStub(new Counter(5)); - expect(received).toBeInstanceOf(Set); - expect(Object.getOwnPropertyNames(received)).toStrictEqual([]); - expect(await Promise.all([...received].map(blob => blob.text()))) - .toStrictEqual(["first", "second"]); + // Caught by the server as it serializes its result, and reported as the call's rejection. + await expect(() => harness.stub.bounce(counter)).rejects.toThrow(STUB_ERROR); }); - it("substitutes a promise in a Set passed to a local stub", async () => { + it("accepts a stub in a Set passed to a local stub", async () => { using stub = new RpcStub(new SetTarget()); - using promise = stub.square(3); - let source = new Set(["alpha", promise, "omega"]); + using counter = new RpcStub(new Counter(5)); - let result = await stub.inspect(source); + // Same leniency as a Blob on this path: nothing is encoded, so the app just gets the stub. + expect(await stub.incrementAll(new Set([counter]))) + .toStrictEqual({isSet: true, results: [6]}); + }); - expect(result.elements).toStrictEqual(["alpha", 9, "omega"]); - expect(result.strayProps).toStrictEqual([]); + it("rejects a promise arriving inside a Set from a peer", async () => { + // The sender-side check above means a well-behaved peer never produces this message, so we + // have to forge it: rewrite the outgoing call so the argument that was an array containing a + // pipelined promise becomes a *Set* containing that same promise. This exercises the + // receiver's own guard, which is what stops a hostile or buggy peer from corrupting a Set. + // + // Not using `await using`: the forged message breaks the session, so the harness's + // end-of-test "everything was disposed" check does not apply. + let harness = new TestHarness(new SetTarget()); + let stub = harness.stub; - // deepCopy() must fix up its copy, not the caller's Set. - expect([...source][1]).toBe(promise); - expect(Object.getOwnPropertyNames(source)).toStrictEqual([]); + let origSend = harness.clientTransport.send; + harness.clientTransport.send = function(message: string) { + let rewritten = JSON.stringify(JSON.parse(message), function(_key, value) { + // Match the escaped-array encoding `[[]]` and re-encode it as `["set", [...]]`. + if (value instanceof Array && value.length === 1 && + value[0] instanceof Array && value[0].length === 1 && + value[0][0] instanceof Array && value[0][0][0] === "pipeline") { + return ["set", value[0]]; + } + return value; + }); + return origSend.call(this, rewritten); + }; + + using promise = stub.square(3); + await expect(() => stub.inspect([promise] as any)).rejects.toThrow(DESERIALIZE_ERROR); }); }); diff --git a/protocol.md b/protocol.md index ed5ec369..e4575433 100644 --- a/protocol.md +++ b/protocol.md @@ -195,6 +195,8 @@ A JavaScript `Date` value. The number represents milliseconds since the Unix epo A JavaScript `Set` value. `elements` is an array of the set elements. +An element must not be a promise, a stub, or a blob. + `["error", type, message, stack?, props?]` A JavaScript `Error` value. `type` is the name of the specific well-known `Error` subclass, e.g. "TypeError". `message` is a string containing the error message. `stack` may optionally contain the stack trace, though by default stacks will be redacted for security reasons. diff --git a/src/core.ts b/src/core.ts index ed93636d..a2416b53 100644 --- a/src/core.ts +++ b/src/core.ts @@ -629,28 +629,6 @@ export function unwrapStubAndPath(stub: RpcStub): {hook: StubHook, pathIfPromise return stub[RAW_STUB]; } -// RpcPromise elements are set using property access on the parent. -// -// To make this work for `Set`, this function defines a one-time-use setter that inserts the -// resolved value in the correct order and rebuilds the set. -// -// The `property` must be unique for each element. -export function defineSetPromiseSlot(set: Set, property: string, placeholder: unknown) { - if (!(placeholder instanceof RpcPromise)) return; - Object.defineProperty(set, property, { - configurable: true, enumerable: false, - set(resolved: unknown) { - let elms = [...set]; - let ri = elms.indexOf(placeholder); - delete (set as any)[property]; - set.clear(); - for (let i = 0; i < elms.length; i++) { - set.add(i === ri ? resolved : elms[i]); - } - } - }); -} - // Given a promise stub (still wrapped in a Proxy), pull the remote promise and deliver the // payload. This is a helper used to implement the then/catch/finally methods of RpcPromise. async function pullPromise(promise: RpcPromise): Promise { @@ -1011,11 +989,14 @@ export class RpcPayload { // parent. let set = >value; let result = new Set(); - let counter = 0; + let index = 0; for (let val of set) { - let key = `${counter++}`; - let copy = this.deepCopy(val, set, key, result, dupStubs, owner); - defineSetPromiseSlot(result, key, copy); + let copy = this.deepCopy(val, set, index++, result, dupStubs, owner); + if (copy instanceof RpcPromise) { + throw new TypeError( + "Cannot serialize a promise as an element of a Set. Await the value before " + + "adding it to the Set."); + } result.add(copy); } return result; diff --git a/src/serialize.ts b/src/serialize.ts index d5a1e8ed..1017aceb 100644 --- a/src/serialize.ts +++ b/src/serialize.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license found in the LICENSE.txt file or at: // https://opensource.org/license/mit -import { StubHook, RpcPayload, typeForRpc, RpcStub, RpcPromise, LocatedPromise, RpcTarget, unwrapStubAndPath, streamImpl, PromiseStubHook, PayloadStubHook, defineSetPromiseSlot } from "./core.js"; +import { StubHook, RpcPayload, typeForRpc, RpcStub, RpcPromise, LocatedPromise, RpcTarget, unwrapStubAndPath, streamImpl, PromiseStubHook, PayloadStubHook } from "./core.js"; export type ImportId = number; export type ExportId = number; @@ -323,7 +323,27 @@ export class Devaluator { let set = >value; let elements: unknown[] = []; for (let element of set) { - elements.push(this.devaluateImpl(element, set, depth + 1)); + let encoded = this.devaluateImpl(element, set, depth + 1); + + // A Set holds plain data only: no promises, stubs or blobs (see protocol.md). Blobs are + // rejected by the `blob` case, which has to refuse one before it starts a pipe. + if (encoded instanceof Array && typeof encoded[0] === "string") { + switch (encoded[0]) { + case "promise": + case "pipeline": + case "remap": + throw new TypeError( + "Cannot serialize a promise as an element of a Set. Await the value before " + + "adding it to the Set."); + + case "export": + case "import": + throw new TypeError( + "Cannot serialize a stub as an element of a Set."); + } + } + + elements.push(encoded); } return ["set", elements]; } @@ -522,6 +542,14 @@ export class Devaluator { // synchronously, and we MUST serialize the message synchronously. Hence, we have no choice // but to use streaming even for small blobs. let blob = value as Blob; + + // The receiver must buffer the whole stream before it can construct the Blob, so it + // delivers the Blob as a promise. Set doesn't support a promise. To avoid the creation of a + // pipe, we reject it here. + if (parent instanceof Set) { + throw new TypeError("Cannot serialize a Blob as an element of a Set."); + } + let readable = blob.stream(); let hook = streamImpl.createReadableStreamHook(readable); let importId = this.exporter.createPipe(readable, hook); @@ -852,12 +880,18 @@ export class Evaluator { break; case "set": if (value.length === 2 && value[1] instanceof Array) { + let elements = value[1]; let set = new Set(); - let counter = 0; - for (let element of value[1]) { - let key = `${counter++}`; - let copy = this.evaluateImpl(element, set, key, depth + 1); - defineSetPromiseSlot(set, key, copy); + for (let i = 0; i < elements.length; i++) { + // A Set holds plain data only: no promises, stubs or blobs (see protocol.md). All + // three evaluate to an RpcStub, since RpcPromise extends it and a ["blob"] arrives as + // a promise for the streamed bytes. Nothing reads the `i` we pass as the property, + // but it stays a real index because "response" and "blob" elements pass the pair + // down to their contents. + let copy = this.evaluateImpl(elements[i], set, i, depth + 1); + if (copy instanceof RpcStub) { + throw new TypeError("Cannot deserialize a stub or promise as an element of a Set."); + } set.add(copy); } return set;