From 1591f94c70c93a93fd11331f45ad034661244341 Mon Sep 17 00:00:00 2001 From: risk-alt Date: Mon, 10 Aug 2026 13:29:59 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(frontend)=20warn=20before=20upload?= =?UTF-8?q?ing=20a=20file=20over=20the=20size=20limit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropping a file larger than the allowed size showed a bare "unknown error" in the editor. The proxy in front of the API cuts the request and answers a 413 with an HTML body, so errorCauses threw while parsing it as JSON and no cause ever reached the error panel. The size limit the backend already enforces is now exposed by the config endpoint, and the editor checks the file against it before sending anything, with the same toast wording the document import uses. errorCauses no longer throws on a body it cannot parse, and a 413 without a usable cause falls back to an explicit message, which covers the instances whose proxy limit is lower than the application one. The size formatting duplicated in the import hook moved to a shared util. Signed-off-by: risk-alt --- CHANGELOG.md | 2 + src/backend/core/api/viewsets.py | 1 + src/backend/core/tests/test_api_config.py | 1 + .../e2e/__tests__/app-impress/utils-common.ts | 1 + .../impress/src/api/__tests__/utils.test.ts | 13 ++++ src/frontend/apps/impress/src/api/utils.ts | 19 +++-- .../impress/src/core/config/api/useConfig.tsx | 1 + .../doc-editor/api/useCreateDocUpload.tsx | 14 +++- .../hook/__tests__/useUploadFile.test.tsx | 73 +++++++++++++++++++ .../docs/doc-editor/hook/useUploadFile.tsx | 33 ++++++++- .../docs/doc-management/hooks/useImport.tsx | 12 +-- .../src/utils/__tests__/string.test.ts | 20 ++++- src/frontend/apps/impress/src/utils/string.ts | 18 +++++ 13 files changed, 187 insertions(+), 21 deletions(-) create mode 100644 src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useUploadFile.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 7069b03b19..926962669a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to - ♿️(frontend) restore skip to content link after header redesign #2510 - 🌐(i18n) rename cn_CN to zh_CN, add eo_PL and zh_TW locales #2486 - ✨(backend) conditional email notification in server to server api #2554 +- ✨(backend) expose the attachment max size in the config endpoint #2577 ### Changed @@ -23,6 +24,7 @@ and this project adheres to - 🐛(backend) ignore CSPs for API docs in development - 🐛(frontend) export images embedded with a relative url #2573 - 🐛(y-provider) fix sentry init #2579 +- 🐛(frontend) warn before uploading an attachment over the size limit #2577 ## [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/utils-common.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-common.ts index dccb9e746b..8da982414e 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..fc78dc5c3f 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,19 @@ describe('utils', () => { expect(result.cause).toBeUndefined(); expect(result.data).toBeUndefined(); }); + + it('returns undefined causes if the body is not JSON', async () => { + // A proxy answering before the API does, with an HTML error page + const mockResponse = { + status: 413, + json: () => Promise.reject(new SyntaxError('Unexpected token <')), + } as unknown as Response; + + const result = await errorCauses(mockResponse); + + expect(result.status).toBe(413); + expect(result.cause).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..0c779a35bc 100644 --- a/src/frontend/apps/impress/src/api/utils.ts +++ b/src/frontend/apps/impress/src/api/utils.ts @@ -4,18 +4,27 @@ * This is typically used to parse structured error responses from an API * and normalize them into a consistent format with `status`, `cause`, and optional `data`. * + * Errors raised before the API is reached, by a proxy for instance, come with an HTML body: + * those have no cause to extract, but they must not make this helper throw. + * * @param response - The HTTP response object from `fetch()`. * @param data - Optional custom data to include with the error output. * @returns An object containing: * - `status`: HTTP status code from the response - * - `cause`: A flattened list of error messages, or undefined if no body + * - `cause`: A flattened list of error messages, or undefined if no parsable body * - `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; + + try { + errorsBody = (await response.json()) as Record< + string, + string | string[] + > | null; + } catch { + errorsBody = null; + } 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/api/useCreateDocUpload.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/api/useCreateDocUpload.tsx index 2d605d5dfb..0e0caa751c 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-editor/api/useCreateDocUpload.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/api/useCreateDocUpload.tsx @@ -1,4 +1,5 @@ import { useMutation } from '@tanstack/react-query'; +import { t } from 'i18next'; import { APIError, errorCauses, fetchAPI } from '@/api'; @@ -20,10 +21,15 @@ export const createDocAttachment = async ({ }); if (!response.ok) { - throw new APIError( - 'Failed to upload on the doc', - await errorCauses(response), - ); + const causes = await errorCauses(response); + + // A proxy sitting in front of the API can enforce a lower limit than the application + // does, and answers a 413 with an HTML body carrying no usable cause. + if (response.status === 413 && !causes.cause?.length) { + causes.cause = [t('This file is too large to be uploaded.')]; + } + + throw new APIError('Failed to upload on the doc', causes); } return response.json() as Promise; diff --git a/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useUploadFile.test.tsx b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useUploadFile.test.tsx new file mode 100644 index 0000000000..e267825931 --- /dev/null +++ b/src/frontend/apps/impress/src/features/docs/doc-editor/hook/__tests__/useUploadFile.test.tsx @@ -0,0 +1,73 @@ +import { renderHook } from '@testing-library/react'; +import fetchMock from 'fetch-mock'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { AppWrapper } from '@/tests/utils'; + +import { useUploadFile } from '../useUploadFile'; + +const { mockToast } = vi.hoisted(() => ({ mockToast: vi.fn() })); + +const MAX_FILE_SIZE = 1024; + +vi.mock('@gouvfr-lasuite/cunningham-react', async () => { + const actual = await vi.importActual('@gouvfr-lasuite/cunningham-react'); + return { + ...actual, + useToastProvider: () => ({ toast: mockToast }), + }; +}); + +vi.mock('@/core', async () => { + const actual = await vi.importActual('@/core'); + return { + ...actual, + useConfig: () => ({ data: { DOCUMENT_IMAGE_MAX_SIZE: MAX_FILE_SIZE } }), + }; +}); + +const docId = 'test-doc-id'; +const uploadUrl = `http://test.jest/api/v1.0/documents/${docId}/attachment-upload/`; + +const createFile = (size: number) => + new File(['a'.repeat(size)], 'video.mp4', { type: 'video/mp4' }); + +describe('useUploadFile', () => { + beforeEach(() => { + vi.clearAllMocks(); + fetchMock.hardReset(); + fetchMock.mockGlobal(); + + fetchMock.post(uploadUrl, { + body: { file: `/media/${docId}/attachments/video.mp4` }, + }); + }); + + const renderUseUploadFile = () => + renderHook(() => useUploadFile(docId), { wrapper: AppWrapper }).result; + + it('uploads a file below the size limit', async () => { + const result = renderUseUploadFile(); + + await expect( + result.current.uploadFile(createFile(MAX_FILE_SIZE)), + ).resolves.toContain(`/media/${docId}/attachments/video.mp4`); + + expect(fetchMock.callHistory.calls(uploadUrl).length).toBe(1); + expect(mockToast).not.toHaveBeenCalled(); + }); + + it('rejects a file above the size limit without calling the API', async () => { + const result = renderUseUploadFile(); + + await expect( + result.current.uploadFile(createFile(MAX_FILE_SIZE + 1)), + ).rejects.toThrow('File is too large'); + + expect(fetchMock.callHistory.calls(uploadUrl).length).toBe(0); + expect(mockToast).toHaveBeenCalledWith( + 'The file "video.mp4" is too large. Maximum file size is 1KB.', + 'error', + ); + }); +}); 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..031397fa31 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,24 +1,55 @@ import { Block } from '@blocknote/core'; +import { + VariantType, + useToastProvider, +} from '@gouvfr-lasuite/cunningham-react'; import { captureException } from '@sentry/nextjs'; import { useCallback, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { backendUrl } from '@/api'; +import { useConfig } from '@/core'; +import { formatFileSize } from '@/utils'; import { isSafeUrl } from '@/utils/url'; import { useCreateDocAttachment } from '../api'; import { ANALYZE_URL } from '../conf'; import { DocsBlockNoteEditor } from '../types'; +const DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024; // Default to 10MB + export const useUploadFile = (docId: string) => { + const { t } = useTranslation(); + const { toast } = useToastProvider(); + const { data: config } = useConfig(); const { mutateAsync: createDocAttachment, isError: isErrorAttachment, error: errorAttachment, } = useCreateDocAttachment(); + const maxFileSize = config?.DOCUMENT_IMAGE_MAX_SIZE ?? DEFAULT_MAX_FILE_SIZE; + const uploadFile = useCallback( async (file: File) => { + // The server rejects an oversized file, but the proxy in front of it usually cuts the + // request first and answers a bare 413 the editor cannot make sense of. Telling the + // user before sending anything saves them the wait and the cryptic message. + if (file.size > maxFileSize) { + toast( + t( + 'The file "{{fileName}}" is too large. Maximum file size is {{maxFileSize}}.', + { + fileName: file.name, + maxFileSize: formatFileSize(maxFileSize), + }, + ), + VariantType.ERROR, + ); + + throw new Error('File is too large'); + } + const body = new FormData(); body.append('file', file); @@ -29,7 +60,7 @@ export const useUploadFile = (docId: string) => { return `${backendUrl()}${ret.file}`; }, - [createDocAttachment, docId], + [createDocAttachment, docId, maxFileSize, t, toast], ); return { diff --git a/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useImport.tsx b/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useImport.tsx index 6d9f82d50b..5e7dc55abb 100644 --- a/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useImport.tsx +++ b/src/frontend/apps/impress/src/features/docs/doc-management/hooks/useImport.tsx @@ -7,6 +7,7 @@ import { useCallback, useMemo } from 'react'; import { useDropzone } from 'react-dropzone'; import { useConfig } from '@/core'; +import { formatFileSize } from '@/utils'; import { ContentTypes, useImportDoc } from '../api/useImportDoc'; import { Doc } from '../types'; @@ -27,18 +28,9 @@ export const useImport = ({ onDragOver, onImportSuccess }: UseImportProps) => { const MAX_FILE_SIZE = useMemo(() => { const maxSizeInBytes = config?.CONVERSION_FILE_MAX_SIZE ?? 10 * 1024 * 1024; // Default to 10MB - const units = ['bytes', 'KB', 'MB', 'GB']; - let size = maxSizeInBytes; - let unitIndex = 0; - - while (size >= 1024 && unitIndex < units.length - 1) { - size /= 1024; - unitIndex += 1; - } - return { bytes: maxSizeInBytes, - text: `${Math.round(size * 10) / 10}${units[unitIndex]}`, + text: formatFileSize(maxSizeInBytes), }; }, [config?.CONVERSION_FILE_MAX_SIZE]); diff --git a/src/frontend/apps/impress/src/utils/__tests__/string.test.ts b/src/frontend/apps/impress/src/utils/__tests__/string.test.ts index 3211195487..54a6f86344 100644 --- a/src/frontend/apps/impress/src/utils/__tests__/string.test.ts +++ b/src/frontend/apps/impress/src/utils/__tests__/string.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { isValidEmail } from '../string'; +import { formatFileSize, isValidEmail } from '../string'; describe('isValidEmail', () => { [ @@ -30,3 +30,21 @@ describe('isValidEmail', () => { }); }); }); + +describe('formatFileSize', () => { + [ + { bytes: 0, expected: '0bytes' }, + { bytes: 512, expected: '512bytes' }, + { bytes: 1024, expected: '1KB' }, + { bytes: 1536, expected: '1.5KB' }, + { bytes: 10 * 1024 * 1024, expected: '10MB' }, + { bytes: 20971520, expected: '20MB' }, + { bytes: 3 * 1024 * 1024 * 1024, expected: '3GB' }, + // Larger than the biggest unit, stays in GB + { bytes: 2048 * 1024 * 1024 * 1024, expected: '2048GB' }, + ].forEach(({ bytes, expected }) => { + it(`formats ${bytes} bytes as "${expected}"`, () => { + expect(formatFileSize(bytes)).toBe(expected); + }); + }); +}); diff --git a/src/frontend/apps/impress/src/utils/string.ts b/src/frontend/apps/impress/src/utils/string.ts index d10dd20369..dbe00869bc 100644 --- a/src/frontend/apps/impress/src/utils/string.ts +++ b/src/frontend/apps/impress/src/utils/string.ts @@ -6,3 +6,21 @@ export const isValidEmail = (email: string) => { export const toBase64 = (str: Uint8Array): string => Buffer.from(str).toString('base64'); + +const FILE_SIZE_UNITS = ['bytes', 'KB', 'MB', 'GB'] as const; + +/** + * Turn a number of bytes into a short human readable size, e.g. `10MB`. + * Kept unit-suffixed without a space to match the wording of the size limit messages. + */ +export const formatFileSize = (bytes: number): string => { + let size = bytes; + let unitIndex = 0; + + while (size >= 1024 && unitIndex < FILE_SIZE_UNITS.length - 1) { + size /= 1024; + unitIndex += 1; + } + + return `${Math.round(size * 10) / 10}${FILE_SIZE_UNITS[unitIndex]}`; +};