From 7221c29c1e57acad67e3096d1c76b96665de8cd8 Mon Sep 17 00:00:00 2001 From: Johannes Fleck Date: Thu, 12 Nov 2026 15:08:03 +0100 Subject: [PATCH] fix(storage): Show the binary fallback for files the server marks as not renderable For content types in KNOWN_BINARY_TYPES the preview endpoint returns an empty body with X-Preview-Renderable: false. The preview modal ignored that header, decoded the empty body as valid UTF-8 and showed an empty, read-only text editor instead of the binary fallback. Check the header right after the 7z/rar checks and show the fallback. The unit test mock now sends the real binaryPreview() headers, and an E2E test covers an application/avro object. --- e2e/storage/preview.spec.ts | 32 +++++++++ .../storage/modals/PreviewModal.svelte | 7 ++ .../modals/PreviewModal.svelte.spec.ts | 69 ++++++++++++++----- 3 files changed, 90 insertions(+), 18 deletions(-) diff --git a/e2e/storage/preview.spec.ts b/e2e/storage/preview.spec.ts index 53248485..c98310de 100644 --- a/e2e/storage/preview.spec.ts +++ b/e2e/storage/preview.spec.ts @@ -90,6 +90,38 @@ test.describe('Storage S3 — Preview', () => { } }); + test('shows binary fallback preview for content types the server knows to be binary', async ({ + page + }, testInfo) => { + const credentials = requireGarageCredentials(); + const client = createS3Client(credentials); + const prefix = uniquePrefix(testInfo, 'preview-known-bin'); + const cleanupKeys = [`${prefix}events.avro`]; + + try { + await client.send( + new PutObjectCommand({ + Bucket: credentials.bucket, + Key: `${prefix}events.avro`, + // Avro object container magic bytes + Body: Buffer.from([0x4f, 0x62, 0x6a, 0x01, 0x00, 0x00]), + ContentType: 'application/avro' + }) + ); + + await connectAndOpenPrefix(page, credentials, prefix); + + await rowByName(page, 'events.avro').dblclick(); + + await expect(page.getByRole('heading', { name: 'events.avro' })).toBeVisible(); + await expect(page.getByText('Binary content')).toBeVisible(); + await expect(page.getByTestId('editor-readonly-notice')).toHaveCount(0); + await expect(page.getByRole('dialog').getByRole('application')).toHaveCount(0); + } finally { + await deleteKnownKeys(client, credentials.bucket, cleanupKeys); + } + }); + test('shows a PDF preview for PDF files', async ({ page }, testInfo) => { const credentials = requireGarageCredentials(); const client = createS3Client(credentials); diff --git a/src/lib/components/storage/modals/PreviewModal.svelte b/src/lib/components/storage/modals/PreviewModal.svelte index 171c5b76..d09def32 100644 --- a/src/lib/components/storage/modals/PreviewModal.svelte +++ b/src/lib/components/storage/modals/PreviewModal.svelte @@ -250,6 +250,13 @@ return; } + // The server sends no body for content types it knows to be binary. + if (res.headers.get('X-Preview-Renderable') === 'false') { + await res.body?.cancel(); + preview = { kind: 'fallback', contentType, isBinary: true }; + return; + } + if (contentType.startsWith('image/')) { if (truncated) { await res.body?.cancel(); diff --git a/src/lib/components/storage/modals/PreviewModal.svelte.spec.ts b/src/lib/components/storage/modals/PreviewModal.svelte.spec.ts index f41040b3..f09b7b34 100644 --- a/src/lib/components/storage/modals/PreviewModal.svelte.spec.ts +++ b/src/lib/components/storage/modals/PreviewModal.svelte.spec.ts @@ -47,6 +47,20 @@ function mockFetchResponse( etag?: string; } ): Response { + if (opts?.renderable === false) { + // Mirrors binaryPreview() in $lib/server/storage/preview/binary.ts. + return new Response(null, { + status: 200, + headers: { + 'Content-Type': opts.contentType ?? 'application/octet-stream', + 'X-Preview-Renderable': 'false', + 'X-Preview-Total-Size': String(opts.totalSize ?? 11), + 'X-Preview-Bytes': '0', + 'X-Preview-Truncated': 'false', + 'Cache-Control': 'no-store' + } + }); + } return new Response(body, { status: 200, headers: { @@ -665,22 +679,35 @@ describe('PreviewModal basics', () => { .not.toBeInTheDocument(); }); - it('should render fallback when X-Preview-Renderable is false', async () => { - vi.stubGlobal( - 'fetch', - vi.fn().mockResolvedValue( - mockFetchResponse('', { - contentType: 'application/zip', - renderable: false - }) - ) - ); - render(PreviewModal, { ...defaultProps, objectKey: 'archive.zip' }); - - // Fallback should not show a footer with download/close buttons - // Wait for the preview to load - await expect.element(page.getByText('archive.zip')).toBeInTheDocument(); - }); + it.each([ + ['data/events.avro', 'application/avro'], + ['logs/app.log.gz', 'application/gzip'], + ['reports/q3.xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['archive.zip', 'application/zip'] + ])( + 'should render the binary fallback when X-Preview-Renderable is false (%s)', + async (objectKey, contentType) => { + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValue( + mockFetchResponse(null, { contentType, renderable: false, totalSize: 4096 }) + ) + ); + render(PreviewModal, { ...defaultProps, objectKey }); + + await expect + .element( + page.getByText( + 'This file appears to contain binary data and cannot be displayed as text.' + ) + ) + .toBeInTheDocument(); + await expect.element(page.getByTestId('editor-readonly-notice')).not.toBeInTheDocument(); + await expect.element(page.getByRole('application')).not.toBeInTheDocument(); + } + ); it('should render fallback for binary content that fails UTF-8 decode', async () => { // Create bytes that are invalid UTF-8 (not a CSV/TSV extension) @@ -703,8 +730,14 @@ describe('PreviewModal basics', () => { ); render(PreviewModal, { ...defaultProps, objectKey: 'data/file.bin' }); - // Should eventually show fallback (no footer since fallback kind) - await expect.element(page.getByText('file.bin')).toBeInTheDocument(); + await expect + .element( + page.getByText( + 'This file appears to contain binary data and cannot be displayed as text.' + ) + ) + .toBeInTheDocument(); + await expect.element(page.getByRole('application')).not.toBeInTheDocument(); }); });