Skip to content

Commit 942cc08

Browse files
committed
fix(files): settle the audio/video container ambiguity at the call site
Follow-up to the review pass on this branch. - Revert the global .webm -> video/webm remap. EXTENSION_TO_MIME is shared with non-viewer callers, and a .webm with an empty stored type would have started taking the STT route's video branch (stt/route.ts:211 -> extractAudioFromVideo), which 500s where no ffmpeg binary is on PATH. The ambiguity is now settled in resolveMediaMimeType, which knows which element the caller is rendering - Resolve the public share route's Content-Type from the filename via getContentType, matching the workspace serve route, instead of echoing the client-declared stored type into a public unauthenticated response. Add the audio/video entries contentTypeMap was missing so a shared media file keeps a real Content-Type (disposition is unchanged - none are inline-safe) - Make resolveEffectiveMimeType total (string, not string | null); the null contract only bought one label edge case and cost a ?? at every call site, one of which was dead - Drop .jsonl from the text-editable set. The editor loads the whole file and only CSV has a byte cap, so a large .jsonl would trade a download-only fallback for a crashed tab. Needs the size guard generalized first - Trim two comments that restated their code
1 parent 191f1e6 commit 942cc08

8 files changed

Lines changed: 116 additions & 107 deletions

File tree

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ 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 { createErrorResponse, createFileResponse, FileNotFoundError } from '@/app/api/files/utils'
14+
import {
15+
createErrorResponse,
16+
createFileResponse,
17+
FileNotFoundError,
18+
getContentType,
19+
} from '@/app/api/files/utils'
1520

1621
export const dynamic = 'force-dynamic'
1722

@@ -73,7 +78,11 @@ export const GET = withRouteHandler(
7378
}
7479

7580
const buffer = servable.kind === 'artifact' ? servable.buffer : raw
76-
const contentType = servable.kind === 'artifact' ? servable.contentType : file.contentType
81+
// This response is `nosniff`, so a stored `application/octet-stream` refuses to render
82+
// even though the bytes are fine. Resolving from the filename also keeps this route on
83+
// the same inline allowlist as the workspace serve route.
84+
const contentType =
85+
servable.kind === 'artifact' ? servable.contentType : getContentType(file.originalName)
7786

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

apps/sim/app/api/files/utils.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ export const contentTypeMap: Record<string, string> = {
6363
avif: 'image/avif',
6464
bmp: 'image/bmp',
6565
ico: 'image/x-icon',
66+
mp3: 'audio/mpeg',
67+
m4a: 'audio/mp4',
68+
wav: 'audio/wav',
69+
ogg: 'audio/ogg',
70+
flac: 'audio/flac',
71+
aac: 'audio/aac',
72+
opus: 'audio/opus',
73+
mp4: 'video/mp4',
74+
mov: 'video/quicktime',
75+
avi: 'video/x-msvideo',
76+
mkv: 'video/x-matroska',
77+
webm: 'video/webm',
6678
zip: 'application/zip',
6779
googleFolder: 'application/vnd.google-apps.folder',
6880
}

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-category.test.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,6 @@ describe('resolveFileCategory — formats accepted on upload must be previewable
238238
expect(resolveFileCategory(null, filename)).toBe('unsupported')
239239
}
240240
)
241-
242-
it('.jsonl opens in the text editor', () => {
243-
expect(resolveFileCategory('application/jsonl', 'events.jsonl')).toBe('text-editable')
244-
expect(resolveFileCategory('application/octet-stream', 'events.jsonl')).toBe('text-editable')
245-
})
246241
})
247242

