From c64df8179d12ebb836725c464e3cd7882a8b9e45 Mon Sep 17 00:00:00 2001 From: Igor Loskutov Date: Sat, 14 Mar 2026 20:26:56 -0400 Subject: [PATCH] fix: prevent identity mismatch on reconnect during in-flight retrieveId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit new Peer() → disconnect() → reconnect() while retrieveId() is in-flight creates a WebSocket registered with a null ID on the server, causing all signaling to silently fail. Three independent fixes: - Guard _initialize() against destroyed/disconnected state - Abort in-flight retrieveId() fetch on disconnect via AbortController - Reject reconnect() when _lastServerId is null (no ID was ever assigned) --- __test__/peer.identity.spec.ts | 134 +++++++++++++++++++++++++++++++++ lib/api.ts | 10 ++- lib/peer.ts | 31 +++++++- 3 files changed, 168 insertions(+), 7 deletions(-) create mode 100644 __test__/peer.identity.spec.ts diff --git a/__test__/peer.identity.spec.ts b/__test__/peer.identity.spec.ts new file mode 100644 index 000000000..e1c5a1356 --- /dev/null +++ b/__test__/peer.identity.spec.ts @@ -0,0 +1,134 @@ +import "./setup"; +import { Peer } from "../lib/peer"; +import { API } from "../lib/api"; +import { PeerErrorType } from "../lib/enums"; +import { expect, describe, it, jest, afterEach } from "@jest/globals"; + +describe("Peer identity mismatch on reconnect", () => { + let peer: Peer; + + afterEach(() => { + peer?.destroy(); + }); + + it("disconnect+reconnect during retrieveId should not open socket with null id", async () => { + // retrieveId returns a promise that never resolves (simulates in-flight) + let resolveId: (id: string) => void; + jest + .spyOn(API.prototype, "retrieveId") + .mockImplementation( + () => new Promise((resolve) => (resolveId = resolve)), + ); + + peer = new Peer({ host: "localhost", port: 8080 }); + + const socketStartSpy = jest.spyOn(peer.socket, "start"); + + // disconnect before retrieveId resolves + peer.disconnect(); + + // now reconnect - _lastServerId is null + peer.reconnect(); + + // socket.start should NOT have been called with null id + for (const call of socketStartSpy.mock.calls) { + expect(call[0]).not.toBeNull(); + expect(call[0]).not.toBe("null"); + expect(call[0]).toBeTruthy(); + } + + socketStartSpy.mockRestore(); + }); + + it("late retrieveId resolve after disconnect should not initialize", async () => { + let resolveId: (id: string) => void; + jest + .spyOn(API.prototype, "retrieveId") + .mockImplementation( + () => new Promise((resolve) => (resolveId = resolve)), + ); + + peer = new Peer({ host: "localhost", port: 8080 }); + + const socketStartSpy = jest.spyOn(peer.socket, "start"); + + // disconnect before retrieveId resolves + peer.disconnect(); + + // now resolve the id - _initialize should be guarded + resolveId!("late-id"); + + // wait for microtask to flush + await Promise.resolve(); + + // socket.start should NOT have been called after disconnect + expect(socketStartSpy).not.toHaveBeenCalled(); + + socketStartSpy.mockRestore(); + }); + + it("reconnect with null _lastServerId should emit error", () => { + let resolveId: (id: string) => void; + jest + .spyOn(API.prototype, "retrieveId") + .mockImplementation( + () => new Promise((resolve) => (resolveId = resolve)), + ); + + peer = new Peer({ host: "localhost", port: 8080 }); + + const socketStartSpy = jest.spyOn(peer.socket, "start"); + + peer.disconnect(); + + const errors: { type: string }[] = []; + peer.on("error", (err) => errors.push(err)); + + peer.reconnect(); + + expect(errors.length).toBe(1); + expect(errors[0].type).toBe(PeerErrorType.Disconnected); + expect(socketStartSpy).not.toHaveBeenCalled(); + + socketStartSpy.mockRestore(); + }); + + it("retrieveId rejection after disconnect should not emit error", async () => { + let rejectId: (err: Error) => void; + jest + .spyOn(API.prototype, "retrieveId") + .mockImplementation( + () => new Promise((_, reject) => (rejectId = reject)), + ); + + peer = new Peer({ host: "localhost", port: 8080 }); + + peer.disconnect(); + + const errors: { type: string }[] = []; + peer.on("error", (err) => errors.push(err)); + + // reject the retrieveId - should be swallowed since peer is disconnected + rejectId!(new Error("aborted")); + + // wait for microtask + await Promise.resolve(); + await Promise.resolve(); + + expect(errors.length).toBe(0); + }); + + it("destroy during retrieveId + reconnect throws", async () => { + jest + .spyOn(API.prototype, "retrieveId") + .mockImplementation(() => new Promise(() => {})); + + peer = new Peer({ host: "localhost", port: 8080 }); + + peer.destroy(); + + expect(() => peer.reconnect()).toThrow( + "This peer cannot reconnect to the server. It has already been destroyed.", + ); + }); +}); diff --git a/lib/api.ts b/lib/api.ts index d17d29929..22a288f49 100644 --- a/lib/api.ts +++ b/lib/api.ts @@ -6,7 +6,10 @@ import { version } from "./version"; export class API { constructor(private readonly _options: PeerJSOption) {} - private _buildRequest(method: string): Promise { + private _buildRequest( + method: string, + options?: { signal?: AbortSignal }, + ): Promise { const protocol = this._options.secure ? "https" : "http"; const { host, port, path, key } = this._options; const url = new URL(`${protocol}://${host}:${port}${path}${key}/${method}`); @@ -15,13 +18,14 @@ export class API { url.searchParams.set("version", version); return fetch(url.href, { referrerPolicy: this._options.referrerPolicy, + signal: options?.signal, }); } /** Get a unique ID from the server via XHR and initialize with it. */ - async retrieveId(): Promise { + async retrieveId(options?: { signal?: AbortSignal }): Promise { try { - const response = await this._buildRequest("id"); + const response = await this._buildRequest("id", options); if (response.status !== 200) { throw new Error(`Error. Status:${response.status}`); diff --git a/lib/peer.ts b/lib/peer.ts index 6fa306cbc..89e1f047c 100644 --- a/lib/peer.ts +++ b/lib/peer.ts @@ -127,6 +127,7 @@ export class Peer extends EventEmitterWithError { private _id: string | null = null; private _lastServerId: string | null = null; + private _retrieveIdController: AbortController | null = null; // States. private _destroyed = false; // Connections have been killed @@ -291,10 +292,15 @@ export class Peer extends EventEmitterWithError { if (userId) { this._initialize(userId); } else { + this._retrieveIdController = new AbortController(); this._api - .retrieveId() + .retrieveId({ signal: this._retrieveIdController.signal }) .then((id) => this._initialize(id)) - .catch((error) => this._abort(PeerErrorType.ServerError, error)); + .catch((error) => { + if (!this.destroyed && !this.disconnected) { + this._abort(PeerErrorType.ServerError, error); + } + }); } } @@ -341,6 +347,9 @@ export class Peer extends EventEmitterWithError { /** Initialize a connection with the server. */ private _initialize(id: string): void { + if (this.destroyed || this.disconnected) { + return; + } this._id = id; this.socket.start(id, this._options.token!); } @@ -688,6 +697,9 @@ export class Peer extends EventEmitterWithError { logger.log(`Disconnect peer with ID:${currentId}`); + this._retrieveIdController?.abort(); + this._retrieveIdController = null; + this._disconnected = true; this._open = false; @@ -708,11 +720,18 @@ export class Peer extends EventEmitterWithError { */ reconnect(): void { if (this.disconnected && !this.destroyed) { + if (this._lastServerId === null) { + this.emitError( + PeerErrorType.Disconnected, + "Cannot reconnect: peer has no ID. Create a new Peer instead.", + ); + return; + } logger.log( `Attempting reconnection to server with ID ${this._lastServerId}`, ); this._disconnected = false; - this._initialize(this._lastServerId!); + this._initialize(this._lastServerId); } else if (this.destroyed) { throw new Error( "This peer cannot reconnect to the server. It has already been destroyed.", @@ -739,6 +758,10 @@ export class Peer extends EventEmitterWithError { this._api .listAllPeers() .then((peers) => cb(peers)) - .catch((error) => this._abort(PeerErrorType.ServerError, error)); + .catch((error) => { + if (!this.destroyed && !this.disconnected) { + this._abort(PeerErrorType.ServerError, error); + } + }); } }