Skip to content

Commit 1defff3

Browse files
authored
fix(public-shares): stream shared files with byte ranges instead of buffering them under 100 MB (#9550)
* fix(public-shares): stream shared files with byte ranges instead of buffering them under 100 MB * fix(public-shares): size a streamed share from storage, audit every delivered response, cover both read paths
1 parent cdff0e5 commit 1defff3

5 files changed

Lines changed: 306 additions & 73 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: 144 additions & 39 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({
@@ -719,31 +800,38 @@ describe('Public file shares against PostgreSQL and private storage', () => {
719800
'@/lib/public-shares/application'
720801
)
721802
const { encryptSecret } = await import('@/lib/core/security/encryption')
722-
for (const change of ['disabled', 'password', 'archived', 'provenance'] as const) {
723-
const f = await fixture()
724-
const { file } = await createProjectFile.execute({
725-
principal: f.principal,
726-
input: createInput(f.projectId),
727-
})
728-
const { share } = await updateProjectFileShare.execute({
729-
principal: f.principal,
730-
input: {
731-
projectId: f.projectId,
732-
fileId: file.id,
733-
isActive: true,
734-
authType: 'password',
735-
password: 'initial-password',
736-
},
737-
})
738-
const access = await authorizePublicFileShare({
739-
token: share.token,
740-
credential: { method: 'POST', password: 'initial-password' },
741-
})
742-
if (!access.authorized) throw new Error('Initial credential rejected')
743-
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) {
803+
// A Markdown file streams from storage; a page is rendered whole. Both reads must refuse.
804+
const shared = {
805+
streamed: (projectId: string) => createInput(projectId),
806+
rendered: (projectId: string) => ({
807+
...createInput(projectId, '---\ntitle: Architecture\n---\n\nShared page'),
808+
name: 'Architecture',
809+
contentType: 'text/x-sim-page',
810+
}),
811+
}
812+
for (const [read, input] of Object.entries(shared))
813+
for (const change of ['disabled', 'password', 'archived', 'provenance'] as const) {
814+
const f = await fixture()
815+
const { file } = await createProjectFile.execute({
816+
principal: f.principal,
817+
input: input(f.projectId),
818+
})
819+
const { share } = await updateProjectFileShare.execute({
820+
principal: f.principal,
821+
input: {
822+
projectId: f.projectId,
823+
fileId: file.id,
824+
isActive: true,
825+
authType: 'password',
826+
password: 'initial-password',
827+
},
828+
})
829+
const access = await authorizePublicFileShare({
830+
token: share.token,
831+
credential: { method: 'POST', password: 'initial-password' },
832+
})
833+
if (!access.authorized) throw new Error('Initial credential rejected')
834+
const applyChange = async () => {
747835
if (change === 'disabled')
748836
await db
749837
.update(publicShare)
@@ -765,13 +853,30 @@ describe('Public file shares against PostgreSQL and private storage', () => {
765853
.set({ deletedAt: new Date() })
766854
.where(eq(workspaceFiles.id, file.id))
767855
}
768-
return result
769-
})
770-
await expect(readPublicFileShareContent({ grant: access.grant })).rejects.toMatchObject({
771-
code: change === 'provenance' ? 'forbidden' : 'not_found',
772-
})
773-
spy.mockRestore()
774-
}
856+
// The change lands while the object is being read, buffered or streamed alike.
857+
const download = storage.downloadFile
858+
const downloadStream = storage.downloadFileStream
859+
const spies = [
860+
vi.spyOn(storage, 'downloadFile').mockImplementation(async (options) => {
861+
const result = await download(options)
862+
if (options.key === file.key) await applyChange()
863+
return result
864+
}),
865+
vi.spyOn(storage, 'downloadFileStream').mockImplementation(async (options) => {
866+
const result = await downloadStream(options)
867+
if (options.key === file.key) await applyChange()
868+
return result
869+
}),
870+
]
871+
const delivered = readPublicFileShareContent({ grant: access.grant })
872+
await expect(sharedBytes(delivered)).rejects.toMatchObject({
873+
code: change === 'provenance' ? 'forbidden' : 'not_found',
874+
})
875+
// Each fixture took the path it names: the stream refuses on open, the page as it renders.
876+
if (read === 'streamed') expect(await delivered).toMatchObject({ kind: 'stream' })
877+
else await expect(delivered).rejects.toBeDefined()
878+
for (const spy of spies) spy.mockRestore()
879+
}
775880
}
776881
)
777882

@@ -836,7 +941,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
836941
})
837942
if (!legacyAccess.authorized) throw new Error('Existing workspace share rejected')
838943
expect(
839-
(await readPublicFileShareContent({ grant: legacyAccess.grant })).buffer.toString()
944+
(await sharedBytes(readPublicFileShareContent({ grant: legacyAccess.grant }))).toString()
840945
).toBe('Workspace link')
841946
}
842947
)
@@ -892,7 +997,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
892997
if (!access.authorized) throw new Error('Public page rejected')
893998
const rendered = await readPublicFileShareContent({ grant: access.grant })
894999
expect(rendered.contentType).toBe('text/html')
895-
expect(rendered.buffer.toString()).toContain(
1000+
expect((await sharedBytes(rendered)).toString()).toContain(
8961001
`data:image/png;base64,${image.toString('base64')}`
8971002
)
8981003
expect(

0 commit comments

Comments
 (0)