Skip to content

Commit 75544e3

Browse files
committed
fix(files): keep acknowledgements responsive and fence admission
1 parent 0ca4588 commit 75544e3

7 files changed

Lines changed: 171 additions & 14 deletions

File tree

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

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ interface Backing {
3939
maxReadStreams: number
4040
maxReadCount: number
4141
onSnapshot?: () => Promise<void>
42+
onLength?: () => Promise<void>
4243
}
4344

4445
const state = vi.hoisted(() => ({ backing: null as Backing | null }))
@@ -93,7 +94,10 @@ function makeClient(): any {
9394
.reverse()
9495
.slice(0, options?.COUNT)
9596
.map((entry) => ({ ...entry })),
96-
xLen: async (key: string) => (b().streams.get(key) ?? []).length,
97+
xLen: async (key: string) => {
98+
await b().onLength?.()
99+
return (b().streams.get(key) ?? []).length
100+
},
97101
xTrim: async (key: string, _strategy: string, minid: string) => {
98102
const arr = b().streams.get(key) ?? []
99103
b().streams.set(
@@ -1131,6 +1135,46 @@ describe('FileDocStore', () => {
11311135
})
11321136
})
11331137

1138+
it.each(['client', 'server'] as const)(
1139+
'does not delay a durable %s append for pending compaction',
1140+
async (publisher) => {
1141+
const store = await newStore()
1142+
await store.seedIfEmpty(NAME, seedFor('base'))
1143+
const doc = new Y.Doc()
1144+
await store.attachRoom(NAME, doc)
1145+
const room = storeInternals(store).rooms.get(NAME)!
1146+
room.publishes = 63
1147+
let finishCompaction!: () => void
1148+
const compaction = new Promise<void>((resolve) => {
1149+
finishCompaction = resolve
1150+
})
1151+
state.backing!.onLength = vi.fn(() => compaction)
1152+
const publish = (id: string) =>
1153+
publisher === 'client'
1154+
? store.publishClientUpdateAndWait(NAME, id, updateFor(id), 'doc-base')
1155+
: store.publishAndWait(NAME, updateFor(id), 'doc-base')
1156+
let accepted = false
1157+
const pending = publish('first').then(() => {
1158+
accepted = true
1159+
})
1160+
try {
1161+
await vi.waitFor(() => expect(accepted).toBe(true))
1162+
expect(room.compacting).toBe(true)
1163+
expect(state.backing!.streams.get(`filedoc:stream:${NAME}`)).toHaveLength(2)
1164+
room.publishes = 127
1165+
await publish('second')
1166+
expect(state.backing!.onLength).toHaveBeenCalledOnce()
1167+
expect(state.backing!.streams.get(`filedoc:stream:${NAME}`)).toHaveLength(3)
1168+
} finally {
1169+
finishCompaction()
1170+
await pending
1171+
await vi.waitFor(() => expect(room.compacting).toBe(false))
1172+
store.detachRoom(NAME)
1173+
doc.destroy()
1174+
}
1175+
}
1176+
)
1177+
11341178
it('compacts on retained bytes before the entry-count threshold can exhaust replay', async () => {
11351179
const streamKey = `filedoc:stream:${NAME}`
11361180
const snapshot = Buffer.from(Y.encodeStateAsUpdate(new Y.Doc())).toString('base64')

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ export class FileDocStore {
513513
room.uncompactedDeltaBytes >= COMPACT_ENCODED_BYTES ||
514514
room.publishes % COMPACT_CHECK_EVERY === 0
515515
) {
516-
await this.maybeCompact(name)
516+
void this.maybeCompact(name)
517517
}
518518
}
519519
}
@@ -595,7 +595,7 @@ export class FileDocStore {
595595
room.uncompactedDeltaBytes >= COMPACT_ENCODED_BYTES ||
596596
room.publishes % COMPACT_CHECK_EVERY === 0
597597
) {
598-
await this.maybeCompact(name)
598+
void this.maybeCompact(name)
599599
}
600600
}
601601
return

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

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ import {
4545
} from '@/handlers/file-doc'
4646
import { FileDocInvalidatedError, getFileDocStore } from '@/handlers/file-doc-store'
4747
import * as permissions from '@/middleware/permissions'
48-
import { beginRoomPermissionRead, commitRoomPermission } from '@/middleware/permissions'
48+
import {
49+
beginRoomPermissionRead,
50+
commitRoomPermission,
51+
ROLE_REVALIDATION_TTL_MS,
52+
} from '@/middleware/permissions'
4953

5054
type Handler = (...payload: unknown[]) => Promise<void> | void
5155

@@ -1146,6 +1150,85 @@ describe('setupWorkspaceFileDocHandlers', () => {
11461150
}
11471151
)
11481152

