Repository navigation
Cap each WebSocket action frame at one legal action - #39
Open
JacobStephens2 wants to merge 1 commit into
Open
JacobStephens2 wants to merge 1 commit into
JacobStephens2 wants to merge 1 commit into
Conversation
A seated player's socket accepted frames up to the ws default of 100 MiB and parsed them in the message handler. The server now sets maxPayload to the compact JSON size of the longest action the engine can mint: PLAY_OPEN with two follow-<safe integer> instance ids and a safe-integer sequence (181 bytes). ws closes a larger frame with 1009 before it emits message, so trailing whitespace after a CONFIRM_HAND does not apply that action. A compact CONFIRM_HAND still applies and is broadcast. The payload error is ignored so the close does not exit the process. The socket wiring now lives in packages/server/src/sockets.ts. Tests: packages/server/tests/action-frame.test.ts.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A seated player's socket accepted frames up to the
ws8.21.1 default of 100 MiB and parsed them in the message handler.attachGameSocketsnow setsmaxPayloadto the compact JSON size of the longest action the engine can mint:PLAY_OPENwith twofollow-<safe integer>instance ids and a safe-integer sequence.Seam: the authenticated
/wsseat socket (packages/server/src/sockets.ts,attachGameSockets). One action message per frame. The room either applies it and broadcasts a snapshot, orwscloses a larger frame before the handler runs.Evidence
new WebSocketServer({ noServer: true })inpackages/server/src/index.tsleftmaxPayloadat 104857600. Themessagehandler ranJSON.parseon the frame that arrived.After:
npx vitest run packages/server/tests/action-frame.test.tspassed (2 tests, 96ms). A compactCONFIRM_HANDmoves the room sequence from 0 to 1 and is broadcast. The same action padded toACTION_FRAME_MAX_BYTES + 1closes with 1009, sends no further message, and leaves the sequence at 0.Merge Danger
Door: two-way
Revert the commit and the socket returns to the 100 MiB default. No data migration.
Blast Radius: online seat sockets
One oversized frame closes that socket. A compact legal action still applies. The
WS_ERR_UNSUPPORTED_MESSAGE_LENGTHerror is ignored so the close does not exit the process. Any other socket error is still thrown.Unaddressed findings
Standards
Judgement call - Duplicated Code (
packages/server/src/sockets.tslines 10-22,largestInstanceIdandACTION_FRAME_MAX_BYTES). The reviewer says the cap re-encodes the engine instance-id shape`${role}-${nextInstance}`and thePLAY_OPENfield list, and that a longer action would not fail the build.Refuted by what those lines are: a byte length for
maxPayload, not a second mint and not a secondPLAY_OPEN. Instance ids are still minted only inpackages/engine/src/game.ts(makeInstancesandnewOwnedInstance,`${role}-${nextInstance++}`).GameActionSchemainpackages/engine/src/schemas.tsleaves those strings unbounded. Issue 38 asks for a frame cap that still accepts one legal action, which is this measurement of the longest compact message the engine can mint. The engine should not own WebSocket frame bytes, so there is no shared helper.The finding does not say the behaviour is wrong and does not include a failing test.
Spec
None.
Security
None.
Security review outcome
Review coverage
Both axes read every changed file:
packages/server/src/index.ts,packages/server/src/sockets.ts, andpackages/server/tests/action-frame.test.ts. No changed file was left unread.Closes #38
Built with grok · grok-4.7 · xhigh