From cf53b5cbb8469b4fb5a17c7888a3c8a2b3ca779b Mon Sep 17 00:00:00 2001 From: dai123x Date: Tue, 6 Oct 2026 07:51:41 +0900 Subject: [PATCH] fix: skip unsupported files in a batch instead of rejecting the whole upload --- .../Files/__tests__/useFileHandling.test.ts | 68 ++++++++++++++++ client/src/hooks/Files/useFileHandling.ts | 21 ++++- client/src/locales/en/translation.json | 1 + .../src/utils/__tests__/validateFiles.spec.ts | 69 +++++++++++++++- client/src/utils/files.ts | 78 ++++++++++++++----- 5 files changed, 213 insertions(+), 24 deletions(-) diff --git a/client/src/hooks/Files/__tests__/useFileHandling.test.ts b/client/src/hooks/Files/__tests__/useFileHandling.test.ts index efee9673914..c021309a257 100644 --- a/client/src/hooks/Files/__tests__/useFileHandling.test.ts +++ b/client/src/hooks/Files/__tests__/useFileHandling.test.ts @@ -605,6 +605,74 @@ describe('useFileHandling', () => { } }); + it('uploads the rest of a batch when one file has an unsupported type', async () => { + jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); + try { + mockFileConfig = mergeFileConfig({ + endpoints: { default: { fileSizeLimit: 20, totalSizeLimit: 500 } }, + }); + const useFileHandling = await loadHook(); + const { result } = renderHook(() => useFileHandling()); + + let accepted: boolean | undefined; + await act(async () => { + accepted = await result.current.handleFiles([ + makeSizedFile('photo.png', 'image/png', 1 * megabyte), + makeSizedFile('setup.exe', 'application/x-msdownload', 2 * megabyte), + ]); + }); + await act(async () => { + jest.advanceTimersByTime(250); + }); + + expect(accepted).toBe(true); + expect(mockMutate).toHaveBeenCalledTimes(1); + expect(mockMutate.mock.calls[0][0].get('file').name).toBe('photo.png'); + expect(mockLocalize).toHaveBeenCalledWith('com_error_files_skipped_unsupported', { + 0: 'setup.exe', + }); + } finally { + jest.useRealTimers(); + } + }); + + it('rejects the whole batch when nothing in it is supported', async () => { + jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); + try { + mockFileConfig = mergeFileConfig({ + endpoints: { default: { fileSizeLimit: 20, totalSizeLimit: 500 } }, + }); + mockValidateFiles.mockImplementation(jest.requireActual('~/utils/files').validateFiles); + const useFileHandling = await loadHook(); + const { result } = renderHook(() => useFileHandling()); + + let accepted: boolean | undefined; + await act(async () => { + accepted = await result.current.handleFiles([ + makeSizedFile('setup.exe', 'application/x-msdownload', 1 * megabyte), + makeSizedFile('run.exe', 'application/x-msdownload', 2 * megabyte), + ]); + }); + await act(async () => { + jest.advanceTimersByTime(250); + }); + + expect(accepted).toBe(false); + expect(mockMutate).not.toHaveBeenCalled(); + expect(mockLocalize).not.toHaveBeenCalledWith( + 'com_error_files_skipped_unsupported', + expect.anything(), + ); + expect(mockShowToast).toHaveBeenCalledWith({ + message: 'Unsupported file type: application/x-msdownload', + status: 'error', + duration: 5000, + }); + } finally { + jest.useRealTimers(); + } + }); + it('does not announce skipped files when the batch is rejected anyway', async () => { jest.useFakeTimers({ doNotFake: ['queueMicrotask'] }); try { diff --git a/client/src/hooks/Files/useFileHandling.ts b/client/src/hooks/Files/useFileHandling.ts index 3253f4b095a..406c4d59f1c 100644 --- a/client/src/hooks/Files/useFileHandling.ts +++ b/client/src/hooks/Files/useFileHandling.ts @@ -267,11 +267,16 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil } const duplicates: string[] = []; const oversized: string[] = []; + const unsupported: string[] = []; for (const { file, reason } of skipped) { if (reason === 'duplicate') { duplicates.push(file.name); continue; } + if (reason === 'unsupported') { + unsupported.push(file.name); + continue; + } oversized.push(file.name); } if (duplicates.length > 0) { @@ -285,6 +290,9 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil }), ); } + if (unsupported.length > 0) { + setError(localize('com_error_files_skipped_unsupported', { 0: unsupported.join(', ') })); + } }; const { addFile, replaceFile, updateFileById, deleteFileById } = useUpdateFiles(fileSetter); const { isConfigPending, waitForConfig, resizeImageIfNeeded } = useClientResize(); @@ -724,14 +732,17 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil return withoutSource; })(); - /** Drop duplicates one by one rather than rejecting everything picked alongside them, and hand - * the survivors to `validateFiles`. Sizes wait for the partition below, once processing has - * settled each file's final bytes, which is also where the file count is finally applied: a - * file still headed for the discard pile must not spend a `fileLimit` slot. */ + /** Drop duplicates and files the endpoint cannot accept one by one rather than rejecting + * everything picked alongside them, and hand the survivors to `validateFiles`. Sizes wait for + * the partition below, once processing has settled each file's final bytes, which is also where + * the file count is finally applied: a file still headed for the discard pile must not spend a + * `fileLimit` slot. */ const selection = partitionUploads({ files: filesForValidation, fileList, endpointFileConfig, + fileConfig: currentFileConfig, + toolResource: _toolResource, skipSizeValidation: true, }); /** Nothing survived, so the whole selection is rejected and the untouched list reports it @@ -966,6 +977,8 @@ const useFileHandlingCore = (params: UseFileHandling | undefined, fileState: Fil files: filesForValidation, fileList: processedFileList, endpointFileConfig, + fileConfig: currentFileConfig, + toolResource: _toolResource, }); acceptedUploads = batch.keptIndices.map((index) => eligibleUploads[index]); const acceptedFiles = acceptedUploads.map(({ extendedFile }) => extendedFile.file as File); diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index df36d52e6e0..141d4afca38 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -668,6 +668,7 @@ "com_error_files_process": "An error occurred while processing the file.", "com_error_files_skipped_dupe": "Skipped duplicate file(s): {{0}}", "com_error_files_skipped_size": "Skipped file(s) over the {{0}} MB size limit: {{1}}", + "com_error_files_skipped_unsupported": "Skipped unsupported file(s): {{0}}", "com_error_files_unsupported": "This file type can't be attached here.", "com_error_files_upload": "An error occurred while uploading the file.", "com_error_files_upload_canceled": "The file upload request was canceled. Note: the file upload may still be processing and will need to be manually deleted.", diff --git a/client/src/utils/__tests__/validateFiles.spec.ts b/client/src/utils/__tests__/validateFiles.spec.ts index 68fac8f9917..7f7d84a4af0 100644 --- a/client/src/utils/__tests__/validateFiles.spec.ts +++ b/client/src/utils/__tests__/validateFiles.spec.ts @@ -1,4 +1,4 @@ -import { megabyte, fileConfig as defaultFileConfig } from 'librechat-data-provider'; +import { megabyte, fileConfig as defaultFileConfig, EToolResources } from 'librechat-data-provider'; import type { EndpointFileConfig, FileConfig } from 'librechat-data-provider'; import type { ExtendedFile } from '~/common'; import { validateFiles, validateFileSizes, partitionUploads } from '../files'; @@ -400,4 +400,71 @@ describe('partitionUploads', () => { expect(keptIndices).toEqual([0, 1]); expect(skipped).toEqual([]); }); + + it('skips an unsupported file and keeps the supported ones picked alongside it', () => { + const fileList = [ + makeSizedFile('photo.png', 'image/png', 1024), + makeSizedFile('notes.txt', 'text/plain', 1024), + makeSizedFile('setup.exe', 'application/x-msdownload', 1024), + ]; + + const { keptIndices, skipped } = partitionUploads({ files, fileList, endpointFileConfig }); + + expect(keptIndices).toEqual([0, 1]); + expect(skipped).toEqual([{ index: 2, file: fileList[2], reason: 'unsupported' }]); + }); + + it('skips every file when none of them has a supported type', () => { + const fileList = [ + makeSizedFile('one.exe', 'application/x-msdownload', 1024), + makeSizedFile('two.exe', 'application/x-msdownload', 1024), + ]; + + const { keptIndices, skipped } = partitionUploads({ files, fileList, endpointFileConfig }); + + expect(keptIndices).toEqual([]); + expect(skipped.map(({ reason }) => reason)).toEqual(['unsupported', 'unsupported']); + }); + + it('re-types a file whose type is inferred from its extension and keeps it', () => { + const fileList = [makeSizedFile('archive.zip', '', 1024)]; + + const { keptIndices, skipped } = partitionUploads({ files, fileList, endpointFileConfig }); + + expect(keptIndices).toEqual([0]); + expect(skipped).toEqual([]); + expect(fileList[0].type).toBe('application/zip'); + }); + + it('skips a file whose type cannot be determined without dropping the rest', () => { + const fileList = [ + makeSizedFile('mystery', '', 1024), + makeSizedFile('notes.txt', 'text/plain', 1024), + ]; + + const { keptIndices, skipped } = partitionUploads({ files, fileList, endpointFileConfig }); + + expect(keptIndices).toEqual([1]); + expect(skipped).toEqual([{ index: 0, file: fileList[0], reason: 'unsupported' }]); + }); + + it('checks context tool resources against the configured text, OCR, and STT lists', () => { + endpointFileConfig = makeEndpointConfig({ supportedMimeTypes: [/^image\/(jpeg|png)$/] }); + const fileConfig: FileConfig = { + endpoints: {}, + ocr: { supportedMimeTypes: [/^application\/pdf$/] }, + }; + const fileList = [makeSizedFile('doc.pdf', 'application/pdf', 1024)]; + + const { keptIndices, skipped } = partitionUploads({ + files, + fileList, + endpointFileConfig, + fileConfig, + toolResource: EToolResources.context, + }); + + expect(keptIndices).toEqual([0]); + expect(skipped).toEqual([]); + }); }); diff --git a/client/src/utils/files.ts b/client/src/utils/files.ts index b846996b092..8a7d544b619 100644 --- a/client/src/utils/files.ts +++ b/client/src/utils/files.ts @@ -330,7 +330,7 @@ export const validateFileLimit = ({ return true; }; -export type UploadSkipReason = 'duplicate' | 'fileSize'; +export type UploadSkipReason = 'duplicate' | 'fileSize' | 'unsupported'; export type SkippedUpload = { /** Position in the `fileList` handed to `partitionUploads`, so callers can map back to their own parallel arrays */ @@ -344,25 +344,59 @@ export type UploadPartition = { skipped: SkippedUpload[]; }; +/** The MIME allowlist a file must match: the endpoint's own list, or — for context (RAG) tool + * resources — what the configured text, OCR, and STT endpoints accept. Unified mode routes by + * MIME type but does not widen what may be uploaded: this stays the same ceiling the server + * enforces in `filterFile`, so accepting extraction-capable types beyond it only turns a + * preflight message into a failed request. */ +const getMimeTypesToCheck = ({ + endpointFileConfig, + fileConfig, + toolResource, +}: { + endpointFileConfig: EndpointFileConfig; + fileConfig: FileConfig | null; + toolResource?: string; +}): RegexLike[] => { + if (toolResource === EToolResources.context) { + return [ + ...(fileConfig?.text?.supportedMimeTypes || []), + ...(fileConfig?.ocr?.supportedMimeTypes || []), + ...(fileConfig?.stt?.supportedMimeTypes || []), + ]; + } + return endpointFileConfig.supportedMimeTypes ?? []; +}; + /** * Splits a selection into the files that may be uploaded and the ones that cannot, so a single * offender no longer rejects everything picked alongside it. Duplicates are matched against files - * already attached and against earlier entries in the same selection. Only per-file rules belong - * here: `totalSizeLimit` is a property of the batch as a whole, so callers still run - * `validateFileSizes` over whatever survives. + * already attached and against earlier entries in the same selection. Type and size are per-file + * rules checked here; `totalSizeLimit` is a property of the batch as a whole, so callers still run + * `validateFileSizes` over whatever survives. Files whose type must be inferred are replaced in + * `fileList` with the re-typed copy, mirroring `validateFiles`. */ export const partitionUploads = ({ files, fileList, endpointFileConfig, + fileConfig, + toolResource, skipSizeValidation = false, }: { fileList: File[]; files: Map; endpointFileConfig: EndpointFileConfig; + fileConfig?: FileConfig | null; + toolResource?: string; skipSizeValidation?: boolean; }): UploadPartition => { const fileSizeLimit = skipSizeValidation ? null : getFileSizeLimit(endpointFileConfig); + const mimeTypesToCheck = getMimeTypesToCheck({ + endpointFileConfig, + fileConfig: fileConfig ?? null, + toolResource, + }); const keptIndices: number[] = []; const skipped: SkippedUpload[] = []; @@ -378,7 +412,7 @@ export const partitionUploads = ({ } for (let i = 0; i < fileList.length; i++) { - const file = fileList[i]; + let file = fileList[i]; const signature = getFileSignature(file.name, file.size, file.type); if (signatures.has(signature)) { skipped.push({ index: i, file, reason: 'duplicate' }); @@ -391,6 +425,20 @@ export const partitionUploads = ({ continue; } + const fileType = inferMimeType(file.name, file.type); + if (!fileType) { + skipped.push({ index: i, file, reason: 'unsupported' }); + continue; + } + if (file.type !== fileType) { + file = new File([file], file.name, { type: fileType }); + fileList[i] = file; + } + if (!checkType(file.type, mimeTypesToCheck)) { + skipped.push({ index: i, file, reason: 'unsupported' }); + continue; + } + keptIndices.push(i); } @@ -449,7 +497,7 @@ export const validateFiles = ({ * whole — the file count and duplicates — wait until it has dropped what it can */ skipBatchRules?: boolean; }) => { - const { supportedMimeTypes, disabled } = endpointFileConfig; + const { disabled } = endpointFileConfig; /** Block all uploads if the endpoint is explicitly disabled */ if (disabled === true) { setError('com_ui_attach_error_disabled'); @@ -482,19 +530,11 @@ export const validateFiles = ({ fileList[i] = newFile; } - /* Unified mode routes by MIME type but does not widen what may be uploaded: the - * endpoint allowlist is the same ceiling the server enforces in `filterFile`, so - * accepting extraction-capable types beyond it only turns a preflight message into - * a failed request. */ - let mimeTypesToCheck = supportedMimeTypes; - if (toolResource === EToolResources.context) { - mimeTypesToCheck = [ - ...(fileConfig?.text?.supportedMimeTypes || []), - ...(fileConfig?.ocr?.supportedMimeTypes || []), - ...(fileConfig?.stt?.supportedMimeTypes || []), - ]; - } - + const mimeTypesToCheck = getMimeTypesToCheck({ + endpointFileConfig, + fileConfig, + toolResource, + }); if (!checkType(originalFile.type, mimeTypesToCheck)) { setError(`Unsupported file type: ${originalFile.type}`); return false;