Skip to content

Commit a83cfce

Browse files
committed
fix(files): make the preview derivative opt-in and never show a broken image
Five issues from review, all interlocking around one decision. The derivative is now requested with preview=1 rather than suppressed with raw=1. raw=1 would have corrupted generated-document downloads: every non-markdown workspace download routes through the serve route and relies on resolveServableDocBytes compiling stored source into the real binary. Opt-in separates the three consumers cleanly — previews get the JPEG, downloads get untouched stored bytes, and doc compilation stays unconditional. - Public shares resolve the derivative too, with the same preview/download split; the viewer requests it, the download button does not. - Split the brand predicate. isHeifContainer stays broad for the vision path, where it only runs after sharp has already failed. The serve path runs first, so it uses isHevcHeifContainer — an AVIF was costing a storage round-trip, a WASM load and a misleading warn per request. - A derivative that cannot be produced (past the 20MB ceiling, or a decode failure) now falls back to 'Preview not available' instead of a broken image. UnsupportedPreview moved to preview-shared to avoid a module cycle. - The chat composer chip requests the derivative, so HEIC attachments stop rendering as broken thumbnails.
1 parent 64c8392 commit a83cfce

14 files changed

Lines changed: 301 additions & 110 deletions

File tree

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111
import { enforcePublicFileRateLimit } from '@/lib/public-shares/rate-limit'
1212
import { resolveActiveShareByToken } from '@/lib/public-shares/share-manager'
1313
import { downloadFile } from '@/lib/uploads/core/storage-service'
14+
import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative'
1415
import {
1516
createErrorResponse,
1617
createFileResponse,
@@ -31,8 +32,10 @@ const logger = createLogger('PublicFileContentAPI')
3132
*
3233
* Generated office docs are stored as source; {@link resolveServableDoc} swaps in
3334
* their prebuilt compiled binary (read-only, never compiles). Uploaded binaries
34-
* pass through untouched. A generated doc whose compiled artifact isn't built yet
35-
* returns 409 rather than serving raw source under a binary content type.
35+
* pass through untouched, except under `preview=1`, where a format no browser
36+
* decodes is substituted with a renderable derivative. A generated doc whose
37+
* compiled artifact isn't built yet returns 409 rather than serving raw source
38+
* under a binary content type.
3639
*/
3740
export const GET = withRouteHandler(
3841
async (request: NextRequest, context: { params: Promise<{ token: string }> }) => {
@@ -45,6 +48,7 @@ export const GET = withRouteHandler(
4548
const parsed = await parseRequest(getPublicFileContentContract, request, context)
4649
if (!parsed.success) return parsed.response
4750
const { token } = parsed.data.params
51+
const preview = parsed.data.query.preview === '1'
4852

4953
const resolved = await resolveActiveShareByToken(token)
5054
if (!resolved) {
@@ -77,12 +81,20 @@ export const GET = withRouteHandler(
7781
)
7882
}
7983

80-
const buffer = servable.kind === 'artifact' ? servable.buffer : raw
8184
// This response is `nosniff`, so a stored `application/octet-stream` refuses to render
8285
// even though the bytes are fine. Resolving from the filename also keeps this route on
8386
// the same inline allowlist as the workspace serve route.
84-
const contentType =
85-
servable.kind === 'artifact' ? servable.contentType : getContentType(file.originalName)
87+
let buffer = raw
88+
let contentType = getContentType(file.originalName)
89+
if (servable.kind === 'artifact') {
90+
buffer = servable.buffer
91+
contentType = servable.contentType
92+
} else if (preview) {
93+
// Only for a render request: the Download button omits `preview`, so a saved
94+
// file is always the bytes that were shared.
95+
const image = await resolveServableImageBytes(raw, file.key)
96+
if (image) ({ buffer, contentType } = image)
97+
}
8698

8799
logger.info('Public shared file served', { token, key: file.key, size: buffer.length })
88100

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

Lines changed: 61 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -26,27 +26,43 @@ import {
2626

2727
const logger = createLogger('FilesServeAPI')
2828

29+
interface ServeOptions {
30+
/** `raw=1` — bypass all resolution and serve the stored source as-is. */
31+
raw: boolean
32+
/** `preview=1` — the caller renders these bytes rather than saving them. */
33+
preview: boolean
34+
/** `v=<updatedAt>` — the URL addresses content-immutable bytes. */
35+
versioned: boolean
36+
}
37+
2938
/**
30-
* Resolves the bytes + content type to serve for a stored file via the shared
31-
* {@link resolveServableDocBytes} (generated docs → compiled artifact). `raw=1`
32-
* bypasses resolution and serves the stored source as-is.
39+
* Resolves the bytes + content type to serve for a stored file.
40+
*
41+
* Document compilation is unconditional: a generated `.docx`/`.xlsx`/`.pptx` is
42+
* stored as source, so the compiled artifact *is* the file, and every download
43+
* routes through here. An image derivative is the opposite — the stored bytes are
44+
* the file — so it is served only when the caller asked to preview, never when it
45+
* asked to download.
3346
*/
34-
async function resolveServableBytes(
35-
buffer: Buffer,
36-
filename: string,
37-
storageKey: string,
38-
workspaceId: string | undefined,
39-
raw: boolean,
40-
ownerKey: string | undefined,
47+
async function resolveServableBytes(params: {
48+
buffer: Buffer
49+
filename: string
50+
storageKey: string
51+
workspaceId: string | undefined
52+
options: ServeOptions
53+
ownerKey: string | undefined
4154
signal: AbortSignal | undefined
42-
): Promise<{ buffer: Buffer; contentType: string }> {
43-
if (raw) return { buffer, contentType: getContentType(filename) }
44-
45-
// Images resolve first and independently of the document path: a HEIF has no
46-
// compiled-source concept, and its derivative is keyed by storage key rather
47-
// than by source hash.
48-
const image = await resolveServableImageBytes(buffer, storageKey)
49-
if (image) return image
55+
}): Promise<{ buffer: Buffer; contentType: string }> {
56+
const { buffer, filename, storageKey, workspaceId, options, ownerKey, signal } = params
57+
if (options.raw) return { buffer, contentType: getContentType(filename) }
58+
59+
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.
63+
const image = await resolveServableImageBytes(buffer, storageKey)
64+
if (image) return image
65+
}
5066

5167
return resolveServableDocBytes({
5268
rawBuffer: buffer,
@@ -126,10 +142,14 @@ export const GET = withRouteHandler(
126142

127143
const query = fileServeQuerySchema.parse({
128144
raw: request.nextUrl.searchParams.get('raw'),
145+
preview: request.nextUrl.searchParams.get('preview'),
129146
v: request.nextUrl.searchParams.get('v'),
130147
})
131-
const raw = query.raw === '1'
132-
const versioned = query.v != null
148+
const options: ServeOptions = {
149+
raw: query.raw === '1',
150+
preview: query.preview === '1',
151+
versioned: query.v != null,
152+
}
133153

134154
const authResult = await checkSessionOrInternalAuth(request, { requireWorkflowId: false })
135155

@@ -144,10 +164,10 @@ export const GET = withRouteHandler(
144164
const userId = authResult.userId
145165

146166
if (isUsingCloudStorage()) {
147-
return await handleCloudProxy(cloudKey, userId, raw, versioned, request.signal)
167+
return await handleCloudProxy(cloudKey, userId, options, request.signal)
148168
}
149169

150-
return await handleLocalFile(cloudKey, userId, raw, versioned, request.signal)
170+
return await handleLocalFile(cloudKey, userId, options, request.signal)
151171
} catch (error) {
152172
// An in-progress/incomplete doc source fails to compile — this is expected
153173
// mid-generation, not a server fault. Return 409 (not 500) so it isn't an
@@ -174,8 +194,7 @@ export const GET = withRouteHandler(
174194
async function handleLocalFile(
175195
filename: string,
176196
userId: string,
177-
raw: boolean,
178-
versioned: boolean,
197+
options: ServeOptions,
179198
signal: AbortSignal | undefined
180199
): Promise<NextResponse> {
181200
const ownerKey = `user:${userId}`
@@ -207,23 +226,23 @@ async function handleLocalFile(
207226
const segment = filename.split('/').pop() || filename
208227
const displayName = stripStorageKeyPrefix(segment)
209228
const workspaceId = getWorkspaceIdForCompile(filename)
210-
const { buffer: fileBuffer, contentType } = await resolveServableBytes(
211-
rawBuffer,
212-
displayName,
213-
filename,
229+
const { buffer: fileBuffer, contentType } = await resolveServableBytes({
230+
buffer: rawBuffer,
231+
filename: displayName,
232+
storageKey: filename,
214233
workspaceId,
215-
raw,
234+
options,
216235
ownerKey,
217-
signal
218-
)
236+
signal,
237+
})
219238

220239
logger.info('Local file served', { userId, filename, size: fileBuffer.length })
221240

222241
return createFileResponse({
223242
buffer: fileBuffer,
224243
contentType,
225244
filename: displayName,
226-
cacheControl: resolveServeCacheControl(versioned, contextParam),
245+
cacheControl: resolveServeCacheControl(options.versioned, contextParam),
227246
})
228247
} catch (error) {
229248
logger.error('Error reading local file:', error)
@@ -234,9 +253,8 @@ async function handleLocalFile(
234253
async function handleCloudProxy(
235254
cloudKey: string,
236255
userId: string,
237-
raw = false,
238-
versioned = false,
239-
signal: AbortSignal | undefined = undefined
256+
options: ServeOptions,
257+
signal: AbortSignal | undefined
240258
): Promise<NextResponse> {
241259
const ownerKey = `user:${userId}`
242260
try {
@@ -270,15 +288,15 @@ async function handleCloudProxy(
270288
const segment = cloudKey.split('/').pop() || 'download'
271289
const displayName = stripStorageKeyPrefix(segment)
272290
const workspaceId = getWorkspaceIdForCompile(cloudKey)
273-
const { buffer: fileBuffer, contentType } = await resolveServableBytes(
274-
rawBuffer,
275-
displayName,
276-
cloudKey,
291+
const { buffer: fileBuffer, contentType } = await resolveServableBytes({
292+
buffer: rawBuffer,
293+
filename: displayName,
294+
storageKey: cloudKey,
277295
workspaceId,
278-
raw,
296+
options,
279297
ownerKey,
280-
signal
281-
)
298+
signal,
299+
})
282300

283301
logger.info('Cloud file served', {
284302
userId,
@@ -291,7 +309,7 @@ async function handleCloudProxy(
291309
buffer: fileBuffer,
292310
contentType,
293311
filename: displayName,
294-
cacheControl: resolveServeCacheControl(versioned, context),
312+
cacheControl: resolveServeCacheControl(options.versioned, context),
295313
})
296314
} catch (error) {
297315
logger.error('Error downloading from cloud storage:', error)

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

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { Music } from '@sim/emcn/icons'
55
import dynamic from 'next/dynamic'
66
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
7-
import { getFileExtension, resolveMediaMimeType } from '@/lib/uploads/utils/file-utils'
7+
import { resolveMediaMimeType } from '@/lib/uploads/utils/file-utils'
88
import {
99
useWorkspaceFileBinary,
1010
useWorkspaceFileContent,
@@ -28,6 +28,7 @@ import {
2828
PreviewErrorBoundary,
2929
PreviewLoadingFrame,
3030
resolvePreviewError,
31+
UnsupportedPreview,
3132
} from './preview-shared'
3233
import { TextEditor } from './text-editor'
3334
import { useDocPreviewBinary } from './use-doc-preview-binary'
@@ -421,22 +422,3 @@ const MediaPreview = memo(function MediaPreview({
421422
</div>
422423
)
423424
})
424-
425-
const UnsupportedPreview = memo(function UnsupportedPreview({
426-
file,
427-
}: {
428-
file: WorkspaceFileRecord
429-
}) {
430-
const ext = getFileExtension(file.name)
431-
432-
return (
433-
<div className='flex flex-1 flex-col items-center justify-center gap-[8px]'>
434-
<p className='font-medium text-[14px] text-[var(--text-primary)]'>
435-
Preview not available{ext ? ` for .${ext} files` : ' for this file'}
436-
</p>
437-
<p className='text-[13px] text-[var(--text-muted)]'>
438-
Use the download button to view this file
439-
</p>
440-
</div>
441-
)
442-
})
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
7+
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
8+
import { ImagePreview } from './image-preview'
9+
10+
const file = {
11+
id: 'file-1',
12+
workspaceId: 'ws-1',
13+
name: 'photo.heic',
14+
key: 'workspace/ws-1/photo.heic',
15+
path: '',
16+
size: 1024,
17+
type: 'image/heic',
18+
uploadedBy: 'user-1',
19+
folderId: null,
20+
uploadedAt: new Date('2026-01-01T00:00:00Z'),
21+
updatedAt: new Date('2026-01-01T00:00:00Z'),
22+
} satisfies WorkspaceFileRecord
23+
24+
let container: HTMLDivElement
25+
let root: Root
26+
27+
beforeEach(() => {
28+
globalThis.IS_REACT_ACT_ENVIRONMENT = true
29+
// ZoomablePreview measures its content through one; jsdom has no implementation.
30+
globalThis.ResizeObserver = class {
31+
observe() {}
32+
unobserve() {}
33+
disconnect() {}
34+
}
35+
container = document.createElement('div')
36+
document.body.appendChild(container)
37+
root = createRoot(container)
38+
})
39+
40+
afterEach(() => {
41+
act(() => root.unmount())
42+
container.remove()
43+
})
44+
45+
function render() {
46+
act(() => root.render(<ImagePreview file={file} />))
47+
}
48+
49+
describe('ImagePreview', () => {
50+
it('requests the preview derivative rather than the stored bytes', () => {
51+
render()
52+
53+
const src = container.querySelector('img')?.getAttribute('src') ?? ''
54+
expect(src).toContain('preview=1')
55+
expect(src).not.toContain('raw=1')
56+
})
57+
58+
it('falls back to the unsupported state when the image fails to decode', () => {
59+
render()
60+
61+
const img = container.querySelector('img')
62+
expect(img).not.toBeNull()
63+
64+
act(() => {
65+
img?.dispatchEvent(new Event('error'))
66+
})
67+
68+
expect(container.querySelector('img')).toBeNull()
69+
expect(container.textContent).toContain('Preview not available')
70+
})
71+
})

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,25 @@
33
import { memo, useState } from 'react'
44
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
55
import { useFileContentSource } from '@/hooks/use-file-content-source'
6-
import { PREVIEW_LOADING_OVERLAY } from './preview-shared'
6+
import { PREVIEW_LOADING_OVERLAY, UnsupportedPreview } from './preview-shared'
77
import { ZoomablePreview } from './zoomable-preview'
88

99
export const ImagePreview = memo(function ImagePreview({ file }: { file: WorkspaceFileRecord }) {
1010
const source = useFileContentSource()
11-
const [hasSettled, setHasSettled] = useState(false)
11+
const [status, setStatus] = useState<'loading' | 'loaded' | 'error'>('loading')
1212
// Version the URL on updatedAt: overwrites keep the same storage key, so an unversioned
1313
// URL would resolve to a previously cached copy instead of the rewritten bytes.
14+
// `preview` lets the server substitute a renderable derivative for a HEIC.
1415
const serveUrl = source.buildUrl(file.key, {
1516
version: Number(new Date(file.updatedAt)) || file.size,
17+
preview: true,
1618
})
1719

20+
// Covers every way the bytes can turn out unrenderable — a derivative the server
21+
// declined to build (too large, undecodable) and a corrupt or truncated image
22+
// alike — rather than leaving a broken image in the viewer.
23+
if (status === 'error') return <UnsupportedPreview file={file} />
24+
1825
return (
1926
<div className='relative flex min-h-0 flex-1 flex-col'>
2027
<ZoomablePreview className='flex flex-1' contentClassName='h-full w-full'>
@@ -24,11 +31,11 @@ export const ImagePreview = memo(function ImagePreview({ file }: { file: Workspa
2431
className='max-h-full max-w-full select-none rounded-md object-contain'
2532
draggable={false}
2633
loading='eager'
27-
onLoad={() => setHasSettled(true)}
28-
onError={() => setHasSettled(true)}
34+
onLoad={() => setStatus('loaded')}
35+
onError={() => setStatus('error')}
2936
/>
3037
</ZoomablePreview>
31-
{!hasSettled && PREVIEW_LOADING_OVERLAY}
38+
{status === 'loading' && PREVIEW_LOADING_OVERLAY}
3239
</div>
3340
)
3441
})

0 commit comments

Comments
 (0)