mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-05-13 07:46:47 +00:00
🧭 fix: Navigate Signed CDN Downloads (#12998)
* fix(files): navigate signed CDN downloads * fix(files): avoid popup target for signed downloads * test(files): restore download URL mock
This commit is contained in:
parent
40a05bbf83
commit
8f92ec012c
10 changed files with 279 additions and 36 deletions
|
|
@ -621,6 +621,9 @@ const processCodeOutput = async ({
|
|||
message: 'Error downloading/processing code environment file',
|
||||
error,
|
||||
});
|
||||
logger.warn(
|
||||
`[processCodeOutput] Falling back to Code API download URL for strategy ${appConfig.fileStrategy}`,
|
||||
);
|
||||
|
||||
// Fallback for download errors - return download URL so user can still manually download
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -138,7 +138,7 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
|||
const { convertImage } = require('~/server/services/Files/images/convert');
|
||||
const { determineFileType } = require('~/server/utils');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { codeServerHttpAgent, codeServerHttpsAgent } = require('@librechat/api');
|
||||
const { codeServerHttpAgent, codeServerHttpsAgent, getStorageMetadata } = require('@librechat/api');
|
||||
|
||||
const { processCodeOutput, getSessionInfo, readSandboxFile, primeFiles } = require('./process');
|
||||
|
||||
|
|
@ -320,6 +320,70 @@ describe('Code Process', () => {
|
|||
expect(result.bytes).toBe(100);
|
||||
});
|
||||
|
||||
it.each([
|
||||
[
|
||||
'slides.pptx',
|
||||
'application/vnd.openxmlformats-officedocument.presentationml.presentation',
|
||||
],
|
||||
['sheet.xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'],
|
||||
[
|
||||
'document.docx',
|
||||
'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
|
||||
],
|
||||
])('preserves stored metadata for code-generated office file %s', async (name, mime) => {
|
||||
const cloudfrontReq = {
|
||||
...mockReq,
|
||||
user: { ...mockReq.user, tenantId: 'tenantA' },
|
||||
config: { ...mockReq.config, fileStrategy: 'cloudfront' },
|
||||
};
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
const filepath = `https://cdn.example.com/r/us-east-2/t/tenantA/uploads/user-123/mock-uuid-1234__${name}`;
|
||||
const storageKey = `r/us-east-2/t/tenantA/uploads/user-123/mock-uuid-1234__${name}`;
|
||||
mockAxios.mockResolvedValue({ data: smallBuffer });
|
||||
determineFileType.mockResolvedValue({ mime });
|
||||
mockHasOfficeHtmlPath.mockReturnValueOnce(true);
|
||||
getStorageMetadata.mockReturnValueOnce({ storageKey, storageRegion: 'us-east-2' });
|
||||
const mockSaveBuffer = jest.fn().mockResolvedValue(filepath);
|
||||
getStrategyFunctions.mockReturnValue({ saveBuffer: mockSaveBuffer });
|
||||
|
||||
const { file: result, finalize } = await processCodeOutput({
|
||||
...baseParams,
|
||||
req: cloudfrontReq,
|
||||
name,
|
||||
});
|
||||
|
||||
expect(mockSaveBuffer).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: 'user-123',
|
||||
basePath: 'uploads',
|
||||
tenantId: 'tenantA',
|
||||
}),
|
||||
);
|
||||
expect(result).toMatchObject({
|
||||
file_id: 'mock-uuid-1234',
|
||||
user: 'user-123',
|
||||
tenantId: 'tenantA',
|
||||
source: 'cloudfront',
|
||||
filename: name,
|
||||
filepath,
|
||||
storageKey,
|
||||
storageRegion: 'us-east-2',
|
||||
status: 'pending',
|
||||
});
|
||||
expect(createFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
file_id: 'mock-uuid-1234',
|
||||
user: 'user-123',
|
||||
tenantId: 'tenantA',
|
||||
source: 'cloudfront',
|
||||
storageKey,
|
||||
storageRegion: 'us-east-2',
|
||||
}),
|
||||
true,
|
||||
);
|
||||
expect(typeof finalize).toBe('function');
|
||||
});
|
||||
|
||||
it('passes and persists tenantId for non-image code output records', async () => {
|
||||
const tenantReq = { ...mockReq, user: { ...mockReq.user, tenantId: 'tenantA' } };
|
||||
const smallBuffer = Buffer.alloc(100);
|
||||
|
|
@ -595,6 +659,9 @@ describe('Code Process', () => {
|
|||
|
||||
const { file: result } = await processCodeOutput(baseParams);
|
||||
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Falling back to Code API download URL for strategy local'),
|
||||
);
|
||||
expect(result.filepath).toContain('/api/files/code/download/session-123/file-id-123');
|
||||
expect(result.conversationId).toBe('conv-123');
|
||||
expect(result.messageId).toBe('msg-123');
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ import { useRecoilValue } from 'recoil';
|
|||
import { Download } from 'lucide-react';
|
||||
import { OGDialog, OGDialogContent, OGDialogTitle, OGDialogDescription } from '@librechat/client';
|
||||
import CopyButton from '~/components/Messages/Content/CopyButton';
|
||||
import { logger, sortPagesByRelevance } from '~/utils';
|
||||
import { revokeDownloadURL, useFileDownload } from '~/data-provider';
|
||||
import { logger, sortPagesByRelevance, triggerDownload } from '~/utils';
|
||||
import { useFileDownload } from '~/data-provider';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -201,13 +201,7 @@ export default function FilePreviewDialog({
|
|||
if (!result.data) {
|
||||
return;
|
||||
}
|
||||
const a = document.createElement('a');
|
||||
a.href = result.data;
|
||||
a.setAttribute('download', fileName);
|
||||
document.body.appendChild(a);
|
||||
a.click();
|
||||
document.body.removeChild(a);
|
||||
setTimeout(() => revokeDownloadURL(result.data), 1000);
|
||||
triggerDownload(result.data, fileName);
|
||||
} catch (err) {
|
||||
logger.error('[FilePreviewDialog] Download failed:', err);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,9 +5,9 @@ import { PermissionTypes, Permissions, apiBaseUrl } from 'librechat-data-provide
|
|||
import Mermaid, { MermaidErrorBoundary } from '~/components/Messages/Content/Mermaid';
|
||||
import CodeBlock from '~/components/Messages/Content/CodeBlock';
|
||||
import useHasAccess from '~/hooks/Roles/useHasAccess';
|
||||
import { revokeDownloadURL, useFileDownload } from '~/data-provider';
|
||||
import { useFileDownload } from '~/data-provider';
|
||||
import { useCodeBlockContext } from '~/Providers';
|
||||
import { handleDoubleClick } from '~/utils';
|
||||
import { handleDoubleClick, triggerDownload } from '~/utils';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -150,13 +150,7 @@ export const a: React.ElementType = memo(function MarkdownAnchor({ href, childre
|
|||
});
|
||||
return;
|
||||
}
|
||||
const link = document.createElement('a');
|
||||
link.href = stream.data;
|
||||
link.setAttribute('download', filename);
|
||||
document.body.appendChild(link);
|
||||
link.click();
|
||||
document.body.removeChild(link);
|
||||
revokeDownloadURL(stream.data);
|
||||
triggerDownload(stream.data, filename);
|
||||
} catch (error) {
|
||||
console.error('Error downloading file:', error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
import React from 'react';
|
||||
import { FileSources } from 'librechat-data-provider';
|
||||
import { useToastContext } from '@librechat/client';
|
||||
import { revokeDownloadURL, useCodeOutputDownload, useFileDownload } from '~/data-provider';
|
||||
import { useCodeOutputDownload, useFileDownload } from '~/data-provider';
|
||||
import { isHttpDownloadTarget, triggerDownload } from '~/utils';
|
||||
|
||||
interface LogLinkProps {
|
||||
href: string;
|
||||
|
|
@ -54,6 +55,11 @@ export const useAttachmentLink = ({
|
|||
const handleDownload = async (event: React.MouseEvent<HTMLAnchorElement | HTMLButtonElement>) => {
|
||||
event.preventDefault();
|
||||
try {
|
||||
if (!useLocalDownload && isHttpDownloadTarget(href)) {
|
||||
triggerDownload(href, filename);
|
||||
return;
|
||||
}
|
||||
|
||||
const stream = useLocalDownload ? await downloadFromApi() : await downloadFromUrl();
|
||||
if (stream.data == null || stream.data === '') {
|
||||
console.error('Error downloading file: No data found');
|
||||
|
|
@ -63,13 +69,7 @@ export const useAttachmentLink = ({
|
|||
});
|
||||
return;
|
||||
}
|
||||
const link = document.createElement('a');
|
||||
link.href = stream.data;
|
||||
link.setAttribute('download', filename);
|
||||
document.body.appendChild(link);
|
||||
link.click();
|
||||
document.body.removeChild(link);
|
||||
revokeDownloadURL(stream.data);
|
||||
triggerDownload(stream.data, filename);
|
||||
} catch (error) {
|
||||
console.error('Error downloading file:', error);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,104 @@
|
|||
import React from 'react';
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import { FileSources } from 'librechat-data-provider';
|
||||
import LogLink from '../LogLink';
|
||||
|
||||
const mockShowToast = jest.fn();
|
||||
const mockDownloadFromApi = jest.fn();
|
||||
const mockDownloadFromUrl = jest.fn();
|
||||
const mockTriggerDownload = jest.fn();
|
||||
|
||||
jest.mock('@librechat/client', () => ({
|
||||
useToastContext: () => ({ showToast: mockShowToast }),
|
||||
}));
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useFileDownload: () => ({ refetch: mockDownloadFromApi }),
|
||||
useCodeOutputDownload: () => ({ refetch: mockDownloadFromUrl }),
|
||||
}));
|
||||
|
||||
jest.mock('~/utils', () => ({
|
||||
isHttpDownloadTarget: (target?: string | null) => /^https?:\/\//i.test(target ?? ''),
|
||||
triggerDownload: (...args: Parameters<typeof mockTriggerDownload>) =>
|
||||
mockTriggerDownload(...args),
|
||||
}));
|
||||
|
||||
describe('LogLink download routing', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('navigates directly to http URLs when no stored file metadata is available', async () => {
|
||||
const filename = 'report.pptx';
|
||||
render(
|
||||
<LogLink href="https://cdn.example.com/uploads/report.pptx" filename={filename}>
|
||||
{filename}
|
||||
</LogLink>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('link', { name: filename }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockTriggerDownload).toHaveBeenCalledWith(
|
||||
'https://cdn.example.com/uploads/report.pptx',
|
||||
'report.pptx',
|
||||
);
|
||||
});
|
||||
expect(mockDownloadFromUrl).not.toHaveBeenCalled();
|
||||
expect(mockDownloadFromApi).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('uses the authorized file download route when stored metadata is available', async () => {
|
||||
const filename = 'file.pdf';
|
||||
mockDownloadFromApi.mockResolvedValue({ data: 'https://cdn.example.com/signed/file.pdf' });
|
||||
|
||||
render(
|
||||
<LogLink
|
||||
user="user-1"
|
||||
file_id="file-1"
|
||||
filename={filename}
|
||||
source={FileSources.cloudfront}
|
||||
href="https://cdn.example.com/uploads/file.pdf"
|
||||
>
|
||||
{filename}
|
||||
</LogLink>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('link', { name: filename }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockTriggerDownload).toHaveBeenCalledWith(
|
||||
'https://cdn.example.com/signed/file.pdf',
|
||||
'file.pdf',
|
||||
);
|
||||
});
|
||||
expect(mockDownloadFromApi).toHaveBeenCalledTimes(1);
|
||||
expect(mockDownloadFromUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps legacy code-output handles on the blob download path', async () => {
|
||||
const filename = 'legacy.txt';
|
||||
mockDownloadFromUrl.mockResolvedValue({ data: 'blob:https://app.example.com/file' });
|
||||
|
||||
render(
|
||||
<LogLink
|
||||
href="/api/files/code/download/session-1/file-1"
|
||||
filename={filename}
|
||||
source={FileSources.execute_code}
|
||||
>
|
||||
{filename}
|
||||
</LogLink>,
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByRole('link', { name: filename }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockTriggerDownload).toHaveBeenCalledWith(
|
||||
'blob:https://app.example.com/file',
|
||||
'legacy.txt',
|
||||
);
|
||||
});
|
||||
expect(mockDownloadFromUrl).toHaveBeenCalledTimes(1);
|
||||
expect(mockDownloadFromApi).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -16,10 +16,10 @@ import {
|
|||
import type { ValidSource, ImageResult } from 'librechat-data-provider';
|
||||
import { FaviconImage, getCleanDomain } from '~/components/Web/SourceHovercard';
|
||||
import SourcesErrorBoundary from './SourcesErrorBoundary';
|
||||
import { revokeDownloadURL, useFileDownload } from '~/data-provider';
|
||||
import { useFileDownload } from '~/data-provider';
|
||||
import { useSearchContext } from '~/Providers';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
import { cn, triggerDownload } from '~/utils';
|
||||
import store from '~/store';
|
||||
|
||||
interface SourceItemProps {
|
||||
|
|
@ -257,13 +257,7 @@ const FileItem = React.memo(function FileItem({
|
|||
});
|
||||
return;
|
||||
}
|
||||
const link = document.createElement('a');
|
||||
link.href = stream.data;
|
||||
link.setAttribute('download', file.filename);
|
||||
document.body.appendChild(link);
|
||||
link.click();
|
||||
document.body.removeChild(link);
|
||||
revokeDownloadURL(stream.data);
|
||||
triggerDownload(stream.data, file.filename);
|
||||
} catch (error) {
|
||||
console.error('Error downloading file:', error);
|
||||
}
|
||||
|
|
|
|||
71
client/src/utils/__tests__/downloadFile.test.ts
Normal file
71
client/src/utils/__tests__/downloadFile.test.ts
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
import { isHttpDownloadTarget, triggerDownload } from '../downloadFile';
|
||||
|
||||
describe('downloadFile utilities', () => {
|
||||
let clickSpy: jest.SpyInstance;
|
||||
let appendSpy: jest.SpyInstance;
|
||||
let removeSpy: jest.SpyInstance;
|
||||
let revokeSpy: jest.Mock;
|
||||
let appendedLink: HTMLAnchorElement | null;
|
||||
let originalRevokeObjectURL: typeof URL.revokeObjectURL | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.useFakeTimers();
|
||||
appendedLink = null;
|
||||
originalRevokeObjectURL = URL.revokeObjectURL;
|
||||
clickSpy = jest.spyOn(HTMLAnchorElement.prototype, 'click').mockImplementation();
|
||||
appendSpy = jest.spyOn(document.body, 'appendChild').mockImplementation((node: Node) => {
|
||||
appendedLink = node as HTMLAnchorElement;
|
||||
return node;
|
||||
});
|
||||
removeSpy = jest.spyOn(document.body, 'removeChild').mockImplementation((node: Node) => node);
|
||||
revokeSpy = jest.fn();
|
||||
Object.defineProperty(URL, 'revokeObjectURL', {
|
||||
configurable: true,
|
||||
value: revokeSpy,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
clickSpy.mockRestore();
|
||||
appendSpy.mockRestore();
|
||||
removeSpy.mockRestore();
|
||||
Object.defineProperty(URL, 'revokeObjectURL', {
|
||||
configurable: true,
|
||||
value: originalRevokeObjectURL,
|
||||
});
|
||||
jest.useRealTimers();
|
||||
});
|
||||
|
||||
it('detects absolute http download targets', () => {
|
||||
expect(isHttpDownloadTarget('https://cdn.example.com/file.pdf')).toBe(true);
|
||||
expect(isHttpDownloadTarget('http://cdn.example.com/file.pdf')).toBe(true);
|
||||
expect(isHttpDownloadTarget('blob:https://app.example.com/id')).toBe(false);
|
||||
expect(isHttpDownloadTarget('/api/files/code/download/session/file')).toBe(false);
|
||||
expect(isHttpDownloadTarget(undefined)).toBe(false);
|
||||
});
|
||||
|
||||
it('navigates http URLs in the same tab without revoking them', () => {
|
||||
triggerDownload('https://cdn.example.com/file.pdf?Policy=abc', 'file.pdf');
|
||||
|
||||
expect(clickSpy).toHaveBeenCalledTimes(1);
|
||||
expect(appendSpy).toHaveBeenCalledTimes(1);
|
||||
expect(removeSpy).toHaveBeenCalledTimes(1);
|
||||
expect(appendedLink?.href).toBe('https://cdn.example.com/file.pdf?Policy=abc');
|
||||
expect(appendedLink?.download).toBe('file.pdf');
|
||||
expect(appendedLink?.target).toBe('');
|
||||
expect(appendedLink?.rel).toBe('');
|
||||
jest.runOnlyPendingTimers();
|
||||
expect(revokeSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('revokes blob URLs after the click', () => {
|
||||
triggerDownload('blob:https://app.example.com/download-id', 'file.pdf');
|
||||
|
||||
expect(clickSpy).toHaveBeenCalledTimes(1);
|
||||
expect(appendedLink?.target).toBe('');
|
||||
expect(appendedLink?.rel).toBe('');
|
||||
expect(revokeSpy).not.toHaveBeenCalled();
|
||||
jest.advanceTimersByTime(1000);
|
||||
expect(revokeSpy).toHaveBeenCalledWith('blob:https://app.example.com/download-id');
|
||||
});
|
||||
});
|
||||
15
client/src/utils/downloadFile.ts
Normal file
15
client/src/utils/downloadFile.ts
Normal file
|
|
@ -0,0 +1,15 @@
|
|||
export const isHttpDownloadTarget = (target?: string | null): boolean =>
|
||||
/^https?:\/\//i.test(target ?? '');
|
||||
|
||||
export function triggerDownload(target: string, filename: string): void {
|
||||
const isBlob = target.startsWith('blob:');
|
||||
const link = document.createElement('a');
|
||||
link.href = target;
|
||||
link.setAttribute('download', filename);
|
||||
document.body.appendChild(link);
|
||||
link.click();
|
||||
document.body.removeChild(link);
|
||||
if (isBlob) {
|
||||
setTimeout(() => URL.revokeObjectURL(target), 1000);
|
||||
}
|
||||
}
|
||||
|
|
@ -24,6 +24,7 @@ export * from './redirect';
|
|||
export * from './languages';
|
||||
export * from './endpoints';
|
||||
export * from './resources';
|
||||
export * from './downloadFile';
|
||||
export * from './scaleImage';
|
||||
export * from './timestamps';
|
||||
export * from './localStorage';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue