mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-29 05:20:49 +00:00
📄 fix: Serve Stored Text for "Upload as Text" File Downloads (#14723)
* 📄 fix: Serve Stored Text for Text-Source File Downloads
"Upload as Text" attachments store extracted content in the DB with
source 'text'; OCR uploads persist the OCR strategy name (e.g.
'mistral_ocr') as a filepath placeholder since no backing file exists.
The download route resolved these records to the local strategy and
passed the placeholder to fs.createReadStream, which failed with
ENOENT — and the response was never ended after the stream error, so
the request hung until the client timed out.
Serve the stored text directly as a .txt download for text-source
files (re-fetched by _id, as getFiles excludes 'text' by default),
and end the response on stream errors: 500 without the download
headers before headers are sent, otherwise abort the truncated
response so clients detect the failure.
* fix: Preserve text-source preview semantics
* fix: Complete text-source download coverage
* fix: Tie text downloads to blob lifecycle
* fix: Isolate preview downloads and share text snapshots
* fix: Keep shared previews in share scope
* style: Sort text download imports
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
d602452c05
commit
b399ad8370
21 changed files with 622 additions and 98 deletions
|
|
@ -945,6 +945,28 @@ describe('share-scoped file routes', () => {
|
|||
expect(response.headers['content-disposition']).toContain('attachment');
|
||||
});
|
||||
|
||||
it('downloads stored text for a snapshotted text-source file', async () => {
|
||||
getFiles.mockResolvedValue([{ status: 'ready', text: 'Shared extracted text' }]);
|
||||
getSharedLinkFile.mockResolvedValue({
|
||||
file: {
|
||||
file_id: 'file-1',
|
||||
source: 'text',
|
||||
filepath: 'mistral_ocr',
|
||||
type: 'application/pdf',
|
||||
filename: 'report.pdf',
|
||||
},
|
||||
hasSnapshots: true,
|
||||
});
|
||||
|
||||
const response = await request(buildApp()).get('/api/share/share-123/files/file-1/download');
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers['content-type']).toContain('text/plain');
|
||||
expect(response.headers['content-disposition']).toContain('attachment; report.pdf.txt');
|
||||
expect(response.text).toBe('Shared extracted text');
|
||||
expect(mockGetStrategyFunctions).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns 500 when the backing stream fails before sending bytes', async () => {
|
||||
const failingStream = new Readable({
|
||||
read() {
|
||||
|
|
|
|||
|
|
@ -570,6 +570,26 @@ router.get('/download/:userId/:file_id', fileAccess, async (req, res) => {
|
|||
// Access already validated by fileAccess middleware
|
||||
const file = req.fileAccess.file;
|
||||
|
||||
// Text-source files store extracted content in the DB; there is no backing file to stream
|
||||
if (file.source === FileSources.text) {
|
||||
/** `getFiles` excludes `text` by default, so the authorized record is re-fetched by `_id` */
|
||||
const [textFile] = (await db.getFiles({ _id: file._id }, null, { text: 1 })) ?? [];
|
||||
if (textFile?.text == null) {
|
||||
logger.warn(`File download requested by user ${userId} has no stored text: ${file_id}`);
|
||||
return res.status(404).send('No file content found');
|
||||
}
|
||||
const textFilename = file.filename?.toLowerCase().endsWith('.txt')
|
||||
? file.filename
|
||||
: `${file.filename || file_id}.txt`;
|
||||
res.setHeader('Content-Disposition', getContentDisposition(textFilename));
|
||||
res.setHeader('Content-Type', 'text/plain; charset=utf-8');
|
||||
res.setHeader(
|
||||
'X-File-Metadata',
|
||||
encodeURIComponent(JSON.stringify(getDownloadFileMetadata(file))),
|
||||
);
|
||||
return res.send(textFile.text);
|
||||
}
|
||||
|
||||
if (checkOpenAIStorage(file.source) && !file.model) {
|
||||
logger.warn(`File download requested by user ${userId} has no associated model: ${file_id}`);
|
||||
return res.status(400).send('The model used when creating this file is not available');
|
||||
|
|
@ -642,6 +662,16 @@ router.get('/download/:userId/:file_id', fileAccess, async (req, res) => {
|
|||
|
||||
fileStream.on('error', (streamError) => {
|
||||
logger.error('[DOWNLOAD ROUTE] Stream error:', streamError);
|
||||
if (res.headersSent) {
|
||||
if (!res.writableEnded) {
|
||||
res.destroy();
|
||||
}
|
||||
return;
|
||||
}
|
||||
res.removeHeader('Content-Disposition');
|
||||
res.removeHeader('Content-Type');
|
||||
res.removeHeader('X-File-Metadata');
|
||||
res.status(500).send('Error downloading file');
|
||||
});
|
||||
|
||||
setHeaders();
|
||||
|
|
|
|||
|
|
@ -940,6 +940,160 @@ describe('File Routes - Delete with Agent Access', () => {
|
|||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('serves stored text for text-source files instead of streaming', async () => {
|
||||
const userFileId = uuidv4();
|
||||
const getDownloadStream = jest.fn();
|
||||
getStrategyFunctions.mockReturnValue({ getDownloadStream });
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'screenshot.png',
|
||||
filepath: FileSources.mistral_ocr,
|
||||
bytes: 70,
|
||||
type: 'text/plain',
|
||||
source: FileSources.text,
|
||||
text: 'Extracted OCR text',
|
||||
});
|
||||
|
||||
const response = await request(app).get(`/files/download/${otherUserId}/${userFileId}`);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers['content-type']).toContain('text/plain');
|
||||
expect(response.headers['content-disposition']).toContain('screenshot.png.txt');
|
||||
expect(response.text).toBe('Extracted OCR text');
|
||||
const metadata = JSON.parse(decodeURIComponent(response.headers['x-file-metadata']));
|
||||
expect(metadata).toMatchObject({ file_id: userFileId, source: FileSources.text });
|
||||
expect(metadata).not.toHaveProperty('text');
|
||||
expect(getDownloadStream).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not append .txt when the text-source filename already ends in .txt', async () => {
|
||||
const userFileId = uuidv4();
|
||||
getStrategyFunctions.mockReturnValue({});
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'NOTES.TXT',
|
||||
filepath: FileSources.mistral_ocr,
|
||||
bytes: 20,
|
||||
type: 'text/plain',
|
||||
source: FileSources.text,
|
||||
text: 'plain text notes',
|
||||
});
|
||||
|
||||
const response = await request(app).get(`/files/download/${otherUserId}/${userFileId}`);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers['content-disposition']).toContain('filename="NOTES.TXT"');
|
||||
expect(response.headers['content-disposition']).not.toContain('NOTES.TXT.txt');
|
||||
expect(response.text).toBe('plain text notes');
|
||||
});
|
||||
|
||||
it('returns 404 for text-source files without stored text', async () => {
|
||||
const userFileId = uuidv4();
|
||||
const getDownloadStream = jest.fn();
|
||||
getStrategyFunctions.mockReturnValue({ getDownloadStream });
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'empty.png',
|
||||
filepath: FileSources.mistral_ocr,
|
||||
bytes: 0,
|
||||
type: 'text/plain',
|
||||
source: FileSources.text,
|
||||
});
|
||||
|
||||
const response = await request(app).get(`/files/download/${otherUserId}/${userFileId}`);
|
||||
|
||||
expect(response.status).toBe(404);
|
||||
expect(response.text).toBe('No file content found');
|
||||
expect(getDownloadStream).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('serves a valid empty stored-text result', async () => {
|
||||
const userFileId = uuidv4();
|
||||
const getDownloadStream = jest.fn();
|
||||
getStrategyFunctions.mockReturnValue({ getDownloadStream });
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'empty.txt',
|
||||
filepath: '/uploads/empty.txt',
|
||||
bytes: 0,
|
||||
type: 'text/plain',
|
||||
source: FileSources.text,
|
||||
text: '',
|
||||
});
|
||||
|
||||
const response = await request(app).get(`/files/download/${otherUserId}/${userFileId}`);
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.headers['content-type']).toContain('text/plain');
|
||||
expect(response.text).toBe('');
|
||||
expect(getDownloadStream).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('responds with 500 when the download stream errors before data is sent', async () => {
|
||||
const userFileId = uuidv4();
|
||||
const erroringStream = new Readable({
|
||||
read() {
|
||||
this.destroy(new Error('ENOENT: no such file or directory'));
|
||||
},
|
||||
});
|
||||
const getDownloadStream = jest.fn().mockResolvedValue(erroringStream);
|
||||
getStrategyFunctions.mockReturnValue({ getDownloadStream });
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'gone.bin',
|
||||
filepath: '/uploads/user/gone.bin',
|
||||
bytes: 5,
|
||||
type: 'application/octet-stream',
|
||||
source: FileSources.local,
|
||||
});
|
||||
|
||||
const response = await request(app).get(`/files/download/${otherUserId}/${userFileId}`);
|
||||
|
||||
expect(response.status).toBe(500);
|
||||
expect(response.text).toBe('Error downloading file');
|
||||
});
|
||||
|
||||
it('aborts the response when the download stream errors mid-transfer', async () => {
|
||||
const userFileId = uuidv4();
|
||||
let pushed = false;
|
||||
const erroringStream = new Readable({
|
||||
read() {
|
||||
if (!pushed) {
|
||||
pushed = true;
|
||||
this.push('partial content');
|
||||
return;
|
||||
}
|
||||
this.destroy(new Error('read failed mid-stream'));
|
||||
},
|
||||
});
|
||||
const getDownloadStream = jest.fn().mockResolvedValue(erroringStream);
|
||||
getStrategyFunctions.mockReturnValue({ getDownloadStream });
|
||||
|
||||
await createFile({
|
||||
user: otherUserId,
|
||||
file_id: userFileId,
|
||||
filename: 'truncated.bin',
|
||||
filepath: '/uploads/user/truncated.bin',
|
||||
bytes: 100,
|
||||
type: 'application/octet-stream',
|
||||
source: FileSources.local,
|
||||
});
|
||||
|
||||
await expect(
|
||||
request(app).get(`/files/download/${otherUserId}/${userFileId}`),
|
||||
).rejects.toThrow(/aborted|socket hang up|ECONNRESET/i);
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /files/usage', () => {
|
||||
|
|
|
|||
|
|
@ -197,7 +197,6 @@ const resolveShareFile = async (req, res, next) => {
|
|||
/** Stream (or redirect to) a snapshotted file from its original stored object. */
|
||||
const streamSharedFile = async (req, res, file, requestedDisposition) => {
|
||||
const source = file.source || FileSources.local;
|
||||
const { getDownloadStream, getDownloadURL } = getStrategyFunctions(source);
|
||||
|
||||
// An update keeps the shareId, so these URLs are stable across re-publishes. Without
|
||||
// revalidation a viewer's cached copy would outlive a revoked "share files" choice or a
|
||||
|
|
@ -209,6 +208,22 @@ const streamSharedFile = async (req, res, file, requestedDisposition) => {
|
|||
return res.status(304).end();
|
||||
}
|
||||
|
||||
if (source === FileSources.text) {
|
||||
if (req.liveFile?.text == null) {
|
||||
return res.status(404).send('No file content found');
|
||||
}
|
||||
const textFilename = file.filename?.toLowerCase().endsWith('.txt')
|
||||
? file.filename
|
||||
: `${file.filename || file.file_id}.txt`;
|
||||
const disposition = requestedDisposition === 'inline' ? 'inline' : 'attachment';
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('Content-Disposition', getContentDisposition(textFilename, disposition));
|
||||
res.setHeader('Content-Type', 'text/plain; charset=utf-8');
|
||||
return res.send(req.liveFile.text);
|
||||
}
|
||||
|
||||
const { getDownloadStream, getDownloadURL } = getStrategyFunctions(source);
|
||||
|
||||
// Inline only safe preview types; anything else is forced to attachment.
|
||||
const disposition =
|
||||
requestedDisposition === 'inline' && SAFE_INLINE_TYPES.has(file.type) ? 'inline' : 'attachment';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue