Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
89 changes: 13 additions & 76 deletions __tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { deserialize, serialize, RpcSession, type RpcSessionOptions, RpcTranspor
type RpcTransportWithCustomEncoding, RpcTarget, RpcStub, newWebSocketRpcSession,
newMessagePortRpcSession,
newHttpBatchRpcSession} from "../src/index.js"
import { MAX_CLOSE_REASON_BYTES } from "../src/websocket.js"
import { Counter, TestTarget } from "./test-util.js";

type CustomEncodingLevel = RpcTransportWithCustomEncoding["encodingLevel"];
Expand Down Expand Up @@ -3451,67 +3452,7 @@ describe("deserialization and transport correctness", () => {
expect(String(error.message)).toContain("bad RPC message");
});

it("handles a duplicate resolve message for an import without leaking or double-releasing", async () => {
// A recording transport pair so we can observe outgoing messages and re-inject one.
class RecTransport implements RpcTransport {
sent: string[] = [];
private incoming: string[] = [];
partner!: RecTransport;
private waiter?: () => void;

async send(message: string): Promise<void> {
message = message.replaceAll("$remove$", "");
this.sent.push(message);
this.partner.deliver(message);
}

deliver(message: string) {
this.incoming.push(message);
this.waiter?.();
this.waiter = undefined;
}

async receive(): Promise<string> {
while (this.incoming.length === 0) {
await new Promise<void>(resolve => { this.waiter = resolve; });
}
return this.incoming.shift()!;
}
}

let clientT = new RecTransport();
let serverT = new RecTransport();
clientT.partner = serverT;
serverT.partner = clientT;

let client = new RpcSession<any>(clientT);
// Keep a reference so the session is not collected; the server's main is a Counter.
let _server = new RpcSession(serverT, new Counter(5));

// A normal call creates and then resolves+releases a client import.
let value = await client.getRemoteMain().increment(1);
expect(value).toBe(6);

let resolveMsg = serverT.sent.find(m => m.startsWith('["resolve"'));
expect(resolveMsg).toBeDefined();

let releasesBefore = clientT.sent.filter(m => m.startsWith('["release"')).length;
let statsBefore = client.getStats();

// Re-inject the resolve for the (now already-resolved-and-released) import. This must be
// handled cleanly: no duplicate release is emitted, the resolution hook is disposed rather
// than leaked, and the session is not aborted. The guard added in ImportTableEntry.resolve()
// is the defensive backstop for this scenario; via the public protocol the duplicate lands
// on the "import not found" path because the table entry is removed after the first
// resolution.
clientT.deliver(resolveMsg!);
await pumpMicrotasks();

expect(clientT.sent.filter(m => m.startsWith('["release"')).length).toBe(releasesBefore);
expect(client.getStats()).toStrictEqual(statsBefore);
});