1153+
it('rejects a generation invalidated while final authorization is pending', async () => {
1154+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Old', 'doc-old'))
1155+
const { io } = createIo()
1156+
const pending = setup('socket-final-authorization-invalidation', io)
1157+
let finishAuthorization!: (permission: 'write') => void
1158+
const authorization = new Promise<'write'>((resolve) => {
1159+
finishAuthorization = resolve
1160+
})
1161+
const guard = vi
1162+
.spyOn(permissions, 'resolveCurrentRoomPermission')
1163+
.mockResolvedValueOnce('write')
1164+
.mockImplementationOnce(() => authorization)
1165+
const joining = pending.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
1166+
try {
1167+
await vi.waitFor(() => expect(guard).toHaveBeenCalledTimes(2))
1168+
await getFileDocStore().invalidateDocument(ROOM_NAME, 2)
1169+
finishAuthorization('write')
1170+
await joining
1171+
expect(pending.socket.join).not.toHaveBeenCalledWith(ROOM_NAME)
1172+
expect(joinSuccessFileId(pending.socket)).toBeUndefined()
1173+
expect(pending.socket.emit).toHaveBeenCalledWith(
1174+
FILE_DOC_EVENTS.JOIN_ERROR,
1175+
expect.objectContaining({ code: 'JOIN_FAILED', retryable: true })
1176+
)
1177+
expect(pending.socket.leave).toHaveBeenCalledWith(fileDocAdmissionRoom('file-1'))
1178+
} finally {
1179+
finishAuthorization('write')
1180+
await joining
1181+
guard.mockRestore()
1182+
}
1183+
})
1184+
1185+
it.each(['revoked', 'expired'] as const)(
1186+
'rejects access %s while the final generation check is pending',
1187+
async (access) => {
1188+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Private', 'doc-private'))
1189+
const { io } = createIo()
1190+
const pending = setup('socket-generation-authorization-revocation', io)
1191+
let finishGeneration!: (current: boolean) => void
1192+
const generation = new Promise<boolean>((resolve) => {
1193+
finishGeneration = resolve
1194+
})
1195+
const guard = vi
1196+
.spyOn(getFileDocStore(), 'isDocumentGenerationCurrent')
1197+
.mockImplementationOnce(() => generation)
1198+
const joining = pending.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
1199+
const clock = vi.spyOn(Date, 'now')
1200+
try {
1201+
await vi.waitFor(() => expect(guard).toHaveBeenCalledOnce())
1202+
if (access === 'revoked') {
1203+
commitRoomPermission(
1204+
'user-1',
1205+
{ type: ROOM_TYPES.WORKSPACE_FILE_DOC, id: 'file-1' },
1206+
'read',
1207+
beginRoomPermissionRead()
1208+
)
1209+
} else {
1210+
clock.mockReturnValue(Date.now() + ROLE_REVALIDATION_TTL_MS + 1)
1211+
}
1212+
finishGeneration(true)
1213+
await joining
1214+
expect(pending.socket.join).not.toHaveBeenCalledWith(ROOM_NAME)
1215+
expect(joinSuccessFileId(pending.socket)).toBeUndefined()
1216+
expect(pending.socket.emit).toHaveBeenCalledWith(
1217+
FILE_DOC_EVENTS.JOIN_ERROR,
1218+
expect.objectContaining({
1219+
code: access === 'revoked' ? 'ACCESS_DENIED' : 'JOIN_FAILED',
1220+
retryable: access === 'expired',
1221+
})
1222+
)
1223+
} finally {
1224+
clock.mockRestore()
1225+
finishGeneration(true)
1226+
await joining
1227+
guard.mockRestore()
1228+
}
1229+
}
1230+
)
1231+
11491232
it('receives invalidation while the subscribed generation check is pending', async () => {
11501233
mockFetchFileDocSeed.mockResolvedValue(seedResult('# Old', 'doc-old'))
11511234
const memberships = new Set<string>()

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

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1476,6 +1476,12 @@ export function setupWorkspaceFileDocHandlers(
14761476
await socket.join(admissionName)
14771477
const joinedVersion =
14781478
Math.max(entry.syncedVersion ?? 0, (await store.getSyncedVersion(name)) ?? 0) || undefined
1479+
/** Adapter membership can wait; resolve access again before checking the final generation. */
1480+
const finalPermission = await resolveCurrentRoomPermission(userId, room, FILE_DOC_ACTION)
1481+
if (!satisfiesRoomMembership(finalPermission, ROOM_TYPES.WORKSPACE_FILE_DOC)) {
1482+
emitJoinError(socket, fileId, clientId, 'Access denied to file', 'ACCESS_DENIED', false)
1483+
return
1484+
}
14791485
const currentDocument = await store.isDocumentGenerationCurrent(name, docIdOf(entry.doc))
14801486
if (!isCurrentJoin()) return
14811487
if (!currentDocument) {
@@ -1489,15 +1495,19 @@ export function setupWorkspaceFileDocHandlers(
14891495
)
14901496
return
14911497
}
1492-
/** Adapter membership and generation reads can wait; resolve access again before commit. */
1493-
const finalPermission = await resolveCurrentRoomPermission(userId, room, FILE_DOC_ACTION)
1494-
if (!satisfiesRoomMembership(finalPermission, ROOM_TYPES.WORKSPACE_FILE_DOC)) {
1495-
emitJoinError(socket, fileId, clientId, 'Access denied to file', 'ACCESS_DENIED', false)
1498+
/** The generation read may wait; a revoked or expired access decision must not admit content. */
1499+
const membershipPermission = peekRoomPermission(userId, room)
1500+
if (!satisfiesRoomMembership(membershipPermission ?? null, ROOM_TYPES.WORKSPACE_FILE_DOC)) {
1501+
emitJoinError(
1502+
socket,
1503+
fileId,
1504+
clientId,
1505+
'File access changed while joining',
1506+
membershipPermission === undefined ? 'JOIN_FAILED' : 'ACCESS_DENIED',
1507+
membershipPermission === undefined
1508+
)
14961509
return
14971510
}
1498-
1499-
/** Commit content membership only after the final authorization decision. */
1500-
if (!isCurrentJoin()) return
15011511
await socket.join(name)
15021512
/** An asynchronous adapter join can be superseded by a leave, switch, or disconnect. */
15031513
if (!isCurrentJoin()) return

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,9 @@ describe('FileDocProvider', () => {
570570
await vi.advanceTimersByTimeAsync(UPDATE_BATCH_TEST_WINDOW_MS)
571571
expect(save).toHaveBeenCalledTimes(2)
572572
const recovered = new Y.Doc()
573-
Y.applyUpdate(recovered, save.mock.calls[1][2])
573+
Y.applyUpdate(recovered, save.mock.calls[0][2])
574+
expect(recovered.getText('default').toString()).toBe('first')
575+
Y.applyUpdate(recovered, save.mock.calls[1][1])
574576
expect(recovered.getText('default').toString()).toBe('first second')
575577
recovered.destroy()
576578
await vi.advanceTimersByTimeAsync(1_000)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip-safety.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,18 @@ describe('isRoundTripSafe', () => {
245245
expect(normalizeMarkdownContent(source)).toBe(source)
246246
})
247247

248+
it.each([
249+
'<img src="/image" width>',
250+
'<img src="/image" height>',
251+
'<img src="/image" width="">',
252+
"<img src='/image' height=''>",
253+
'<img WIDTH src="/image" height="20">',
254+
'[<img src="/image" width height>](/link)',
255+
])('keeps valueless image dimensions in source mode: %s', (source) => {
256+
expect(isRoundTripSafe(source)).toBe(false)
257+
expect(normalizeMarkdownContent(source)).toBe(source)
258+
})
259+
248260
it('allows supported image attributes containing quoted angle brackets', () => {
249261
expect(isRoundTripSafe('<img src="/image" title="a>b" width="30">')).toBe(true)
250262
expect(isRoundTripSafe('[<img src="/image" alt="a>b" width="30">](/link)')).toBe(true)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/round-trip-safety.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,16 @@ function inspectHtmlImages(content: string) {
6161
quotedEntities += tag.raw.match(/&quot;/g)?.length ?? 0
6262
const attributes = tag.raw.slice(4, -1)
6363
const seen = new Set<string>()
64-
const pattern = /(?:^|\s)([^\s=/>]+)(?:\s*=\s*(?:"[^"]*"|'[^']*'|[^\s"'=<>`]+))?/g
64+
const pattern = /(?:^|\s)([^\s=/>]+)(?:\s*=\s*("[^"]*"|'[^']*'|[^\s"'=<>`]+))?/g
6565
for (const attribute of attributes.matchAll(pattern)) {
6666
const name = attribute[1].toLowerCase()
67-
if (!SUPPORTED_IMAGE_ATTRIBUTES.has(name) || seen.has(name)) {
67+
const value = attribute[2]
68+
if (
69+
!SUPPORTED_IMAGE_ATTRIBUTES.has(name) ||
70+
seen.has(name) ||
71+
value === undefined ||
72+
((name === 'width' || name === 'height') && (value === '""' || value === "''"))
73+
) {
6874
images.set(tag.raw, (images.get(tag.raw) ?? 0) + 1)
6975
break
7076
}

0 commit comments

Comments
 (0)