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]}`; +};