From 4f7992917be1937488cfae3fd9eeb6acec2b3d07 Mon Sep 17 00:00:00 2001 From: Bastian Seipp Date: Thu, 13 Nov 2025 23:55:58 +0100 Subject: [PATCH 1/2] fix: correct file_id extraction in markdown file download links The markdown link handler was incorrectly extracting file_id and filename from URLs in the format files/{userId}/{file_id}. The .pop() method was applied twice in reverse order, causing the userId to be assigned to the file_id variable and the actual file_id to be used as filename. This caused downloads to fail with 404 errors as the API was called with /api/files/download/{userId}/{userId} instead of the correct /api/files/download/{userId}/{file_id}. Fixed by directly accessing parts[2] to get the file_id and using the link text (children) as the filename. Also added children to the useMemo dependency array to properly track changes. --- .../Chat/Messages/Content/MarkdownComponents.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx b/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx index ed69c677b2..5d08c980d0 100644 --- a/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx +++ b/client/src/components/Chat/Messages/Content/MarkdownComponents.tsx @@ -90,12 +90,13 @@ export const a: React.ElementType = memo(({ href, children }: TAnchorProps) => { if (match && match[0]) { const path = match[0]; const parts = path.split('/'); - const name = parts.pop(); - const file_id = parts.pop(); - return { file_id, filename: name, filepath: path }; + // parts = ['files', userId, file_id] or ['outputs', userId, file_id] + const file_id = parts[2]; // Get the file_id (third element) + const filename = typeof children === 'string' ? children : file_id; // Use link text as filename + return { file_id, filename, filepath: path }; } return { file_id: '', filename: '', filepath: '' }; - }, [user?.id, href]); + }, [user?.id, href, children]); const { refetch: downloadFile } = useFileDownload(user?.id ?? '', file_id); const props: { target?: string; onClick?: React.MouseEventHandler } = { target: '_new' }; From 61a8d81608187ed36b3a80454ad9f48829f6452a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 14 Nov 2025 13:35:51 -0500 Subject: [PATCH 2/2] ci: Add unit tests for file_id extraction in Markdown link component Introduced a new test suite for the Markdown link component to verify the correct extraction of file_id from markdown file links. The tests cover various scenarios, including standard file paths, alternative output paths, and edge cases with UUID-style file_ids. This ensures that the file_id is accurately passed to the download handler, addressing a previously identified bug. --- .../__tests__/MarkdownComponents.test.tsx | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) create mode 100644 client/src/components/Chat/Messages/Content/__tests__/MarkdownComponents.test.tsx diff --git a/client/src/components/Chat/Messages/Content/__tests__/MarkdownComponents.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/MarkdownComponents.test.tsx new file mode 100644 index 0000000000..875db8c52a --- /dev/null +++ b/client/src/components/Chat/Messages/Content/__tests__/MarkdownComponents.test.tsx @@ -0,0 +1,229 @@ +/* eslint-disable i18next/no-literal-string */ +import React from 'react'; +import { render } from '@testing-library/react'; +import { RecoilRoot } from 'recoil'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { a } from '../MarkdownComponents'; +import * as useFileDownloadModule from '~/data-provider'; +import * as useToastContextModule from '@librechat/client'; +import * as useLocalizeModule from '~/hooks'; +import { dataService } from 'librechat-data-provider'; + +// Mock all the complex dependencies +jest.mock('~/components/Messages/Content/CodeBlock', () => ({ + __esModule: true, + default: () =>
CodeBlock
, +})); + +jest.mock('~/hooks/Roles/useHasAccess', () => ({ + __esModule: true, + default: jest.fn(() => true), +})); + +jest.mock('~/Providers', () => ({ + useCodeBlockContext: jest.fn(() => ({ + getNextIndex: jest.fn(() => 0), + resetCounter: jest.fn(), + })), +})); + +jest.mock('~/utils', () => ({ + handleDoubleClick: jest.fn(), +})); + +jest.mock('~/store', () => ({ + user: null, +})); + +jest.mock('~/data-provider', () => ({ + useFileDownload: jest.fn(), +})); + +jest.mock('@librechat/client', () => ({ + useToastContext: jest.fn(), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: jest.fn(), +})); + +jest.mock('librechat-data-provider', () => ({ + dataService: { + getDomainServerBaseUrl: jest.fn(), + }, + PermissionTypes: {}, + Permissions: {}, + fileConfig: { + checkType: jest.fn(), + }, +})); + +jest.mock('recoil', () => ({ + ...jest.requireActual('recoil'), + useRecoilValue: jest.fn(), +})); + +import { useRecoilValue } from 'recoil'; + +describe('MarkdownComponents - Link (a) Component', () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); + + const mockUser = { + id: 'user-123', + username: 'testuser', + }; + + const mockDownloadFile = jest.fn(); + const mockShowToast = jest.fn(); + const mockLocalize = jest.fn((key) => key); + + beforeEach(() => { + jest.clearAllMocks(); + + // Setup mocks + (useRecoilValue as jest.Mock).mockReturnValue(mockUser); + (useToastContextModule.useToastContext as jest.Mock).mockReturnValue({ + showToast: mockShowToast, + }); + (useLocalizeModule.useLocalize as jest.Mock).mockReturnValue(mockLocalize); + (dataService.getDomainServerBaseUrl as jest.Mock).mockReturnValue('http://localhost:3080'); + (useFileDownloadModule.useFileDownload as jest.Mock).mockReturnValue({ + refetch: mockDownloadFile, + }); + }); + + /** + * This test verifies the file_id extraction bug in markdown file links. + * + * BUG: The current implementation uses .pop() twice which extracts values in reverse order: + * - URL format: files/{userId}/{fileId} (3 parts total) + * - parts = ['files', 'user-123', 'file-abc-123'] + * - parts.pop() returns 'file-abc-123' (assigned to filename) - should be file_id! + * - parts.pop() returns 'user-123' (assigned to file_id) - this is the userId! + * + * EXPECTED: useFileDownload should be called with file_id='file-abc-123' + * ACTUAL (BUG): useFileDownload is called with file_id='user-123' (the userId!) + * + * This test will FAIL with the current buggy code and PASS after the fix. + */ + it('should extract correct file_id from markdown file link path', () => { + const testFileId = 'file-abc-123'; + // The URL format is: files/{userId}/{fileId} - no separate filename! + const testHref = `files/${mockUser.id}/${testFileId}`; + + const AnchorComponent = a as React.ComponentType<{ href: string; children: React.ReactNode }>; + + render( + + + Download Test File + + , + ); + + // Verify that useFileDownload was called with the CORRECT file_id + // Currently, the bug causes it to be called with userId instead + expect(useFileDownloadModule.useFileDownload).toHaveBeenCalledWith( + mockUser.id, + testFileId, // This is what we EXPECT (correct behavior) + // Currently the code passes userId here instead (bug!) + ); + }); + + /** + * Additional test: Verify the download handler uses correct file_id + * This test verifies the bug from a different angle - checking the download function call + */ + it('should use correct file_id when download is triggered', () => { + const testFileId = 'file-xyz-789'; + const testHref = `files/${mockUser.id}/${testFileId}`; + + const AnchorComponent = a as React.ComponentType<{ href: string; children: React.ReactNode }>; + + render( + + + Download File + + , + ); + + // Verify useFileDownload hook was initialized with correct file_id + expect(useFileDownloadModule.useFileDownload).toHaveBeenCalledWith( + mockUser.id, + testFileId, // Expected: correct file_id + // Bug: currently passes userId here + ); + }); + + /** + * Test with 'outputs' prefix (alternative path format) + */ + it('should extract correct file_id from outputs path', () => { + const testFileId = 'output-file-456'; + const testHref = `outputs/${mockUser.id}/${testFileId}`; + + const AnchorComponent = a as React.ComponentType<{ href: string; children: React.ReactNode }>; + + render( + + + Download Output + + , + ); + + expect(useFileDownloadModule.useFileDownload).toHaveBeenCalledWith( + mockUser.id, + testFileId, // Should be the actual file_id, not userId + ); + }); + + /** + * Edge case: UUID-style file_id + */ + it('should handle UUID-style file IDs', () => { + const testFileId = '550e8400-e29b-41d4-a716-446655440000'; + const testHref = `files/${mockUser.id}/${testFileId}`; + + const AnchorComponent = a as React.ComponentType<{ href: string; children: React.ReactNode }>; + + render( + + + Download + + , + ); + + expect(useFileDownloadModule.useFileDownload).toHaveBeenCalledWith(mockUser.id, testFileId); + }); + + /** + * Control test: Non-file links should not trigger file download logic + */ + it('should not process regular external links', () => { + const testHref = 'https://example.com/page'; + + const AnchorComponent = a as React.ComponentType<{ href: string; children: React.ReactNode }>; + + render( + + + External Link + + , + ); + + // Should be called with empty strings for non-file links + expect(useFileDownloadModule.useFileDownload).toHaveBeenCalledWith( + mockUser.id, + '', // No file_id for regular links + ); + }); +});