Skip to content

Commit f3ecfa6

Browse files
committed
fix(file-editor): cover provider protocol edge cases
1 parent cf58ead commit f3ecfa6

4 files changed

Lines changed: 33 additions & 22 deletions

File tree

apps/realtime/src/handlers/file-doc.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,12 +233,13 @@ describe('setupWorkspaceFileDocHandlers', () => {
233233
)
234234
})
235235

236-
it('rejects a payload missing the file id or client id before authorizing', async () => {
236+
it('rejects a payload with a missing or out-of-range client id before authorizing', async () => {
237237
const { io } = createIo()
238238
const { socket, handlers } = setup('socket-1', io)
239239

240240
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: '', clientId: 1 })
241241
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1' })
242+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 0x1_0000_0000 })
242243

243244
expect(socket.emit).toHaveBeenCalledWith(
244245
FILE_DOC_EVENTS.JOIN_ERROR,

apps/realtime/src/handlers/file-doc.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,19 @@ const fileDocRooms = new Map<string, FileDocRoom>()
192192
/** socketId → its current file-doc room name (a socket edits at most one doc). */
193193
const socketToRoomName = new Map<string, string>()
194194
/**
195-
* socketId → a monotonic join generation. A JOIN bumps it on arrival and, after
196-
* the async authorization, proceeds only if the generation is still its own — so
197-
* a newer JOIN (a fast document switch) or a disconnect (which drops the entry in
198-
* cleanup) that occurred during authorization aborts the now-stale JOIN. Without
199-
* this, an out-of-order authorize completion could bind the socket to the wrong
200-
* document, or a disconnect-during-authorize could register a dead socket and
201-
* leak its room.
195+
* socketId → a monotonic file-intent generation. Switching files or leaving the
196+
* intended file advances it; co-mounted providers joining the same file share it.
197+
* After async authorization, a join proceeds only while its generation is current,
198+
* preventing an out-of-order completion from binding the socket to the wrong file.
202199
*/
203200
const joinGeneration = new Map<string, number>()
201+
const MAX_YJS_CLIENT_ID = 0xffff_ffff
202+
203+
function isYjsClientId(value: unknown): value is number {
204+
return (
205+
typeof value === 'number' && Number.isInteger(value) && value >= 0 && value <= MAX_YJS_CLIENT_ID
206+
)
207+
}
204208

205209
interface AwarenessChange {
206210
added: number[]
@@ -227,10 +231,8 @@ function originSocketId(origin: unknown): string | null {
227231
* The transaction origin stamped on an agent-streamed frame (a {@link FILE_DOC_MESSAGE_TYPE.SYNC_NO_PERSIST}
228232
* apply). A non-string sentinel, so `originSocketId` returns `null` for it and the update never triggers
229233
* `edited`/`schedulePersist` (the copilot's final `edit_content` write is the durable persist). Unlike a
230-
* client edit, an agent frame is broadcast to the WHOLE room (its originating socket is NOT excluded), so a
231-
* second {@link FileDocProvider} on the same socket — e.g. the chat preview alongside the Files editor —
232-
* also receives the mid-stream ops. The emitting provider no-ops on its own echo (the ops are already
233-
* applied locally), so broadcasting back to the sender is harmless.
234+
* client edit, it is marked so peers do not treat it as a durable user edit. The emitting provider no-ops
235+
* on its own echo because the operations are already applied locally.
234236
*/
235237
const AGENT_SYNC_ORIGIN = Symbol('file-doc-agent-sync')
236238

@@ -897,10 +899,7 @@ function emitJoinError(
897899
code: string,
898900
retryable: boolean
899901
) {
900-
const normalizedClientId =
901-
typeof clientId === 'number' && Number.isInteger(clientId) && clientId >= 0
902-
? clientId
903-
: undefined
902+
const normalizedClientId = isYjsClientId(clientId) ? clientId : undefined
904903
socket.emit(FILE_DOC_EVENTS.JOIN_ERROR, {
905904
fileId: typeof fileId === 'string' ? fileId : '',
906905
clientId: normalizedClientId,
@@ -1137,10 +1136,8 @@ export function setupWorkspaceFileDocHandlers(
11371136
if (
11381137
typeof fileId !== 'string' ||
11391138
fileId.length === 0 ||
1140-
// A Yjs clientID is a uint32; reject NaN/Infinity/negative/non-integer so a malformed id
1141-
// can't become a bogus ownership key.
1142-
!Number.isInteger(clientId) ||
1143-
clientId < 0
1139+
// A Yjs clientID is a uint32; reject malformed values before they can become ownership keys.
1140+
!isYjsClientId(clientId)
11441141
) {
11451142
emitJoinError(socket, fileId, clientId, 'Invalid join payload', 'INVALID_PAYLOAD', false)
11461143
return

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ describe('FileDocProvider', () => {
572572
// Two surfaces in one tab (Files editor + embedded chat panel) share one socket and both open the
573573
// same file. Tearing the first down must NOT strand the second — the server drops the socket from
574574
// the room on any LEAVE, so LEAVE may fire only when the last provider goes away.
575-
const { socket, emit } = createSocket(true)
575+
const { socket, emit, fire } = createSocket(true)
576576
const docA = new Y.Doc()
577577
const docB = new Y.Doc()
578578
const first = new FileDocProvider(
@@ -587,9 +587,20 @@ describe('FileDocProvider', () => {
587587
docB,
588588
new awarenessProtocol.Awareness(docB)
589589
)
590+
fire(FILE_DOC_EVENTS.JOIN_SUCCESS, {
591+
fileId: 'shared-file',
592+
clientId: docA.clientID,
593+
})
594+
fire(FILE_DOC_EVENTS.JOIN_SUCCESS, {
595+
fileId: 'shared-file',
596+
clientId: docB.clientID,
597+
})
590598
emit.mockClear()
591599

592600
first.destroy()
601+
expect(
602+
emittedMessages(emit).some((message) => message[0] === FILE_DOC_MESSAGE_TYPE.AWARENESS)
603+
).toBe(true)
593604
expect(emit).not.toHaveBeenCalledWith(FILE_DOC_EVENTS.LEAVE, expect.anything())
594605

595606
second.destroy()

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,12 @@ export class FileDocProvider extends ObservableV2<FileDocProviderEvents> {
485485
this.disposed = true
486486
this.clearReadinessTimer()
487487
this.clearJoinRetryTimer()
488-
this.joinAccepted = false
489488
this.joinPending = false
490489

490+
// Publish our final awareness removal while this provider is still admitted. A co-mounted sibling
491+
// keeps the socket in the room, so LEAVE cannot clear this provider's caret on its behalf.
491492
awarenessProtocol.removeAwarenessStates(this.awareness, [this.doc.clientID], 'provider-destroy')
493+
this.joinAccepted = false
492494

493495
// Only actually leave the room when this was the last provider for the file on the shared socket —
494496
// otherwise a sibling surface (e.g. the Files editor vs. the embedded chat panel) would be stranded.

0 commit comments

Comments
 (0)