Skip to content

Commit 24d8375

Browse files
waleedlatif1waleedlatif1
committed
fix(public-shares): stream shared files with byte ranges instead of buffering them under 100 MB
1 parent ac604b7 commit 24d8375

4 files changed

Lines changed: 275 additions & 65 deletions

File tree

‎apps/sim/app/api/files/public/[token]/content/route.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import {
1717
} from '@/lib/public-shares/application'
1818
import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit'
1919
import { FILE_CACHE_CONTROL } from '@/lib/uploads/server/delivery'
20-
import { createConditionalFileResponse } from '@/app/api/files/utils'
20+
import {
21+
createConditionalFileResponse,
22+
createStoredFileStreamResponse,
23+
} from '@/app/api/files/utils'
2124

2225
export const dynamic = 'force-dynamic'
2326
const logger = createLogger('PublicFileContentAPI')
@@ -49,6 +52,12 @@ export const GET = withRouteHandler(
4952
preview: parsed.data.query.preview === '1',
5053
request,
5154
})
55+
if (result.kind === 'stream') {
56+
return await createStoredFileStreamResponse(
57+
{ ...result, cacheControl: FILE_CACHE_CONTROL.revalidate },
58+
request.headers
59+
)
60+
}
5261
return createConditionalFileResponse(
5362
{
5463
buffer: result.buffer,

‎apps/sim/hooks/use-file-content-source.tsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,17 @@ export function createPublicFileContentSource(
146146
contentUrl: string,
147147
owner?: EditableFileOwner
148148
): FileContentSource {
149-
return inlineImageSource(
150-
(_key, opts) =>
151-
opts?.preview ? `${contentUrl}${contentUrl.includes('?') ? '&' : '?'}preview=1` : contentUrl,
152-
`/api/files/public/${token}/inline`,
153-
owner
154-
)
149+
return {
150+
...inlineImageSource(
151+
(_key, opts) =>
152+
opts?.preview
153+
? `${contentUrl}${contentUrl.includes('?') ? '&' : '?'}preview=1`
154+
: contentUrl,
155+
`/api/files/public/${token}/inline`,
156+
owner
157+
),
158+
servesByteRanges: true,
159+
}
155160
}
156161

157162
/**

‎apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts‎

Lines changed: 128 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { mkdtempSync } from 'node:fs'
1+
import { existsSync, mkdtempSync } from 'node:fs'
22
import { mkdir, rm, writeFile } from 'node:fs/promises'
33
import { tmpdir } from 'node:os'
44
import { dirname, join } from 'node:path'
@@ -39,8 +39,11 @@ import {
3939
createProjectFile,
4040
updateProjectFileContent,
4141
} from '@/lib/projects/files/application/content'
42+
import type { readPublicFileShareContent } from '@/lib/public-shares/application'
4243
import * as shareManager from '@/lib/public-shares/manager'
4344
import * as storage from '@/lib/uploads/core/storage-service'
45+
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
46+
import { sanitizeFileKey } from '@/lib/uploads/utils/file'
4447

4548
vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
4649

@@ -129,6 +132,16 @@ async function fixture() {
129132
return { ...ids, principal: createSessionPrincipal({ userId: editorId }) }
130133
}
131134

135+
type PublicShareContent = Awaited<ReturnType<typeof readPublicFileShareContent>>
136+
137+
/** The bytes a public read delivers, whether streamed from storage or built whole. */
138+
async function sharedBytes(read: PublicShareContent | Promise<PublicShareContent>) {
139+
const result = await read
140+
return result.kind === 'stream'
141+
? Buffer.from(await new Response(await result.open(undefined)).arrayBuffer())
142+
: result.buffer
143+
}
144+
132145
function createInput(projectId: string, content = 'Project architecture') {
133146
return {
134147
projectId,
@@ -410,9 +423,9 @@ describe('Public file shares against PostgreSQL and private storage', () => {
410423
credential: { method: 'POST', password: 'correct-password' },
411424
})
412425
if (!accepted.authorized) throw new Error('Correct share password rejected')
413-
expect((await readPublicFileShareContent({ grant: accepted.grant })).buffer.toString()).toBe(
414-
'Shared architecture'
415-
)
426+
expect(
427+
(await sharedBytes(readPublicFileShareContent({ grant: accepted.grant }))).toString()
428+
).toBe('Shared architecture')
416429
const [passwordShare] = await db
417430
.select()
418431
.from(publicShare)
@@ -653,6 +666,74 @@ describe('Public file shares against PostgreSQL and private storage', () => {
653666
}
654667
)
655668

669+
check(
670+
'public share streams stored bytes past the buffered ceiling and refuses once the share is disabled',
671+
async () => {
672+
const { updateProjectFileShare } = await import('@/lib/projects/files/application/shares')
673+
const { authorizePublicFileShare, readPublicFileShareContent } = await import(
674+
'@/lib/public-shares/application'
675+
)
676+
const f = await fixture()
677+
const share = async (name: string, contentType: string) => {
678+
const { file } = await createProjectFile.execute({
679+
principal: f.principal,
680+
input: { ...createInput(f.projectId, 'placeholder'), name, contentType },
681+
})
682+
const { share } = await updateProjectFileShare.execute({
683+
principal: f.principal,
684+
input: { projectId: f.projectId, fileId: file.id, isActive: true },
685+
})
686+
const access = await authorizePublicFileShare({
687+
token: share.token,
688+
credential: { method: 'GET' },
689+
})
690+
if (!access.authorized) throw new Error(`Public ${name} fixture rejected`)
691+
return { file, grant: access.grant }
692+
}
693+
694+
const recording = await share('recording.mp4', 'video/mp4')
695+
const size = MAX_BUFFERED_TRANSFER_BYTES + 1024
696+
const bytes = Buffer.alloc(size)
697+
bytes.write('tail', size - 4)
698+
const [row] = await db
699+
.select()
700+
.from(workspaceFiles)
701+
.where(eq(workspaceFiles.id, recording.file.id))
702+
const objectPath = join(localStorageRoot, sanitizeFileKey(row.key))
703+
expect(existsSync(objectPath)).toBe(true)
704+
await writeFile(objectPath, bytes)
705+
await db
706+
.update(workspaceFiles)
707+
.set({ sizeBytes: size })
708+
.where(eq(workspaceFiles.id, recording.file.id))
709+
710+
const stored = await readPublicFileShareContent({ grant: recording.grant })
711+
if (stored.kind !== 'stream') throw new Error('A stored recording was buffered')
712+
expect(stored.size).toBe(size)
713+
const whole = Buffer.from(await new Response(await stored.open(undefined)).arrayBuffer())
714+
expect(whole.length).toBe(size)
715+
expect(whole.subarray(size - 4).toString()).toBe('tail')
716+
const span = await stored.open({ start: size - 4, end: size - 1 })
717+
expect(Buffer.from(await new Response(span).arrayBuffer()).toString()).toBe('tail')
718+
719+
await updateProjectFileShare.execute({
720+
principal: f.principal,
721+
input: { projectId: f.projectId, fileId: recording.file.id, isActive: false },
722+
})
723+
await expect(stored.open({ start: 0, end: 3 })).rejects.toMatchObject({ code: 'not_found' })
724+
725+
const page = await share('notes.html', 'text/html')
726+
expect(await readPublicFileShareContent({ grant: page.grant })).toMatchObject({
727+
kind: 'buffer',
728+
})
729+
// A generation source is compiled, never streamed: with no cached artifact it refuses.
730+
const source = await share('report.pptx', 'text/x-pptxgenjs')
731+
await expect(readPublicFileShareContent({ grant: source.grant })).rejects.toMatchObject({
732+
code: 'conflict',
733+
})
734+
}
735+
)
736+
656737
check(
657738
'public share Office artifacts require the current owner-qualified dependency cache without compilation',
658739
async () => {
@@ -693,7 +774,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
693774
principal: f.principal,
694775
input: { projectId: f.projectId, fileId: source.file.id, maxBytes: 1024 },
695776
})
696-
expect((await readPublicFileShareContent({ grant: access.grant })).buffer).toEqual(
777+
expect(await sharedBytes(readPublicFileShareContent({ grant: access.grant }))).toEqual(
697778
rendered.buffer
698779
)
699780
await updateProjectFileContent.execute({
@@ -740,37 +821,49 @@ describe('Public file shares against PostgreSQL and private storage', () => {
740821
credential: { method: 'POST', password: 'initial-password' },
741822
})
742823
if (!access.authorized) throw new Error('Initial credential rejected')
824+
const applyChange = async () => {
825+
if (change === 'disabled')
826+
await db
827+
.update(publicShare)
828+
.set({ isActive: false })
829+
.where(eq(publicShare.id, share.id))
830+
if (change === 'password')
831+
await db
832+
.update(publicShare)
833+
.set({ password: (await encryptSecret('replacement-password')).encrypted })
834+
.where(eq(publicShare.id, share.id))
835+
if (change === 'provenance')
836+
await db
837+
.update(workspaceFileSecretProvenance)
838+
.set({ status: 'unknown' })
839+
.where(eq(workspaceFileSecretProvenance.fileId, file.id))
840+
if (change === 'archived')
841+
await db
842+
.update(workspaceFiles)
843+
.set({ deletedAt: new Date() })
844+
.where(eq(workspaceFiles.id, file.id))
845+
}
846+
// The change lands while the object is being read, buffered or streamed alike.
743847
const download = storage.downloadFile
744-
const spy = vi.spyOn(storage, 'downloadFile').mockImplementation(async (options) => {
745-
const result = await download(options)
746-
if (options.key === file.key) {
747-
if (change === 'disabled')
748-
await db
749-
.update(publicShare)
750-
.set({ isActive: false })
751-
.where(eq(publicShare.id, share.id))
752-
if (change === 'password')
753-
await db
754-
.update(publicShare)
755-
.set({ password: (await encryptSecret('replacement-password')).encrypted })
756-
.where(eq(publicShare.id, share.id))
757-
if (change === 'provenance')
758-
await db
759-
.update(workspaceFileSecretProvenance)
760-
.set({ status: 'unknown' })
761-
.where(eq(workspaceFileSecretProvenance.fileId, file.id))
762-
if (change === 'archived')
763-
await db
764-
.update(workspaceFiles)
765-
.set({ deletedAt: new Date() })
766-
.where(eq(workspaceFiles.id, file.id))
767-
}
768-
return result
769-
})
770-
await expect(readPublicFileShareContent({ grant: access.grant })).rejects.toMatchObject({
848+
const downloadStream = storage.downloadFileStream
849+
const spies = [
850+
vi.spyOn(storage, 'downloadFile').mockImplementation(async (options) => {
851+
const result = await download(options)
852+
if (options.key === file.key) await applyChange()
853+
return result
854+
}),
855+
vi.spyOn(storage, 'downloadFileStream').mockImplementation(async (options) => {
856+
const result = await downloadStream(options)
857+
if (options.key === file.key) await applyChange()
858+
return result
859+
}),
860+
]
861+
await expect(
862+
sharedBytes(readPublicFileShareContent({ grant: access.grant }))
863+
).rejects.toMatchObject({
771864
code: change === 'provenance' ? 'forbidden' : 'not_found',
772865
})
773-
spy.mockRestore()
866+
for (const spy of spies) spy.mockRestore()
774867
}
775868
}
776869
)
@@ -836,7 +929,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
836929
})
837930
if (!legacyAccess.authorized) throw new Error('Existing workspace share rejected')
838931
expect(
839-
(await readPublicFileShareContent({ grant: legacyAccess.grant })).buffer.toString()
932+
(await sharedBytes(readPublicFileShareContent({ grant: legacyAccess.grant }))).toString()
840933
).toBe('Workspace link')
841934
}
842935
)
@@ -892,7 +985,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
892985
if (!access.authorized) throw new Error('Public page rejected')
893986
const rendered = await readPublicFileShareContent({ grant: access.grant })
894987
expect(rendered.contentType).toBe('text/html')
895-
expect(rendered.buffer.toString()).toContain(
988+
expect((await sharedBytes(rendered)).toString()).toContain(
896989
`data:image/png;base64,${image.toString('base64')}`
897990
)
898991
expect(

0 commit comments

Comments
 (0)