Skip to content

Commit 0f4222a

Browse files
authored
🪞 fix: Prevent Revoked Blob URLs in Uploaded Images (FileRow) (danny-avila#10361)
1 parent 772b706 commit 0f4222a

2 files changed

Lines changed: 348 additions & 1 deletion

File tree

client/src/components/Chat/Input/Files/FileRow.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ export default function FileRow({
133133
>
134134
{isImage ? (
135135
<Image
136-
url={file.preview ?? file.filepath}
136+
url={file.progress === 1 ? file.filepath : (file.preview ?? file.filepath)}
137137
onDelete={handleDelete}
138138
progress={file.progress}
139139
source={file.source}
Lines changed: 347 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,347 @@
1+
import React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import '@testing-library/jest-dom';
4+
import { FileSources } from 'librechat-data-provider';
5+
import type { ExtendedFile } from '~/common';
6+
import FileRow from '../FileRow';
7+
8+
jest.mock('~/hooks', () => ({
9+
useLocalize: jest.fn(),
10+
}));
11+
12+
jest.mock('~/data-provider', () => ({
13+
useDeleteFilesMutation: jest.fn(),
14+
}));
15+
16+
jest.mock('~/hooks/Files', () => ({
17+
useFileDeletion: jest.fn(),
18+
}));
19+
20+
jest.mock('~/utils', () => ({
21+
logger: {
22+
log: jest.fn(),
23+
},
24+
}));
25+
26+
jest.mock('../Image', () => {
27+
return function MockImage({ url, progress, source }: any) {
28+
return (
29+
<div data-testid="mock-image">
30+
<span data-testid="image-url">{url}</span>
31+
<span data-testid="image-progress">{progress}</span>
32+
<span data-testid="image-source">{source}</span>
33+
</div>
34+
);
35+
};
36+
});
37+
38+
jest.mock('../FileContainer', () => {
39+
return function MockFileContainer({ file }: any) {
40+
return (
41+
<div data-testid="mock-file-container">
42+
<span data-testid="file-name">{file.filename}</span>
43+
</div>
44+
);
45+
};
46+
});
47+
48+
const mockUseLocalize = jest.requireMock('~/hooks').useLocalize;
49+
const mockUseDeleteFilesMutation = jest.requireMock('~/data-provider').useDeleteFilesMutation;
50+
const mockUseFileDeletion = jest.requireMock('~/hooks/Files').useFileDeletion;
51+
52+
describe('FileRow', () => {
53+
const mockSetFiles = jest.fn();
54+
const mockSetFilesLoading = jest.fn();
55+
const mockDeleteFile = jest.fn();
56+
57+
beforeEach(() => {
58+
jest.clearAllMocks();
59+
60+
mockUseLocalize.mockReturnValue((key: string) => {
61+
const translations: Record<string, string> = {
62+
com_ui_deleting_file: 'Deleting file...',
63+
};
64+
return translations[key] || key;
65+
});
66+
67+
mockUseDeleteFilesMutation.mockReturnValue({
68+
mutateAsync: jest.fn(),
69+
});
70+
71+
mockUseFileDeletion.mockReturnValue({
72+
deleteFile: mockDeleteFile,
73+
});
74+
});
75+
76+
/**
77+
* Creates a mock ExtendedFile with sensible defaults
78+
*/
79+
const createMockFile = (overrides: Partial<ExtendedFile> = {}): ExtendedFile => ({
80+
file_id: 'test-file-id',
81+
type: 'image/png',
82+
preview: 'blob:http://localhost:3080/preview-blob-url',
83+
filepath: '/images/user123/test-file-id__image.png',
84+
filename: 'test-image.png',
85+
progress: 1,
86+
size: 1024,
87+
source: FileSources.local,
88+
...overrides,
89+
});
90+
91+
const renderFileRow = (files: Map<string, ExtendedFile>) => {
92+
return render(
93+
<FileRow files={files} setFiles={mockSetFiles} setFilesLoading={mockSetFilesLoading} />,
94+
);
95+
};
96+
97+
describe('Image URL Selection Logic', () => {
98+
it('should use filepath instead of preview when progress is 1 (upload complete)', () => {
99+
const file = createMockFile({
100+
file_id: 'uploaded-file',
101+
preview: 'blob:http://localhost:3080/temp-preview',
102+
filepath: '/images/user123/uploaded-file__image.png',
103+
progress: 1,
104+
});
105+
106+
const filesMap = new Map<string, ExtendedFile>();
107+
filesMap.set(file.file_id, file);
108+
109+
renderFileRow(filesMap);
110+
111+
const imageUrl = screen.getByTestId('image-url').textContent;
112+
expect(imageUrl).toBe('/images/user123/uploaded-file__image.png');
113+
expect(imageUrl).not.toContain('blob:');
114+
});
115+
116+
it('should use preview when progress is less than 1 (uploading)', () => {
117+
const file = createMockFile({
118+
file_id: 'uploading-file',
119+
preview: 'blob:http://localhost:3080/temp-preview',
120+
filepath: undefined,
121+
progress: 0.5,
122+
});
123+
124+
const filesMap = new Map<string, ExtendedFile>();
125+
filesMap.set(file.file_id, file);
126+
127+
renderFileRow(filesMap);
128+
129+
const imageUrl = screen.getByTestId('image-url').textContent;
130+
expect(imageUrl).toBe('blob:http://localhost:3080/temp-preview');
131+
});
132+
133+
it('should fallback to filepath when preview is undefined and progress is less than 1', () => {
134+
const file = createMockFile({
135+
file_id: 'file-without-preview',
136+
preview: undefined,
137+
filepath: '/images/user123/file-without-preview__image.png',
138+
progress: 0.7,
139+
});
140+
141+
const filesMap = new Map<string, ExtendedFile>();
142+
filesMap.set(file.file_id, file);
143+
144+
renderFileRow(filesMap);
145+
146+
const imageUrl = screen.getByTestId('image-url').textContent;
147+
expect(imageUrl).toBe('/images/user123/file-without-preview__image.png');
148+
});
149+
150+
it('should use filepath when both preview and filepath exist and progress is exactly 1', () => {
151+
const file = createMockFile({
152+
file_id: 'complete-file',
153+
preview: 'blob:http://localhost:3080/old-blob',
154+
filepath: '/images/user123/complete-file__image.png',
155+
progress: 1.0,
156+
});
157+
158+
const filesMap = new Map<string, ExtendedFile>();
159+
filesMap.set(file.file_id, file);
160+
161+
renderFileRow(filesMap);
162+
163+
const imageUrl = screen.getByTestId('image-url').textContent;
164+
expect(imageUrl).toBe('/images/user123/complete-file__image.png');
165+
});
166+
});
167+
168+
describe('Progress States', () => {
169+
it('should pass correct progress value during upload', () => {
170+
const file = createMockFile({
171+
progress: 0.65,
172+
});
173+
174+
const filesMap = new Map<string, ExtendedFile>();
175+
filesMap.set(file.file_id, file);
176+
177+
renderFileRow(filesMap);
178+
179+
const progress = screen.getByTestId('image-progress').textContent;
180+
expect(progress).toBe('0.65');
181+
});
182+
183+
it('should pass progress value of 1 when upload is complete', () => {
184+
const file = createMockFile({
185+
progress: 1,
186+
});
187+
188+
const filesMap = new Map<string, ExtendedFile>();
189+
filesMap.set(file.file_id, file);
190+
191+
renderFileRow(filesMap);
192+
193+
const progress = screen.getByTestId('image-progress').textContent;
194+
expect(progress).toBe('1');
195+
});
196+
});
197+
198+
describe('File Source', () => {
199+
it('should pass local source to Image component', () => {
200+
const file = createMockFile({
201+
source: FileSources.local,
202+
});
203+
204+
const filesMap = new Map<string, ExtendedFile>();
205+
filesMap.set(file.file_id, file);
206+
207+
renderFileRow(filesMap);
208+
209+
const source = screen.getByTestId('image-source').textContent;
210+
expect(source).toBe(FileSources.local);
211+
});
212+
213+
it('should pass openai source to Image component', () => {
214+
const file = createMockFile({
215+
source: FileSources.openai,
216+
});
217+
218+
const filesMap = new Map<string, ExtendedFile>();
219+
filesMap.set(file.file_id, file);
220+
221+
renderFileRow(filesMap);
222+
223+
const source = screen.getByTestId('image-source').textContent;
224+
expect(source).toBe(FileSources.openai);
225+
});
226+
});
227+
228+
describe('File Type Detection', () => {
229+
it('should render Image component for image files', () => {
230+
const file = createMockFile({
231+
type: 'image/jpeg',
232+
});
233+
234+
const filesMap = new Map<string, ExtendedFile>();
235+
filesMap.set(file.file_id, file);
236+
237+
renderFileRow(filesMap);
238+
239+
expect(screen.getByTestId('mock-image')).toBeInTheDocument();
240+
expect(screen.queryByTestId('mock-file-container')).not.toBeInTheDocument();
241+
});
242+
243+
it('should render FileContainer for non-image files', () => {
244+
const file = createMockFile({
245+
type: 'application/pdf',
246+
filename: 'document.pdf',
247+
});
248+
249+
const filesMap = new Map<string, ExtendedFile>();
250+
filesMap.set(file.file_id, file);
251+
252+
renderFileRow(filesMap);
253+
254+
expect(screen.getByTestId('mock-file-container')).toBeInTheDocument();
255+
expect(screen.queryByTestId('mock-image')).not.toBeInTheDocument();
256+
});
257+
});
258+
259+
describe('Multiple Files', () => {
260+
it('should render multiple image files with correct URLs based on their progress', () => {
261+
const filesMap = new Map<string, ExtendedFile>();
262+
263+
const uploadingFile = createMockFile({
264+
file_id: 'file-1',
265+
preview: 'blob:http://localhost:3080/preview-1',
266+
filepath: undefined,
267+
progress: 0.3,
268+
});
269+
270+
const completedFile = createMockFile({
271+
file_id: 'file-2',
272+
preview: 'blob:http://localhost:3080/preview-2',
273+
filepath: '/images/user123/file-2__image.png',
274+
progress: 1,
275+
});
276+
277+
filesMap.set(uploadingFile.file_id, uploadingFile);
278+
filesMap.set(completedFile.file_id, completedFile);
279+
280+
renderFileRow(filesMap);
281+
282+
const images = screen.getAllByTestId('mock-image');
283+
expect(images).toHaveLength(2);
284+
285+
const urls = screen.getAllByTestId('image-url').map((el) => el.textContent);
286+
expect(urls).toContain('blob:http://localhost:3080/preview-1');
287+
expect(urls).toContain('/images/user123/file-2__image.png');
288+
});
289+
290+
it('should deduplicate files with the same file_id', () => {
291+
const filesMap = new Map<string, ExtendedFile>();
292+
293+
const file1 = createMockFile({ file_id: 'duplicate-id' });
294+
const file2 = createMockFile({ file_id: 'duplicate-id' });
295+
296+
filesMap.set('key-1', file1);
297+
filesMap.set('key-2', file2);
298+
299+
renderFileRow(filesMap);
300+
301+
const images = screen.getAllByTestId('mock-image');
302+
expect(images).toHaveLength(1);
303+
});
304+
});
305+
306+
describe('Empty State', () => {
307+
it('should render nothing when files map is empty', () => {
308+
const filesMap = new Map<string, ExtendedFile>();
309+
310+
const { container } = renderFileRow(filesMap);
311+
312+
expect(container.firstChild).toBeNull();
313+
});
314+
315+
it('should render nothing when files is undefined', () => {
316+
const { container } = render(
317+
<FileRow files={undefined} setFiles={mockSetFiles} setFilesLoading={mockSetFilesLoading} />,
318+
);
319+
320+
expect(container.firstChild).toBeNull();
321+
});
322+
});
323+
324+
describe('Regression: Blob URL Bug Fix', () => {
325+
it('should NOT use revoked blob URL after upload completes', () => {
326+
const file = createMockFile({
327+
file_id: 'regression-test',
328+
preview: 'blob:http://localhost:3080/d25f730c-152d-41f7-8d79-c9fa448f606b',
329+
filepath:
330+
'/images/68c98b26901ebe2d87c193a2/c0fe1b93-ba3d-456c-80be-9a492bfd9ed0__image.png',
331+
progress: 1,
332+
});
333+
334+
const filesMap = new Map<string, ExtendedFile>();
335+
filesMap.set(file.file_id, file);
336+
337+
renderFileRow(filesMap);
338+
339+
const imageUrl = screen.getByTestId('image-url').textContent;
340+
341+
expect(imageUrl).not.toContain('blob:');
342+
expect(imageUrl).toBe(
343+
'/images/68c98b26901ebe2d87c193a2/c0fe1b93-ba3d-456c-80be-9a492bfd9ed0__image.png',
344+
);
345+
});
346+
});
347+
});

0 commit comments

Comments
 (0)