From 7c307d861394744eab5a9583449b247bf5c0973f Mon Sep 17 00:00:00 2001 From: siruimei07 Date: Thu, 8 Oct 2026 18:41:33 +0800 Subject: [PATCH] feedback7 G-TOOLS: a reconnect token cannot be claimed twice by two tabs started apart (PR #299 by @siruimei07) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applied as-is. A tab that is still choosing a reconnect token now answers another tab's query for the same candidate hashes, so a tab started during the 150 ms window cannot adopt a token the earlier tab is also considering. The smaller tab id still wins, and a tab that already holds the token keeps it. Tests: node --test test/client-identity-session.test.js test/client-static.test.js — 319 pass, 0 fail. --- public/js/net.js | 13 ++- test/client-identity-session.test.js | 143 +++++++++++++++++++++++++++ test/client-static.test.js | 52 ++++++++++ 3 files changed, 204 insertions(+), 4 deletions(-) create mode 100644 test/client-identity-session.test.js diff --git a/public/js/net.js b/public/js/net.js index b921a444..5916d3bd 100644 --- a/public/js/net.js +++ b/public/js/net.js @@ -726,7 +726,7 @@ export function createIdentity(deps = {}) { let initPromise = null; let initialized = false; let current = null; // token this tab uses (null = let the server create a session) - let resolving = null; // { taken: Set } while init() waits for claims + let resolving = null; // { hashes: Set, taken: Set } while init() waits for claims const readRecent = () => { try { @@ -751,7 +751,12 @@ export function createIdentity(deps = {}) { if (m.hashes.includes(h)) post({ type: 'mine', hash: h, to: m.from }); } // Another tab is choosing at the same time: the smaller tab id has precedence. - if (resolving && m.from < tabId) for (const h of m.hashes) if (typeof h === 'string') resolving.taken.add(h); + // Answer for pending candidates too: a later channel may have missed our initial query. + if (resolving) for (const h of m.hashes) { + if (typeof h !== 'string') continue; + if (m.from < tabId) resolving.taken.add(h); + else if (resolving.hashes.has(h)) post({ type: 'mine', hash: h, to: m.from }); + } } else if (m.type === 'mine' && resolving && m.to === tabId && typeof m.hash === 'string') { resolving.taken.add(m.hash); } @@ -768,8 +773,8 @@ export function createIdentity(deps = {}) { // Without a channel we cannot tell whether a shared token is in use: never adopt one. const candidates = [...new Set([ownOk, ...(channel ? readRecent() : [])].filter(isToken))]; if (!channel || candidates.length === 0) return ownOk; - resolving = { taken: new Set() }; - post({ type: 'who', hashes: candidates.map(tokenHash) }); + resolving = { hashes: new Set(candidates.map(tokenHash)), taken: new Set() }; + post({ type: 'who', hashes: [...resolving.hashes] }); await new Promise((r) => setTimer(r, queryMs)); const { taken } = resolving; resolving = null; diff --git a/test/client-identity-session.test.js b/test/client-identity-session.test.js new file mode 100644 index 00000000..18a97e1f --- /dev/null +++ b/test/client-identity-session.test.js @@ -0,0 +1,143 @@ +// Real BroadcastChannel election followed by real client/server session handshakes. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { once } from 'node:events'; +import { BroadcastChannel } from 'node:worker_threads'; +import WebSocket from 'ws'; +import { startServer } from '../server/index.js'; +import { CLAIM_QUERY_MS, createIdentity, Net } from '../public/js/net.js'; + +function storage(seed = {}) { + const values = new Map(Object.entries(seed)); + return { + getItem: (key) => values.get(key) ?? null, + setItem: (key, value) => values.set(key, String(value)), + removeItem: (key) => values.delete(key), + }; +} + +function nextEvent(net, type) { + return new Promise((resolve, reject) => { + const off = net.on(type, (value) => { clearTimeout(timer); off(); resolve(value); }); + const timer = setTimeout(() => { off(); reject(new Error(`Timed out waiting for ${type}`)); }, 2000); + }); +} + +async function harness(t) { + const srv = await startServer({ port: 0, host: '127.0.0.1', quiet: true }); + const clients = [], channels = [], replaced = []; + const local = storage(); + const channelName = `sp.identity.test.${process.pid}.${srv.port}`; + t.after(async () => { + clients.forEach((net) => net.close()); + channels.forEach((channel) => channel.close()); + await srv.close(); + }); + return { + local, replaced, + tab(tabId, session = storage(), setTimeout) { + const channel = new BroadcastChannel(channelName); + channels.push(channel); + const id = createIdentity({ local, session, channel, tabId, setTimeout }); + return { id, session, channel }; + }, + async connect(name, tab) { + const net = new Net({ url: `ws://127.0.0.1:${srv.port}/ws`, WebSocket, getToken: () => tab?.id.getToken() ?? null }); + clients.push(net); + net.on('replaced', (error) => replaced.push({ name, code: error.code })); + net.on('welcome', (welcome) => tab?.id.saveToken(welcome.token)); + const welcome = nextEvent(net, 'welcome'); + net.setName(name); + return { net, welcome: await welcome }; + }, + }; +} + +async function closeClient(net) { + const closed = once(net.ws, 'close'); + net.close(); + await closed; +} + +async function assertOnline(h, clients) { + // A round trip from each socket also lets a queued replacement close reach the client. + const replies = await Promise.allSettled(clients.map(({ net }) => net.request('ping', { c: Date.now() }))); + assert.deepEqual(h.replaced, [], 'no client is replaced by another tab'); + for (const [index, client] of clients.entries()) { + assert.equal(client.net.status, 'online'); + assert.equal(client.net.lastError, null); + assert.equal(replies[index].status, 'fulfilled'); + assert.equal(replies[index].value.t, 'pong'); + } +} + +for (const [firstId, secondId] of [['a-tab', 'b-tab'], ['b-tab', 'a-tab']]) { + test(`staggered tabs ${firstId} then ${secondId} keep separate server sessions`, { timeout: 5000 }, async (t) => { + const h = await harness(t); + const seed = await h.connect('Seed'); + await closeClient(seed.net); + h.local.setItem('sp.tokens', JSON.stringify([seed.welcome.token])); + + const deadlines = []; + const holdDeadline = (fn, ms) => { assert.equal(ms, CLAIM_QUERY_MS); deadlines.push(fn); }; + const first = h.tab(firstId, storage(), holdDeadline); + const firstInit = first.id.init(); + // The first who has already been sent when the second channel is created. + const second = h.tab(secondId, storage(), holdDeadline); + const delivered = new Promise((resolve) => { + second.channel.addEventListener('message', (event) => { + if (event.data.type === 'test.barrier') resolve(); + }); + }); + first.channel.addEventListener('message', (event) => { + if (event.data.type === 'who') first.channel.postMessage({ type: 'test.barrier' }); + }); + const secondInit = second.id.init(); + // FIFO delivery puts this barrier after any real reply to the later who. Both default + // 150 ms deadlines remain pending, without relying on sleeps or a fake channel hub. + await delivered; + assert.equal(deadlines.length, 2); + deadlines[0](); + await firstInit; + deadlines[1](); + await secondInit; + + const a = await h.connect('First', first); + const b = await h.connect('Second', second); + await assertOnline(h, [a, b]); + assert.notEqual(a.welcome.token, b.welcome.token); + assert.notEqual(a.welcome.playerId, b.welcome.playerId); + const resumed = firstId < secondId ? a : b; + assert.equal(resumed.welcome.token, seed.welcome.token); + assert.equal(resumed.welcome.playerId, seed.welcome.playerId); + }); +} + +test('live token ownership survives a duplicated tab, reload and reconnect', { timeout: 5000 }, async (t) => { + const h = await harness(t); + const owner = h.tab('z-owner'); + await owner.id.init(); + const a = await h.connect('Owner', owner); + + // A duplicated tab copies sessionStorage, but its smaller tab ID cannot evict a live owner. + const duplicate = h.tab('a-duplicate', storage({ 'sp.token': a.welcome.token })); + await duplicate.id.init(); + const b = await h.connect('Duplicate', duplicate); + assert.notEqual(a.welcome.token, b.welcome.token); + assert.notEqual(a.welcome.playerId, b.welcome.playerId); + await assertOnline(h, [a, b]); + + await closeClient(a.net); + owner.channel.close(); + const reloaded = h.tab('z-reloaded', owner.session); + await reloaded.id.init(); + const a2 = await h.connect('Owner', reloaded); + assert.equal(a2.welcome.token, a.welcome.token); + assert.equal(a2.welcome.playerId, a.welcome.playerId); + const reconnected = nextEvent(a2.net, 'welcome'); + a2.net.reconnectNow(); + const welcome = await reconnected; + assert.equal(welcome.token, a.welcome.token); + assert.equal(welcome.playerId, a.welcome.playerId); + await assertOnline(h, [a2, b]); +}); diff --git a/test/client-static.test.js b/test/client-static.test.js index af58833d..db8c8437 100644 --- a/test/client-static.test.js +++ b/test/client-static.test.js @@ -790,6 +790,58 @@ describe('identity (reconnect-token selection across tabs)', () => { assert.equal(ty, null); }); + for (const [firstId, secondId, expected] of [ + ['a-tab', 'b-tab', ['tokShared', null]], + ['b-tab', 'a-tab', [null, 'tokShared']], + ]) { + test(`staggered channels: ${firstId} starts before ${secondId}`, async () => { + const { createIdentity, CLAIM_QUERY_MS } = await mod('net.js'); + const hub = channelHub(), timers = fakeTimers(), local = memStorage(); + local.setItem('sp.tokens', JSON.stringify(['tokShared'])); + const firstSession = memStorage(), secondSession = memStorage(); + const first = createIdentity({ local, session: firstSession, tabId: firstId, + channel: hub.create(), setTimeout: timers.setTimeout }); + const firstPick = first.init(); + timers.advance(30); + // This channel did not exist when the first tab broadcast its query. + const second = createIdentity({ local, session: secondSession, tabId: secondId, + channel: hub.create(), setTimeout: timers.setTimeout }); + const secondPick = second.init(); + await new Promise((r) => setImmediate(r)); + timers.advance(CLAIM_QUERY_MS - 30); + assert.equal(await firstPick, expected[0]); + timers.advance(30); + assert.equal(await secondPick, expected[1]); + assert.deepEqual([first.getToken(), second.getToken()], expected); + assert.deepEqual([firstSession.getItem('sp.token'), secondSession.getItem('sp.token')], expected); + }); + } + + test('staggered elections reserve fallback candidates while a live owner keeps its token', async () => { + const { createIdentity, CLAIM_QUERY_MS } = await mod('net.js'); + const hub = channelHub(), timers = fakeTimers(), local = memStorage(); + const owner = createIdentity({ local, session: memStorage(), tabId: 'z-owner', channel: hub.create() }); + await owner.init(); + owner.saveToken('tokOwned'); + local.setItem('sp.tokens', JSON.stringify(['tokOwned', 'tokFree'])); + const first = createIdentity({ local, session: memStorage(), tabId: 'a-tab', + channel: hub.create(), setTimeout: timers.setTimeout }); + const firstPick = first.init(); + timers.advance(30); + const duplicateSession = memStorage(); + duplicateSession.setItem('sp.token', 'tokOwned'); + const second = createIdentity({ local, session: duplicateSession, tabId: 'b-tab', + channel: hub.create(), setTimeout: timers.setTimeout }); + const secondPick = second.init(); + await new Promise((r) => setImmediate(r)); + timers.advance(CLAIM_QUERY_MS - 30); + assert.equal(await firstPick, 'tokFree'); + timers.advance(30); + assert.equal(await secondPick, null); + assert.equal(duplicateSession.getItem('sp.token'), null, 'the copied live token is discarded'); + assert.equal(owner.getToken(), 'tokOwned', 'a live owner outranks even a smaller tab id'); + }); + test('without BroadcastChannel only the own token is used; init is idempotent; welcome during init wins', async () => { const { createIdentity } = await mod('net.js'); const local = memStorage();