rook/val Public Public: anyone can read this; nobody can change it.
forked from benjivers/val
workbenchthis machine
Fix a stale socket close evicting a live reconnect's seat
A page reload opens the new websocket before the old one's close event lands. release() keyed only on session, so the old close would delete whatever the session's current seat entry was (the new socket's) and, finding nobody else holding the player, flip a connected human back to a bot — command() then silently drops that player's input. attach/ release now carry a per-socket token so a stale close only releases the socket it belongs to. Added a regression test in server.test.ts that reproduces the race directly against attach/release; confirmed it fails on the old code and passes with the fix. Co-Authored-By: Claude Sonnet 5 <[email protected]>
3 files
changed +29
−11
src/arena.ts +12 −6
| @@ -31,7 +31,7 @@ import { isWeaponId, WEAPONS } from "./weapons.ts"; | ||
| 31 | 31 | export const TICK = 1 / 30; |
| 32 | 32 | const BROADCAST_EVERY = 2; |
| 33 | 33 | |
| 34 | export type Seat = { playerId: string; send: (data: string) => void }; | |
| 34 | export type Seat = { playerId: string; token: string; send: (data: string) => void }; | |
| 35 | 35 | |
| 36 | 36 | let state: MatchState = newMatch(); |
| 37 | 37 | const seats = new Map<string, Seat>(); |
| @@ -86,15 +86,21 @@ export function takeSeat(session: string, name: string): Player | null { | ||
| 86 | 86 | return seat; |
| 87 | 87 | } |
| 88 | 88 | |
| 89 | export function attach(session: string, playerId: string, send: (data: string) => void): void { | |
| 90 | seats.set(session, { playerId, send }); | |
| 89 | export function attach(session: string, playerId: string, token: string, send: (data: string) => void): void { | |
| 90 | seats.set(session, { playerId, token, send }); | |
| 91 | 91 | } |
| 92 | 92 | |
| 93 | /** A person leaving gives the seat back to a bot, mid-round and all. */ | |
| 94 | export function release(session: string): void { | |
| 93 | /** | |
| 94 | * A person leaving gives the seat back to a bot, mid-round and all — but only | |
| 95 | * if this is still the socket the session is on. A reload opens the new | |
| 96 | * socket before the old one's close event arrives; without the token, that | |
| 97 | * late close would delete the reconnect's seat entry and hand a live human's | |
| 98 | * seat to a bot mid-round. | |
| 99 | */ | |
| 100 | export function release(session: string, token: string): void { | |
| 95 | 101 | const seat = seats.get(session); |
| 102 | if (!seat || seat.token !== token) return; | |
| 96 | 103 | seats.delete(session); |
| 97 | if (!seat) return; | |
| 98 | 104 | const player = findPlayer(seat.playerId); |
| 99 | 105 | if (player && !stillHeld(seat.playerId)) { |
| 100 | 106 | player.bot = true; |
src/server.test.ts +11
| @@ -207,6 +207,17 @@ test("the page carries the first-run card and the prompt line", async () => { | ||
| 207 | 207 | expect(body).toContain('id="viewtoggle"'); |
| 208 | 208 | }); |
| 209 | 209 | |
| 210 | test("a stale socket closing does not evict a live reconnect's seat", async () => { | |
| 211 | // A reload opens the new socket before the old one's close event lands. | |
| 212 | // The old close must not turn a player who is still connected into a bot. | |
| 213 | const { attach, release } = await import("./arena.ts"); | |
| 214 | const player = takeSeat("reload-session", "Reload")!; | |
| 215 | attach("reload-session", player.id, "first", () => {}); | |
| 216 | attach("reload-session", player.id, "second", () => {}); | |
| 217 | release("reload-session", "first"); | |
| 218 | expect(player.bot).toBe(false); | |
| 219 | }); | |
| 220 | ||
| 210 | 221 | test("two people who pick the same name do not end up with the same name", () => { |
| 211 | 222 | // Everything that addresses a player — the kill feed, the round log, the |
| 212 | 223 | // scoreboard — does it by name, so the match may never hold two of one. |
src/server.ts +6 −5
| @@ -145,7 +145,7 @@ const cleanName = (raw: unknown): string => { | ||
| 145 | 145 | const setCookie = (name: string, value: string, httpOnly: boolean): string => |
| 146 | 146 | `${name}=${encodeURIComponent(value)}; Path=/; SameSite=Lax; Max-Age=604800${httpOnly ? "; HttpOnly" : ""}${publicOrigin.startsWith("https:") ? "; Secure" : ""}`; |
| 147 | 147 | |
| 148 | type SocketData = { session: string; playerId: string }; | |
| 148 | type SocketData = { session: string; playerId: string; token: string }; | |
| 149 | 149 | |
| 150 | 150 | // ---------------------------------------------------------------- routing |
| 151 | 151 | |
| @@ -180,7 +180,8 @@ export const server = Bun.serve<SocketData, Record<string, never>>({ | ||
| 180 | 180 | const name = cleanName(jar[NAME]) || `Guest-${session.slice(0, 4)}`; |
| 181 | 181 | const player = takeSeat(session, name); |
| 182 | 182 | if (!player) return new Response("no seats", { status: 503, headers: SECURITY }); |
| 183 | const upgraded = self.upgrade(request, { data: { session, playerId: player.id } }); | |
| 183 | const token = randomUUID(); | |
| 184 | const upgraded = self.upgrade(request, { data: { session, playerId: player.id, token } }); | |
| 184 | 185 | return upgraded ? undefined : new Response("expected a websocket", { status: 426, headers: SECURITY }); |
| 185 | 186 | } |
| 186 | 187 | |
| @@ -264,8 +265,8 @@ export const server = Bun.serve<SocketData, Record<string, never>>({ | ||
| 264 | 265 | websocket: { |
| 265 | 266 | maxPayloadLength: 4 * 1024, |
| 266 | 267 | open(socket) { |
| 267 | const { session, playerId } = socket.data; | |
| 268 | attach(session, playerId, (data) => socket.send(data)); | |
| 268 | const { session, playerId, token } = socket.data; | |
| 269 | attach(session, playerId, token, (data) => socket.send(data)); | |
| 269 | 270 | socket.send(snapshot(playerId)); |
| 270 | 271 | }, |
| 271 | 272 | message(socket, raw) { |
| @@ -282,7 +283,7 @@ export const server = Bun.serve<SocketData, Record<string, never>>({ | ||
| 282 | 283 | command(socket.data.playerId, parsed as Command); |
| 283 | 284 | }, |
| 284 | 285 | close(socket) { |
| 285 | release(socket.data.session); | |
| 286 | release(socket.data.session, socket.data.token); | |
| 286 | 287 | }, |
| 287 | 288 | }, |
| 288 | 289 |