it("truncates an over-long WebSocket close reason to 123 UTF-8 bytes", async () => {
it("truncates an over-long WebSocket close reason on a code-point boundary", async () => {
let closeArgs: { code: number, reason: string }[] = [];
let closeThrew = false;
let listeners: Record<string, ((ev: any) => void)[]> = {};
Expand All @@ -3523,8 +3464,8 @@ describe("deserialization and transport correctness", () => {
},
send(_message: string) { return Promise.resolve(); },
close(code: number, reason: string) {
// Mimic the real WebSocket contract: a reason longer than 123 UTF-8 bytes throws.
if (new TextEncoder().encode(reason).length > 123) {
// Mimic the real contract: an over-long reason throws.
if (new TextEncoder().encode(reason).length > MAX_CLOSE_REASON_BYTES) {
closeThrew = true;
throw new Error("close reason too long");
}
Expand All @@ -3534,23 +3475,19 @@ describe("deserialization and transport correctness", () => {

newWebSocketRpcSession(fakeWs);

// A malformed message whose resulting "bad RPC message: ..." error far exceeds 123 UTF-8
// bytes and contains multibyte characters, so truncation must land on a character boundary.
let huge = "ABC" + "\u{1F4A5}".repeat(80);
let badMessage = JSON.stringify([huge]);
for (let cb of listeners["message"] || []) {
cb({ data: badMessage });
}
// Peer-supplied abort reason that straddles the limit with a multi-byte character: the 2-byte
// "é" begins at the last allowed byte, so truncation must drop the partial "é" rather than emit
// a replacement character (itself 3 bytes, which would re-exceed the limit).
let reason = "a".repeat(MAX_CLOSE_REASON_BYTES - 1) + "\u00e9";
for (let cb of listeners["message"] || []) cb({ data: JSON.stringify(["abort", reason]) });
await pumpMicrotasks();

expect(closeThrew).toBe(false);
expect(closeArgs.length).toBe(1);
expect(closeArgs[0].code).toBe(3000);

let reason = closeArgs[0].reason;
expect(new TextEncoder().encode(reason).length).toBeLessThanOrEqual(123);
// The truncated reason is still valid (no replacement char from a split multibyte char).
expect(reason).not.toContain("\uFFFD");
expect(reason).toBe(new TextDecoder().decode(new TextEncoder().encode(reason)));
let sentReason = closeArgs[0].reason;
expect(new TextEncoder().encode(sentReason).length).toBeLessThanOrEqual(MAX_CLOSE_REASON_BYTES);
expect(sentReason).not.toContain("\uFFFD");
expect(sentReason).toBe("a".repeat(MAX_CLOSE_REASON_BYTES - 1));
});
});
7 changes: 0 additions & 7 deletions src/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,13 +205,6 @@ class ImportTableEntry {
// a forwarded call. The caller is responsible for ensuring the last of these is handed off
// before direct calls can be delivered.

if (this.resolution) {
Comment thread
ndisidore marked this conversation as resolved.
// Already resolved. A duplicate resolution would overwrite (and leak) the
// existing one, so dispose the incoming hook and ignore it.
resolution.dispose();
return;
}

if (this.localRefcount == 0) {
// Already disposed (canceled), so ignore the resolution and don't send a redundant release.
resolution.dispose();
Expand Down
17 changes: 6 additions & 11 deletions src/serialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,19 +121,14 @@ async function streamToBlob(stream: ReadableStream, type: string): Promise<Blob>
return b.type === type ? b : b.slice(0, b.size, type);
}

// Maps error name to error class for deserialization.
//
// Uses a null prototype so that an attacker-supplied error type name from the wire
// (`value[1]`) cannot resolve to an inherited `Object.prototype` member. Without this,
// e.g. `ERROR_TYPES["constructor"]` would resolve to `Object` (truthy, so the `|| Error`
// fallback below is skipped), producing `new Object(message)` -- a `String` wrapper rather
// than an `Error` (type confusion / `instanceof Error` bypass), and a name like `"toString"`
// would resolve to a non-constructor and throw. With a null prototype, every non-own key
// resolves to `undefined` and correctly falls back to `Error`.
const ERROR_TYPES: Record<string, any> = Object.assign(Object.create(null), {
// Maps error name to error class for deserialization. Null-prototype so a wire-supplied (untrusted)
// name can't resolve to an inherited member like `constructor` (which would build a `String`
// wrapper, not an `Error`) or `toString`; unknown names fall back to `Error`.
const ERROR_TYPES: Record<string, any> = {
__proto__: null,
Error, EvalError, RangeError, ReferenceError, SyntaxError, TypeError, URIError, AggregateError,
// TODO: DOMError? Others?
});
};

// Converts fully-hydrated messages into object trees that are JSON-serializable for sending over
// the wire. This is used to implement serialization -- but it doesn't take the last step of
Expand Down
12 changes: 6 additions & 6 deletions src/websocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import { RpcStub } from "./core.js";
import { RpcSession, RpcSessionOptions } from "./rpc.js";

/** Close-frame reason max UTF-8 bytes: 125 payload − 2-byte status (RFC 6455 §5.5). */
export const MAX_CLOSE_REASON_BYTES = 125 - 2;

export function newWebSocketRpcSession(
webSocket: WebSocket | string, localMain?: any, options?: RpcSessionOptions): RpcStub {
if (typeof webSocket === "string") {
Expand Down Expand Up @@ -124,13 +127,10 @@ export class WebSocketTransport<T extends string | ArrayBuffer = string> {
} else {
message = `${reason}`;
}
// WebSocket close reasons are limited to 123 UTF-8 bytes; a longer string makes
// close() throw. Truncate on a character boundary to stay within the limit.
// `stream: true` drops a trailing partial code point rather than emitting a replacement char.
let reasonBytes = new TextEncoder().encode(message);
if (reasonBytes.length > 123) {
// Decoding with { stream: true } drops any trailing partial code unit, so the
// result is guaranteed to be valid and ≤ 123 bytes.
message = new TextDecoder().decode(reasonBytes.subarray(0, 123), { stream: true });
if (reasonBytes.length > MAX_CLOSE_REASON_BYTES) {
message = new TextDecoder().decode(reasonBytes.subarray(0, MAX_CLOSE_REASON_BYTES), { stream: true });
}
this.#webSocket.close(3000, message);

Expand Down
Loading