diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aafeb4ab4..ba74cb5140 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to ### Added - ♿️(frontend) restore skip to content link after header redesign #2510 +- ✨(frontend) warn the user when trying to upload a file size that exceeds the limit #2522 ## [v5.4.1] - 2026-07-09 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 5d9991bcbf..4d7069ef89 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -3085,6 +3085,7 @@ def get(self, request): "CONVERSION_FILE_EXTENSIONS_ALLOWED", "CONVERSION_FILE_MAX_SIZE", "CONVERSION_UPLOAD_ENABLED", + "DOCUMENT_IMAGE_MAX_SIZE", "ENVIRONMENT", "FRONTEND_CSS_URL", "FRONTEND_HOMEPAGE_FEATURE_ENABLED", diff --git a/src/backend/core/tests/test_api_config.py b/src/backend/core/tests/test_api_config.py index 5f7fef4536..192a3cc2f5 100644 --- a/src/backend/core/tests/test_api_config.py +++ b/src/backend/core/tests/test_api_config.py @@ -61,6 +61,7 @@ def test_api_config(is_authenticated): "CONVERSION_FILE_EXTENSIONS_ALLOWED": [".docx", ".md"], "CONVERSION_FILE_MAX_SIZE": 20971520, "CONVERSION_UPLOAD_ENABLED": False, + "DOCUMENT_IMAGE_MAX_SIZE": 10485760, "ENVIRONMENT": "test", "FRONTEND_CSS_URL": "http://testcss/", "FRONTEND_HOMEPAGE_FEATURE_ENABLED": True, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-editor-upload.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor-upload.spec.ts new file mode 100644 index 0000000000..c4a9bfe0d9 --- /dev/null +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-editor-upload.spec.ts @@ -0,0 +1,108 @@ +import { Page, expect, test } from '@playwright/test'; + +import { createDoc, overrideConfig } from './utils-common'; +import { getEditor } from './utils-editor'; + +const dropFileInEditor = async (page: Page) => { + const dataTransfer = await page.evaluateHandle(() => { + const dt = new DataTransfer(); + // 1MB + 1 byte, exceeds the 1MB limit set in overrideConfig. + const file = new File([new Uint8Array(1048577)], 'video.mp4', { + type: 'video/mp4', + }); + dt.items.add(file); + return dt; + }); + + await page + .getByLabel('Document editor') + .dispatchEvent('drop', { dataTransfer }); +}; + +const pasteFileInEditor = async (page: Page) => { + await page.getByLabel('Document editor').focus(); + + await page.getByLabel('Document editor').evaluate((el) => { + const dt = new DataTransfer(); + const file = new File([new Uint8Array(1048577)], 'video.mp4', { + type: 'video/mp4', + }); + dt.items.add(file); + + const event = new ClipboardEvent('paste', { + clipboardData: dt, + bubbles: true, + cancelable: true, + }); + el.dispatchEvent(event); + }); +}; + +test.describe('Doc Editor - File Upload', () => { + test('dropping a file that is too large shows an error and does not leave a loading block', async ({ + page, + browserName, + }) => { + await overrideConfig(page, { DOCUMENT_IMAGE_MAX_SIZE: 1048576 }); // Override the size limit to 1MB for the test. + await page.goto('/'); + await createDoc(page, 'doc-upload-too-large', browserName, 1); + await getEditor({ page }); + + await dropFileInEditor(page); + + await expect( + page.getByText('File size exceeds the maximum allowed size of 1MB.'), + ).toBeVisible(); + + await expect(page.getByText('Loading...')).toBeHidden(); + }); + + test('dismissing the error and dropping the same file again shows the error again', async ({ + page, + browserName, + }) => { + await overrideConfig(page, { DOCUMENT_IMAGE_MAX_SIZE: 1048576 }); // Override the size limit to 1MB for the test. + await page.goto('/'); + await createDoc(page, 'doc-upload-error-retry', browserName, 1); + await getEditor({ page }); + + await dropFileInEditor(page); + await expect( + page.getByText('File size exceeds the maximum allowed size of 1MB.'), + ).toBeVisible(); + + // Dismiss the error + await page.locator('.--docs--text-errors').getByRole('button').click(); + await expect( + page.getByText('File size exceeds the maximum allowed size of 1MB.'), + ).toBeHidden(); + + // Drop the same file again + await dropFileInEditor(page); + await expect( + page.getByText('File size exceeds the maximum allowed size of 1MB.'), + ).toBeVisible(); + }); + + test('pasting a file that is too large shows an error and does not leave a loading block', async ({ + page, + browserName, + }) => { + test.skip( + browserName === 'firefox', + 'Firefox does not expose clipboardData.items on synthetic ClipboardEvents, making this untestable via dispatchEvent.', + ); + await overrideConfig(page, { DOCUMENT_IMAGE_MAX_SIZE: 1048576 }); + await page.goto('/'); + await createDoc(page, 'doc-upload-paste-too-large', browserName, 1); + await getEditor({ page }); + + await pasteFileInEditor(page); + + await expect( + page.getByText('File size exceeds the maximum allowed size of 1MB.'), + ).toBeVisible(); + + await expect(page.getByText('Loading...')).toBeHidden(); + }); +}); diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts index b42785719e..92c8eac061 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts @@ -24,6 +24,7 @@ export const CONFIG = { CONVERSION_UPLOAD_ENABLED: true, CONVERSION_FILE_EXTENSIONS_ALLOWED: ['.docx', '.md'], CONVERSION_FILE_MAX_SIZE: 20971520, + DOCUMENT_IMAGE_MAX_SIZE: 10485760, ENVIRONMENT: 'development', FRONTEND_CSS_URL: null, FRONTEND_JS_URL: null, diff --git a/src/frontend/apps/impress/src/api/__tests__/utils.test.ts b/src/frontend/apps/impress/src/api/__tests__/utils.test.ts index f4f61bad8b..3d3ec9e449 100644 --- a/src/frontend/apps/impress/src/api/__tests__/utils.test.ts +++ b/src/frontend/apps/impress/src/api/__tests__/utils.test.ts @@ -35,6 +35,20 @@ describe('utils', () => { expect(result.cause).toBeUndefined(); expect(result.data).toBeUndefined(); }); + + it('returns undefined causes when response body is not valid JSON (e.g. 413 from Nginx)', async () => { + const mockResponse = { + status: 413, + json: () => + Promise.reject(new SyntaxError('Unexpected token < in JSON')), + } as unknown as Response; + + const result = await errorCauses(mockResponse); + + expect(result.status).toBe(413); + expect(result.cause).toBeUndefined(); + expect(result.data).toBeUndefined(); + }); }); describe('getCSRFToken', () => { diff --git a/src/frontend/apps/impress/src/api/utils.ts b/src/frontend/apps/impress/src/api/utils.ts index 82bbe505ae..0225c3fcfd 100644 --- a/src/frontend/apps/impress/src/api/utils.ts +++ b/src/frontend/apps/impress/src/api/utils.ts @@ -12,10 +12,12 @@ * - `data`: The optional data passed in */ export const errorCauses = async (response: Response, data?: unknown) => { - const errorsBody = (await response.json()) as Record< - string, - string | string[] - > | null; + let errorsBody: Record | null = null; + try { + errorsBody = await response.json(); + } catch { + // response body is not JSON (e.g. HTML error page from Nginx) + } const causes = errorsBody ? Object.entries(errorsBody) diff --git a/src/frontend/apps/impress/src/core/config/api/useConfig.tsx b/src/frontend/apps/impress/src/core/config/api/useConfig.tsx index b204e5a381..aa5ef10cd9 100644 --- a/src/frontend/apps/impress/src/core/config/api/useConfig.tsx +++ b/src/frontend/apps/impress/src/core/config/api/useConfig.tsx @@ -54,6 +54,7 @@ export interface ConfigResponse { CONVERSION_FILE_EXTENSIONS_ALLOWED: string[]; CONVERSION_FILE_MAX_SIZE: number; CONVERSION_UPLOAD_ENABLED?: boolean; + DOCUMENT_IMAGE_MAX_SIZE?: number; ENVIRONMENT: string; FRONTEND_CSS_URL?: string; FRONTEND_HOMEPAGE_FEATURE_ENABLED?: boolean; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx index 5c9b2b1e29..0a359e2574 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteEditor.tsx @@ -111,7 +111,8 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { ? DEFAULT_LOCALE : i18n.resolvedLanguage; - const { uploadFile, errorAttachment } = useUploadFile(doc.id); + const { uploadFile, checkFileSize, errorAttachment, sizeErrorKey } = + useUploadFile(doc.id); const conf = useConfig().data; const { isFeatureFlagActivated } = useAnalytics(); const aiBlockNoteAllowed = !!( @@ -206,6 +207,13 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { }), }, pasteHandler: ({ event, defaultPasteHandler }) => { + const files = Array.from(event.clipboardData?.files ?? []); + try { + files.forEach(checkFileSize); + } catch { + return; + } + // Get clipboard data const blocknoteData = event.clipboardData?.getData('blocknote/html'); @@ -263,6 +271,41 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { useUploadStatus(editor); + useEffect(() => { + const container = refEditorContainer.current; + if (!container) return; + + const handleDrop = (event: DragEvent) => { + const files = Array.from(event.dataTransfer?.files ?? []); + try { + files.forEach(checkFileSize); + } catch { + event.stopPropagation(); + event.preventDefault(); + } + }; + + const handlePaste = (event: ClipboardEvent) => { + const files = Array.from(event.clipboardData?.items ?? []) + .filter((item) => item.kind === 'file') + .map((item) => item.getAsFile()) + .filter((f): f is File => f !== null); + try { + files.forEach(checkFileSize); + } catch { + event.stopPropagation(); + event.preventDefault(); + } + }; + + container.addEventListener('drop', handleDrop, true); + container.addEventListener('paste', handlePaste, true); + return () => { + container.removeEventListener('drop', handleDrop, true); + container.removeEventListener('paste', handlePaste, true); + }; + }, [checkFileSize]); + useEffect(() => { setEditor(editor); @@ -279,7 +322,10 @@ export const BlockNoteEditor = ({ doc, provider }: BlockNoteEditorProps) => { currentUserAvatarUrl={currentUserAvatarUrl} /> {errorAttachment && ( - + ({ + useConfig: () => ({ + data: { DOCUMENT_IMAGE_MAX_SIZE: 1 * 1024 * 1024 }, // 1MB limit for test + }), +})); + +describe('useUploadFile', () => { + // Fake file with a size slightly under the limit + const smallFile = new File( + [new ArrayBuffer(1 * 1024 * 1024 - 1)], + 'big.png', + { + type: 'image/png', + }, + ); + // Fake file with a size slightly over the limit + const bigFile = new File([new ArrayBuffer(1 * 1024 * 1024 + 1)], 'big.png', { + type: 'image/png', + }); + + beforeEach(() => { + fetchMock.restore(); + }); + + it("proceeds to upload when file doesn't exceed the size limit", async () => { + fetchMock.post( + 'http://test.jest/api/v1.0/documents/doc-id/attachment-upload/', + { body: { file: '/media/test.jpg' } }, + ); + const { result } = renderHook(() => useUploadFile('doc-id'), { + wrapper: AppWrapper, + }); + + await result.current.uploadFile(smallFile); + expect(fetchMock.calls()).toHaveLength(1); + }); + + it('throws an APIError before uploading when file exceeds the size limit', async () => { + const { result } = renderHook(() => useUploadFile('doc-id'), { + wrapper: AppWrapper, + }); + + await expect(result.current.uploadFile(bigFile)).rejects.toThrow(APIError); + expect(fetchMock.calls()).toHaveLength(0); + }); + + it('sets errorAttachment with a user-friendly message when file exceeds the size limit', async () => { + const { result } = renderHook(() => useUploadFile('doc-id'), { + wrapper: AppWrapper, + }); + + await result.current.uploadFile(bigFile).catch(() => {}); + + await waitFor(() => { + expect(result.current.isErrorAttachment).toBe(true); + }); + + expect(result.current.errorAttachment?.cause).toEqual([ + 'File size exceeds the maximum allowed size of 1MB.', + ]); + }); + + it('exposes checkFileSize that does not throw for files within the limit', () => { + const { result } = renderHook(() => useUploadFile('doc-id'), { + wrapper: AppWrapper, + }); + + expect(() => result.current.checkFileSize(smallFile)).not.toThrow(); + }); + + it('exposes checkFileSize that throws and sets errorAttachment for files over the limit', async () => { + const { result } = renderHook(() => useUploadFile('doc-id'), { + wrapper: AppWrapper, + }); + + expect(() => result.current.checkFileSize(bigFile)).toThrow(APIError); + + await waitFor(() => { + expect(result.current.isErrorAttachment).toBe(true); + }); + + expect(result.current.errorAttachment?.cause).toEqual([ + 'File size exceeds the maximum allowed size of 1MB.', + ]); + }); +}); diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useUploadFile.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useUploadFile.tsx index 5913f2d559..f8021c500b 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useUploadFile.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/useUploadFile.tsx @@ -1,9 +1,10 @@ import { Block } from '@blocknote/core'; import { captureException } from '@sentry/nextjs'; -import { useCallback, useEffect } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { backendUrl } from '@/api'; +import { APIError, backendUrl } from '@/api'; +import { useConfig } from '@/core'; import { isSafeUrl } from '@/utils/url'; import { useCreateDocAttachment } from '../api'; @@ -11,14 +12,41 @@ import { ANALYZE_URL } from '../conf'; import { DocsBlockNoteEditor } from '../types'; export const useUploadFile = (docId: string) => { + const { t } = useTranslation(); + const { data: config } = useConfig(); + const [sizeError, setSizeError] = useState(null); + const [sizeErrorKey, setSizeErrorKey] = useState(0); const { mutateAsync: createDocAttachment, isError: isErrorAttachment, error: errorAttachment, } = useCreateDocAttachment(); + const checkFileSize = useCallback( + (file: File) => { + const maxSize = config?.DOCUMENT_IMAGE_MAX_SIZE ?? 10 * 1024 * 1024; // Default to 10MB if config isn't provided by the backend. + if (file.size > maxSize) { + const error = new APIError(t('File is too large'), { + status: 413, // Replicate what Nginx answers when dealing with a file too big. + cause: [ + t('File size exceeds the maximum allowed size of {{size}}MB.', { + size: Math.round(maxSize / (1024 * 1024)), + }), + ], + }); + setSizeError(error); + setSizeErrorKey((prev) => prev + 1); + throw error; + } + setSizeError(null); + }, + [config?.DOCUMENT_IMAGE_MAX_SIZE, setSizeError, setSizeErrorKey, t], + ); + const uploadFile = useCallback( async (file: File) => { + checkFileSize(file); + const body = new FormData(); body.append('file', file); @@ -29,13 +57,15 @@ export const useUploadFile = (docId: string) => { return `${backendUrl()}${ret.file}`; }, - [createDocAttachment, docId], + [checkFileSize, createDocAttachment, docId], ); return { uploadFile, - isErrorAttachment, - errorAttachment, + checkFileSize, + isErrorAttachment: isErrorAttachment || !!sizeError, + errorAttachment: sizeError ?? errorAttachment, + sizeErrorKey, }; };