Skip to content

Commit 6e1de2c

Browse files
committed
improvement(files): drop the dead preview reset and cap the ftyp brand scan
- Content writes mint a new storage key, so the parent's key={file.key} already remounts ImagePreview; the render-phase reset was unreachable and made renames flash a loading overlay. - Clamp the ftyp compatible-brand scan to a real box size. The declared size is attacker-controlled and this now runs on every preview request. - UnsupportedPreview takes a primitive name so memo is load-bearing. - Fix the hardcoded ? in the public preview URL builder.
1 parent c1aec8b commit 6e1de2c

10 files changed

Lines changed: 39 additions & 37 deletions

File tree

apps/sim/app/api/files/serve/[...path]/route.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,8 @@ async function resolveServableBytes(params: {
5757
if (options.raw) return { buffer, contentType: getContentType(filename) }
5858

5959
if (options.preview) {
60-
// Images resolve first and independently of the document path: a HEIF has no
61-
// compiled-source concept, and its derivative is keyed by storage key rather
62-
// than by source hash.
60+
// Images resolve independently of the document path: a HEIF has no compiled-source
61+
// concept, so it never reaches the doc branch.
6362
const image = await resolveServableImageBytes(buffer, storageKey)
6463
if (image) return image
6564
}

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ function FileViewerContent({
171171
// browser. CsvTablePreview's streamed fallback is workspace-only, so on the
172172
// read-only public path a large CSV is download-only.
173173
if (isCsvStreamOnly(file)) {
174-
return <UnsupportedPreview file={file} />
174+
return <UnsupportedPreview name={file.name} />
175175
}
176176
// Markdown renders through the inline rich editor (non-editable) so the public share
177177
// surface matches the in-app reading experience; canEdit={false} disables autosave,
@@ -260,7 +260,7 @@ function FileViewerContent({
260260
return <XlsxPreview key={file.id} file={file} workspaceId={workspaceId} />
261261
}
262262

263-
return <UnsupportedPreview file={file} />
263+
return <UnsupportedPreview name={file.name} />
264264
}
265265

266266
/**

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/image-preview.test.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,18 @@ describe('ImagePreview', () => {
6969
expect(container.textContent).toContain('Preview not available')
7070
})
7171

72-
it('retries when the file is overwritten, which keeps the same storage key', () => {
73-
render()
72+
it('retries after an overwrite, which mints a new storage key and remounts', () => {
73+
act(() => root.render(<ImagePreview key={file.key} file={file} />))
7474

7575
act(() => {
7676
container.querySelector('img')?.dispatchEvent(new Event('error'))
7777
})
7878
expect(container.textContent).toContain('Preview not available')
7979

80-
// An overwrite preserves `file.key` — the parent's key — so only the version
81-
// changes and the component never remounts.
82-
render({ ...file, updatedAt: new Date('2026-02-01T00:00:00Z') })
80+
// Every content write mints a fresh key, so the parent's `key={file.key}`
81+
// remounts this and the previous bytes' outcome cannot stick.
82+
const overwritten = { ...file, key: 'workspace/ws-1/photo-v2.heic' }
83+
act(() => root.render(<ImagePreview key={overwritten.key} file={overwritten} />))
8384

8485
expect(container.querySelector('img')).not.toBeNull()
8586
expect(container.textContent).not.toContain('Preview not available')

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/image-preview.tsx

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,19 @@ import { ZoomablePreview } from './zoomable-preview'
88

99
export const ImagePreview = memo(function ImagePreview({ file }: { file: WorkspaceFileRecord }) {
1010
const source = useFileContentSource()
11-
// Version the URL on updatedAt: overwrites keep the same storage key, so an unversioned
12-
// URL would resolve to a previously cached copy instead of the rewritten bytes.
13-
// `preview` lets the server substitute a renderable derivative for a HEIC.
11+
/** `v` busts the browser cache across content writes; `preview` lets the server
12+
* substitute a renderable JPEG for a HEIC. */
1413
const serveUrl = source.buildUrl(file.key, {
1514
version: Number(new Date(file.updatedAt)) || file.size,
1615
preview: true,
1716
})
1817

1918
const [status, setStatus] = useState<'loading' | 'loaded' | 'error'>('loading')
20-
const [loadedUrl, setLoadedUrl] = useState(serveUrl)
2119

22-
// The parent keys this on `file.key`, which an overwrite preserves — only the
23-
// version changes. Without this the outcome of the previous bytes would stick,
24-
// leaving a replaced image permanently on the unsupported state.
25-
if (loadedUrl !== serveUrl) {
26-
setLoadedUrl(serveUrl)
27-
setStatus('loading')
28-
}
29-
30-
// Covers every way the bytes can turn out unrenderable — a derivative the server
31-
// declined to build (too large, undecodable) and a corrupt or truncated image
32-
// alike — rather than leaving a broken image in the viewer.
33-
if (status === 'error') return <UnsupportedPreview file={file} />
20+
/** A derivative the server declined to build, or corrupt bytes — show the download
21+
* fallback rather than a broken image. Content writes mint a new storage key, so
22+
* the parent's `key={file.key}` resets this. */
23+
if (status === 'error') return <UnsupportedPreview name={file.name} />
3424

3525
return (
3626
<div className='relative flex min-h-0 flex-1 flex-col'>

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/preview-shared.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,8 @@ const logger = createLogger('FilePreview')
1313
* viewer at all, or a viewer that was expected to work failed (e.g. a HEIC whose
1414
* server-side derivative could not be produced).
1515
*/
16-
export const UnsupportedPreview = memo(function UnsupportedPreview({
17-
file,
18-
}: {
19-
file: { name: string }
20-
}) {
21-
const ext = getFileExtension(file.name)
16+
export const UnsupportedPreview = memo(function UnsupportedPreview({ name }: { name: string }) {
17+
const ext = getFileExtension(name)
2218

2319
return (
2420
<div className='flex flex-1 flex-col items-center justify-center gap-[8px]'>

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ export function createPublicFileContentSource(
117117
contentUrl: string
118118
): FileContentSource {
119119
return inlineImageSource(
120-
(_key, opts) => (opts?.preview ? `${contentUrl}?preview=1` : contentUrl),
120+
(_key, opts) =>
121+
opts?.preview ? `${contentUrl}${contentUrl.includes('?') ? '&' : '?'}preview=1` : contentUrl,
121122
`/api/files/public/${token}/inline`
122123
)
123124
}

apps/sim/lib/api/contracts/public-shares.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export const getPublicFileContract = defineRouteContract({
117117
})
118118

119119
const publicFileContentQuerySchema = z.object({
120-
/** `1` => the caller is rendering these bytes, not downloading them, so a format no browser decodes (HEIC) may be substituted with a renderable derivative. Absent => the stored bytes are served. */
120+
/** `1` => rendering, not downloading — a HEIC may be substituted with a JPEG derivative. */
121121
preview: z.string().nullish(),
122122
})
123123

apps/sim/lib/api/contracts/storage-transfer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ export const fileServeParamsSchema = z.object({
501501

502502
export const fileServeQuerySchema = z.object({
503503
raw: z.string().nullish(),
504-
/** `1` => the caller is rendering these bytes, not downloading them, so a format no browser decodes (HEIC) may be substituted with a renderable derivative. Absent => the stored bytes are served. */
504+
/** `1` => rendering, not downloading — a HEIC may be substituted with a JPEG derivative. */
505505
preview: z.string().nullish(),
506506
/** Content version (the file record's `updatedAt`). Present => the URL is content-immutable and may be cached indefinitely by the browser. */
507507
v: z.string().nullish(),

apps/sim/lib/uploads/server/heic.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,17 @@ describe('isHeifContainer', () => {
7373
expect(isHeifContainer(truncated)).toBe(false)
7474
})
7575

76+
it('caps the scan so an inflated declared box size cannot drive the loop', () => {
77+
// The declared size is attacker-controlled; without the cap this scans the
78+
// whole buffer, and this runs on every preview request.
79+
const inflated = Buffer.alloc(64 * 1024)
80+
inflated.writeUInt32BE(0xffffffff, 0)
81+
inflated.write('ftyp', 4, 'ascii')
82+
inflated.write('isom', 8, 'ascii')
83+
inflated.write('heic', 60 * 1024, 'ascii')
84+
expect(isHeifContainer(inflated)).toBe(false)
85+
})
86+
7687
it('rejects buffers too short to carry a brand', () => {
7788
expect(isHeifContainer(Buffer.alloc(0))).toBe(false)
7889
expect(isHeifContainer(ftypHeader('heic').subarray(0, 11))).toBe(false)

apps/sim/lib/uploads/server/heic.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ const HEIF_BRANDS = new Set([...HEVC_HEIF_BRANDS, 'mif1', 'msf1', 'avif', 'avis'
3030
*/
3131
const MAX_TRANSCODE_INPUT_BYTES = 20 * 1024 * 1024
3232

33+
/** A real `ftyp` box holds a handful of brands; anything larger is malformed or hostile. */
34+
const MAX_FTYP_BOX_BYTES = 512
35+
3336
/**
3437
* Whether an ISO-BMFF `ftyp` box names any of `brands`, as either the major brand
3538
* or a compatible brand.
@@ -47,8 +50,9 @@ function declaresBrand(buffer: Buffer, brands: ReadonlySet<string>): boolean {
4750
// the HEIF brand only among the compatible brands, which follow the 4-byte
4851
// minor_version at offset 12 and run to the end of the box. A declared size of 0
4952
// or 1 (the ISO-BMFF size escapes, which `ftyp` does not use) leaves `end` below
50-
// the loop's start, so those simply do not scan.
51-
const end = Math.min(buffer.readUInt32BE(0), buffer.length)
53+
// the loop's start, so those simply do not scan. The size is attacker-controlled
54+
// and this runs on every preview, so cap it rather than trusting the declaration.
55+
const end = Math.min(buffer.readUInt32BE(0), buffer.length, MAX_FTYP_BOX_BYTES)
5256
for (let offset = 16; offset + 4 <= end; offset += 4) {
5357
if (brands.has(buffer.toString('ascii', offset, offset + 4))) return true
5458
}

0 commit comments

Comments
 (0)