248243
describe('resolveFileCategory — extension case', () => {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ const TEXT_EDITABLE_MIME_TYPES = new Set([
55
'text/markdown',
66
'text/plain',
77
'application/json',
8-
'application/jsonl',
98
'application/x-yaml',
109
'text/csv',
1110
'text/html',
@@ -27,7 +26,6 @@ const TEXT_EDITABLE_EXTENSIONS = new Set([
2726
'md',
2827
'txt',
2928
'json',
30-
'jsonl',
3129
'yaml',
3230
'yml',
3331
'csv',

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

Lines changed: 2 additions & 26 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, resolveEffectiveMimeType } from '@/lib/uploads/utils/file-utils'
7+
import { getFileExtension, resolveMediaMimeType } from '@/lib/uploads/utils/file-utils'
88
import {
99
useWorkspaceFileBinary,
1010
useWorkspaceFileContent,
@@ -361,30 +361,6 @@ function useBlobUrl(workspaceId: string, fileId: string, fileKey: string) {
361361
return { fileData, isLoading, error, blobUrl, replaceBlobUrl }
362362
}
363363

364-
const MEDIA_FALLBACK_MIME = { audio: 'audio/mpeg', video: 'video/mp4' } as const
365-
366-
/**
367-
* The `type` for the blob backing an `<audio>`/`<video>` element. That type is the only
368-
* format signal the element gets, so it has to be resolved against the filename rather
369-
* than read off the record: storage legitimately holds `application/octet-stream` (the
370-
* browser reports it for plenty of media formats, and the presigned PUT handshake
371-
* requires persisting it verbatim), and an element handed that type cannot determine the
372-
* format and renders nothing — which is why the same file downloads perfectly.
373-
*
374-
* Anything that does not resolve to a playable media type falls back to the kind's
375-
* default rather than being passed through, so a stale or mismatched stored type can
376-
* never leave the element with a type it cannot play.
377-
*/
378-
function resolveMediaBlobType(
379-
storedType: string | null,
380-
filename: string,
381-
kind: 'audio' | 'video'
382-
): string {
383-
const resolved = resolveEffectiveMimeType(storedType, filename)
384-
if (resolved?.startsWith('audio/') || resolved?.startsWith('video/')) return resolved
385-
return MEDIA_FALLBACK_MIME[kind]
386-
}
387-
388364
/**
389365
* Shared blob-backed preview for audio and video files — the fetch, blob-URL
390366
* lifecycle, and error/loading handling are identical; only the rendered
@@ -407,7 +383,7 @@ const MediaPreview = memo(function MediaPreview({
407383
replaceBlobUrl,
408384
} = useBlobUrl(workspaceId, file.id, file.key)
409385

410-
const mediaType = resolveMediaBlobType(file.type, file.name, kind)
386+
const mediaType = resolveMediaMimeType(file.type, file.name, kind)
411387

412388
useEffect(() => {
413389
if (!fileData) return

apps/sim/app/workspace/[workspaceId]/files/files.tsx

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -197,22 +197,20 @@ const hasExternalFiles = (dataTransfer: DataTransfer): boolean =>
197197
dataTransfer.types.includes('Files')
198198

199199
function formatFileType(storedType: string | null, filename: string): string {
200-
// A stored `application/octet-stream` labels nothing and matches no filter, so the
201-
// effective type is resolved from the filename first — see resolveEffectiveMimeType.
202200
const mimeType = resolveEffectiveMimeType(storedType, filename)
203201

204-
if (mimeType && MIME_TYPE_LABELS[mimeType]) {
202+
if (MIME_TYPE_LABELS[mimeType]) {
205203
return MIME_TYPE_LABELS[mimeType]
206204
}
207205

208-
if (mimeType?.startsWith('audio/')) return 'Audio'
209-
if (mimeType?.startsWith('video/')) return 'Video'
210-
if (mimeType?.startsWith('image/')) return 'Image'
206+
if (mimeType.startsWith('audio/')) return 'Audio'
207+
if (mimeType.startsWith('video/')) return 'Video'
208+
if (mimeType.startsWith('image/')) return 'Image'
211209

212210
const ext = getFileExtension(filename)
213211
if (ext) return ext.toUpperCase()
214212

215-
return mimeType ?? 'File'
213+
return storedType ?? 'File'
216214
}
217215

218216
export function Files() {
@@ -498,9 +496,9 @@ export function Files() {
498496
if (typeFilter.length > 0) {
499497
result = result.filter((f) => {
500498
const ext = getFileExtension(f.name)
501-
// Matching on the raw stored type would hide every file the browser uploaded as
499+
// Matching the raw stored type would hide every file the browser uploaded as
502500
// `application/octet-stream` from the audio/video/image filters.
503-
const type = resolveEffectiveMimeType(f.type, f.name) ?? ''
501+
const type = resolveEffectiveMimeType(f.type, f.name)
504502
if (typeFilter.includes('document') && isSupportedExtension(ext)) return true
505503
if (typeFilter.includes('audio') && isAudioFileType(type)) return true
506504
if (typeFilter.includes('video') && isVideoFileType(type)) return true

apps/sim/lib/uploads/utils/file-utils.test.ts

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
isNetworkError,
1313
processSingleFileToUserFile,
1414
resolveEffectiveMimeType,
15+
resolveMediaMimeType,
1516
resolveTrustedFileContext,
1617
} from '@/lib/uploads/utils/file-utils'
1718

@@ -220,22 +221,37 @@ describe('resolveEffectiveMimeType', () => {
220221
expect(resolveEffectiveMimeType(undefined, 'clip.mp4')).toBe('video/mp4')
221222
})
222223

223-
it('resolves .webm to the video type so the picture is not dropped', () => {
224-
expect(resolveEffectiveMimeType('application/octet-stream', 'clip.webm')).toBe('video/webm')
224+
it('stays generic when the extension identifies nothing either', () => {
225+
expect(resolveEffectiveMimeType('application/octet-stream', 'firmware.bin')).toBe(
226+
'application/octet-stream'
227+
)
228+
expect(resolveEffectiveMimeType(null, 'firmware.bin')).toBe('application/octet-stream')
229+
expect(resolveEffectiveMimeType('', 'noextension')).toBe('application/octet-stream')
230+
})
231+
})
232+
233+
describe('resolveMediaMimeType', () => {
234+
it('resolves a generic stored type from the extension', () => {
235+
expect(resolveMediaMimeType('application/octet-stream', 'clip.mp4', 'video')).toBe('video/mp4')
236+
expect(resolveMediaMimeType('application/octet-stream', 'song.flac', 'audio')).toBe(
237+
'audio/flac'
238+
)
225239
})
226240

227-
it('keeps an explicit audio/webm from the browser', () => {
228-
expect(resolveEffectiveMimeType('audio/webm', 'recording.webm')).toBe('audio/webm')
241+
it('retags a dual audio/video container to the kind being rendered', () => {
242+
expect(resolveMediaMimeType(null, 'clip.webm', 'video')).toBe('video/webm')
243+
expect(resolveMediaMimeType('audio/webm', 'clip.webm', 'video')).toBe('video/webm')
244+
expect(resolveMediaMimeType(null, 'recording.webm', 'audio')).toBe('audio/webm')
245+
expect(resolveMediaMimeType('video/webm', 'recording.webm', 'audio')).toBe('audio/webm')
229246
})
230247

231-
it('returns the generic type when the extension identifies nothing either', () => {
232-
expect(resolveEffectiveMimeType('application/octet-stream', 'firmware.bin')).toBe(
233-
'application/octet-stream'
234-
)
248+
it('keeps a specific type that already names the right kind', () => {
249+
expect(resolveMediaMimeType('video/quicktime', 'clip.mov', 'video')).toBe('video/quicktime')
250+
expect(resolveMediaMimeType('audio/opus', 'voice.opus', 'audio')).toBe('audio/opus')
235251
})
236252

237-
it('returns null when there is neither a stored type nor a known extension', () => {
238-
expect(resolveEffectiveMimeType(null, 'firmware.bin')).toBeNull()
239-
expect(resolveEffectiveMimeType('', 'noextension')).toBeNull()
253+
it('falls back to the kind default when nothing names a media format', () => {
254+
expect(resolveMediaMimeType('application/zip', 'weird.bin', 'audio')).toBe('audio/mpeg')
255+
expect(resolveMediaMimeType(null, 'weird.bin', 'video')).toBe('video/mp4')
240256
})
241257
})

apps/sim/lib/uploads/utils/file-utils.ts

Lines changed: 57 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ const EXTENSION_TO_MIME: Record<string, string> = {
312312
xls: 'application/vnd.ms-excel',
313313
ppt: 'application/vnd.ms-powerpoint',
314314
md: 'text/markdown',
315-
jsonl: 'application/jsonl',
316315
yaml: 'application/x-yaml',
317316
yml: 'application/x-yaml',
318317
rtf: 'application/rtf',
@@ -363,15 +362,12 @@ const EXTENSION_TO_MIME: Record<string, string> = {
363362
graphql: 'text/x-graphql',
364363
gql: 'text/x-graphql',
365364
proto: 'text/x-protobuf',
366-
mmd: 'text/x-mermaid',
367-
diff: 'text/x-diff',
368-
patch: 'text/x-diff',
369-
fish: 'text/x-shellscript',
370365

371366
// Audio
372367
mp3: 'audio/mpeg',
373368
m4a: 'audio/mp4',
374369
wav: 'audio/wav',
370+
webm: 'audio/webm',
375371
ogg: 'audio/ogg',
376372
flac: 'audio/flac',
377373
aac: 'audio/aac',
@@ -382,45 +378,80 @@ const EXTENSION_TO_MIME: Record<string, string> = {
382378
mov: 'video/quicktime',
383379
avi: 'video/x-msvideo',
384380
mkv: 'video/x-matroska',
385-
// `.webm` is both an audio and a video container; the video type is the safe
386-
// resolution because a `<video>` element plays an audio-only stream, while an
387-
// `<audio>` element handed a video stream drops the picture.
388-
webm: 'video/webm',
389381
}
390382

391-
/**
392-
* MIME types that carry no format information. Storage keeps whatever the browser
393-
* reported at upload time, and the direct-PUT path preserves it verbatim (see
394-
* {@link getFileContentType}), so a stored type may be one of these even when the
395-
* filename identifies the format precisely.
396-
*/
397-
const GENERIC_MIME_TYPES = new Set(['application/octet-stream', 'binary/octet-stream'])
383+
const GENERIC_MIME_TYPE = 'application/octet-stream'
384+
385+
/** Every MIME type that identifies no format, including the legacy `binary/` spelling. */
386+
const GENERIC_MIME_TYPES = new Set([GENERIC_MIME_TYPE, 'binary/octet-stream'])
398387

399388
/**
400389
* Get MIME type from file extension (fallback if not provided)
401390
*/
402391
export function getMimeTypeFromExtension(extension: string): string {
403-
return EXTENSION_TO_MIME[extension.toLowerCase()] || 'application/octet-stream'
392+
return EXTENSION_TO_MIME[extension.toLowerCase()] || GENERIC_MIME_TYPE
393+
}
394+
395+
/**
396+
* The MIME type that best identifies `filename`, preferring `declaredType` — by a browser
397+
* at upload time or by storage at read time — and falling back to the extension when what
398+
* was declared identifies no format.
399+
*
400+
* A declared `application/octet-stream` is not an error: browsers report it for plenty of
401+
* real formats, and the presigned PUT handshake requires persisting it verbatim (see
402+
* {@link getFileContentType}). A stored type therefore has to be resolved here before it
403+
* can drive rendering — a truthiness check (`file.type || fallback`) passes the generic
404+
* type straight through, and a `Blob` or media element handed that renders nothing.
405+
*/
406+
export function resolveEffectiveMimeType(
407+
declaredType: string | null | undefined,
408+
filename: string
409+
): string {
410+
const declared = declaredType?.trim()
411+
if (declared && !GENERIC_MIME_TYPES.has(declared)) return declared
412+
return getMimeTypeFromExtension(getFileExtension(filename))
413+
}
414+
415+
const MEDIA_FALLBACK_MIME = { audio: 'audio/mpeg', video: 'video/mp4' } as const
416+
417+
/**
418+
* The MIME type to hand an `<audio>`/`<video>` element, given which of the two the caller
419+
* is rendering.
420+
*
421+
* Beyond {@link resolveEffectiveMimeType} this settles an ambiguity a filename alone cannot:
422+
* `.webm` and `.ogg` are both audio and video containers, so a resolved `audio/webm` would
423+
* make a `<video>` element drop the picture. The caller has already chosen the element, so
424+
* the container subtype is kept and retagged to that kind — the choice belongs here, where
425+
* the kind is known, and not in the extension table, which several non-viewer callers share.
426+
*
427+
* A type naming no media format falls back to the kind's default: passed through, it would
428+
* leave the element unable to determine the format, rendering nothing.
429+
*/
430+
export function resolveMediaMimeType(
431+
declaredType: string | null | undefined,
432+
filename: string,
433+
kind: 'audio' | 'video'
434+
): string {
435+
const resolved = resolveEffectiveMimeType(declaredType, filename)
436+
const [type, subtype] = resolved.split('/')
437+
if (type === kind) return resolved
438+
if (type === 'audio' || type === 'video') return `${kind}/${subtype}`
439+
return MEDIA_FALLBACK_MIME[kind]
404440
}
405441

406442
/**
407443
* Resolve a reliable MIME type from a file, falling back to the extension map
408-
* when the browser reports an empty type. By default treats
409-
* `application/octet-stream` as "unknown" and falls back to the extension —
410-
* pass `{ preserveOctetStream: true }` for direct PUT uploads where the
444+
* when the browser reports an empty or generic type. Pass
445+
* `{ preserveOctetStream: true }` for direct PUT uploads where the
411446
* browser-supplied content-type must match the presigned handshake exactly.
412447
*/
413448
export function resolveFileType(
414449
file: { type: string; name: string },
415450
options?: { preserveOctetStream?: boolean }
416451
): string {
417452
const browserType = file.type?.trim()
418-
if (browserType) {
419-
if (options?.preserveOctetStream || browserType !== 'application/octet-stream') {
420-
return browserType
421-
}
422-
}
423-
return getMimeTypeFromExtension(getFileExtension(file.name))
453+
if (browserType && options?.preserveOctetStream) return browserType
454+
return resolveEffectiveMimeType(browserType, file.name)
424455
}
425456

426457
/**
@@ -432,32 +463,6 @@ export function getFileContentType(file: File): string {
432463
return resolveFileType(file, { preserveOctetStream: true })
433464
}
434465

435-
/**
436-
* The MIME type to render a *stored* file as, resolving the generic types that storage
437-
* legitimately holds against the filename.
438-
*
439-
* A stored `application/octet-stream` is not an error — browsers report it for plenty of
440-
* real formats, and the presigned PUT handshake requires persisting it verbatim. Any
441-
* consumer that feeds a stored type to a `Blob`, a media element, or a type filter must
442-
* go through this rather than trusting `file.type` directly: a truthiness check
443-
* (`file.type || fallback`) passes `application/octet-stream` straight through, and a
444-
* media element handed that blob cannot determine the format and renders nothing.
445-
*
446-
* Returns `null` only when neither the stored type nor the extension identifies the file.
447-
*/
448-
export function resolveEffectiveMimeType(
449-
storedType: string | null | undefined,
450-
filename: string
451-
): string | null {
452-
const stored = storedType?.trim()
453-
if (stored && !GENERIC_MIME_TYPES.has(stored)) return stored
454-
455-
const fromExtension = getMimeTypeFromExtension(getFileExtension(filename))
456-
if (!GENERIC_MIME_TYPES.has(fromExtension)) return fromExtension
457-
458-
return stored || null
459-
}
460-
461466
/**
462467
* Whether `error` is a DOM `AbortError` (XHR `abort()`, fetch `signal.aborted`,
463468
* etc). Used in upload retry loops so aborts short-circuit instead of retrying.

0 commit comments

Comments
 (0)