mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 17:44:11 +00:00
🔧 fix: Route extensionless office files by MIME alone
Codex review on #12934 caught that the office-render gate in `extractCodeArtifactText` only fired when the extension was in `OFFICE_HTML_EXTENSIONS` or the category was `document`/`pptx`. A tool emitting `data` with `text/csv` (no extension) classifies as `utf8-text`, so the gate was skipped and raw CSV text shipped to the client — but the client routes by MIME to the SPREADSHEET bucket expecting a full HTML document, so the panel rendered broken text. Extract a shared `officeHtmlBucket(name, mime)` predicate from `html.ts` (returns the bucket name or null). Both `bufferToOfficeHtml` (the dispatcher) and the upstream gate in `extract.ts` now go through this single source of truth, so they can never drift apart again. The predicate already mirrors the dispatcher's extension/MIME logic (extension wins; MIME is the fallback for extensionless inputs). Adds: - 14 cases for the new `officeHtmlBucket` predicate covering the positive paths (each bucket via extension OR MIME) and the negative paths (txt, py, json, jpg, pdf, zip, odt, plain noext). - A direct regression test in `extract.spec.ts` for the Codex catch: `data` with `text/csv` + utf8-text category routes through the office HTML producer. - Parameterized cases for extensionless DOCX/XLSX/XLS/ODS/PPTX files identified by MIME alone.
This commit is contained in:
parent
fb4ac84250
commit
f87a644eaf
4 changed files with 164 additions and 40 deletions
|
|
@ -20,14 +20,23 @@ jest.mock('~/files/documents/crud', () => ({
|
|||
* assertions (parseDocument receives the canonical MIME) keep exercising
|
||||
* `extractDocument`. Tests that need real HTML output drive `bufferToOfficeHtml`
|
||||
* directly via its own spec file (`html.spec.ts`); a separate `office-html`
|
||||
* describe block below exercises the integration with this mock relaxed. */
|
||||
* describe block below exercises the integration with this mock relaxed.
|
||||
*
|
||||
* `officeHtmlBucket` is the gate predicate the upstream uses to decide
|
||||
* whether to call the dispatcher at all. We pass through to the real
|
||||
* implementation so the gate routes the same files in tests as in prod. */
|
||||
const mockOfficeHtml = jest.fn(
|
||||
async (_buffer: Buffer, _name: string, _mime: string) => null as string | null,
|
||||
);
|
||||
jest.mock('~/files/documents/html', () => ({
|
||||
bufferToOfficeHtml: (buffer: Buffer, name: string, mime: string) =>
|
||||
mockOfficeHtml(buffer, name, mime),
|
||||
}));
|
||||
jest.mock('~/files/documents/html', () => {
|
||||
const actual =
|
||||
jest.requireActual<typeof import('~/files/documents/html')>('~/files/documents/html');
|
||||
return {
|
||||
bufferToOfficeHtml: (buffer: Buffer, name: string, mime: string) =>
|
||||
mockOfficeHtml(buffer, name, mime),
|
||||
officeHtmlBucket: actual.officeHtmlBucket,
|
||||
};
|
||||
});
|
||||
|
||||
describe('extractCodeArtifactText', () => {
|
||||
describe('utf8-text', () => {
|
||||
|
|
@ -298,5 +307,44 @@ describe('extractCodeArtifactText', () => {
|
|||
expect(mockOfficeHtml).not.toHaveBeenCalled();
|
||||
expect(text).toBe('hello world\n');
|
||||
});
|
||||
|
||||
it('routes extensionless CSV-by-MIME (utf8-text category) through the office HTML path', async () => {
|
||||
/* Regression for the Codex review on PR #12934. A tool emitting
|
||||
* `data` with `text/csv` classifies as `utf8-text` (csv has no
|
||||
* extension here, MIME is text/* which the classifier treats as
|
||||
* utf8-text). The previous gate skipped the office-render branch
|
||||
* because the extension wasn't in OFFICE_HTML_EXTENSIONS — so the
|
||||
* raw CSV text shipped to the client, which routes by MIME to the
|
||||
* SPREADSHEET bucket and expects a full HTML document. The fix
|
||||
* shares the dispatcher's MIME-aware predicate so the gate fires
|
||||
* here too. */
|
||||
mockOfficeHtml.mockResolvedValueOnce('<!DOCTYPE html><table><tr><td>1</td></tr></table>');
|
||||
const text = await extractCodeArtifactText(
|
||||
Buffer.from('a,b\n1,2', 'utf-8'),
|
||||
'data',
|
||||
'text/csv',
|
||||
'utf8-text',
|
||||
);
|
||||
expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), 'data', 'text/csv');
|
||||
expect(text).toContain('<table>');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['workbook', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'],
|
||||
['workbook', 'application/vnd.ms-excel'],
|
||||
['workbook', 'application/vnd.oasis.opendocument.spreadsheet'],
|
||||
['report', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'],
|
||||
['deck', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'],
|
||||
])('routes extensionless office files by MIME alone (%s, %s)', async (name, mime) => {
|
||||
mockOfficeHtml.mockResolvedValueOnce('<!DOCTYPE html><body>x</body></html>');
|
||||
const category = mime.includes('presentation')
|
||||
? 'pptx'
|
||||
: mime.startsWith('text/')
|
||||
? 'utf8-text'
|
||||
: 'document';
|
||||
const text = await extractCodeArtifactText(Buffer.from('PK'), name, mime, category);
|
||||
expect(mockOfficeHtml).toHaveBeenCalledWith(expect.any(Buffer), name, mime);
|
||||
expect(text).toContain('<body>');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import { randomUUID } from 'crypto';
|
|||
import { logger } from '@librechat/data-schemas';
|
||||
import type { CodeArtifactCategory } from './classify';
|
||||
import { parseDocument } from '~/files/documents/crud';
|
||||
import { bufferToOfficeHtml } from '~/files/documents/html';
|
||||
import { bufferToOfficeHtml, officeHtmlBucket } from '~/files/documents/html';
|
||||
import { isBinaryBuffer } from '~/skills/binary';
|
||||
import { withTimeout } from '~/utils/promise';
|
||||
|
||||
|
|
@ -17,20 +17,16 @@ const TRUNCATION_MARKER = '\n\n…[truncated]';
|
|||
const TRUNCATION_MARKER_BYTES = Buffer.byteLength(TRUNCATION_MARKER, 'utf-8');
|
||||
|
||||
/**
|
||||
* Extensions whose buffers should be rendered as HTML (rich preview) rather
|
||||
* than plain text. CSV is included here even though it lives in the
|
||||
* `utf8-text` category — when an office HTML producer is available we want
|
||||
* the styled-table preview, not raw comma-separated text.
|
||||
* Decide whether a buffer is a candidate for rich HTML preview. Wraps the
|
||||
* shared `officeHtmlBucket` predicate from `~/files/documents/html` so the
|
||||
* gate here stays in lock-step with what the dispatcher will actually
|
||||
* route — including extensionless office files identified solely by MIME
|
||||
* (e.g. a tool emitting `data` with `text/csv`, which would otherwise
|
||||
* classify as `utf8-text`, skip the office gate, and ship raw CSV text
|
||||
* to the client's SPREADSHEET bucket that expects full HTML).
|
||||
*/
|
||||
const OFFICE_HTML_EXTENSIONS = new Set<string>(['docx', 'xlsx', 'xls', 'ods', 'pptx', 'csv']);
|
||||
|
||||
const extensionOf = (name: string): string => {
|
||||
const dot = name.lastIndexOf('.');
|
||||
if (dot < 0 || dot === name.length - 1) {
|
||||
return '';
|
||||
}
|
||||
return name.slice(dot + 1).toLowerCase();
|
||||
};
|
||||
const hasOfficeHtmlPath = (name: string, mimeType: string): boolean =>
|
||||
officeHtmlBucket(name, mimeType) !== null;
|
||||
|
||||
/**
|
||||
* Truncate UTF-8 content to fit within MAX_TEXT_CACHE_BYTES. Walks back to a
|
||||
|
|
@ -181,14 +177,14 @@ export async function extractCodeArtifactText(
|
|||
return null;
|
||||
}
|
||||
try {
|
||||
/* Office HTML preview takes precedence for any file whose extension marks
|
||||
* it as a rich-preview type. We check extension first because content
|
||||
* sniffing routinely labels these formats as `application/zip` or
|
||||
* `application/octet-stream`. The HTML producer's own dispatcher will
|
||||
* also fall back to MIME for extensionless inputs. */
|
||||
const ext = extensionOf(name);
|
||||
const officeByExt = OFFICE_HTML_EXTENSIONS.has(ext);
|
||||
if (officeByExt || category === 'document' || category === 'pptx') {
|
||||
/* Office HTML preview takes precedence for any file the dispatcher
|
||||
* recognizes — by extension OR by MIME. The MIME fallback matters for
|
||||
* extensionless tool outputs (e.g. `data` with `text/csv`); without
|
||||
* it we'd ship raw CSV text to the client's SPREADSHEET bucket which
|
||||
* expects a full HTML document. The `document`/`pptx` category checks
|
||||
* keep the path open for sniffed `application/zip`/`octet-stream` MIMEs
|
||||
* where neither the extension nor the MIME alone is dispositive. */
|
||||
if (hasOfficeHtmlPath(name, mimeType) || category === 'document' || category === 'pptx') {
|
||||
const html = await renderOfficeHtml(buffer, name, mimeType);
|
||||
if (html != null) {
|
||||
return html;
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
bufferToOfficeHtml,
|
||||
csvToHtml,
|
||||
excelSheetToHtml,
|
||||
officeHtmlBucket,
|
||||
pptxToSlideListHtml,
|
||||
sanitizeOfficeHtml,
|
||||
wordDocToHtml,
|
||||
|
|
@ -219,6 +220,50 @@ describe('Office HTML producers', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('officeHtmlBucket predicate', () => {
|
||||
/* The shared predicate is the single source of truth for "should the
|
||||
* office HTML pipeline handle this file?". The upstream gate in
|
||||
* `extract.ts` calls it directly; the dispatcher above delegates to
|
||||
* it. Tests here lock in MIME-only routing for extensionless inputs
|
||||
* (the case Codex flagged on PR #12934). */
|
||||
it.each([
|
||||
['report.docx', 'application/zip', 'docx'],
|
||||
['report.docx', '', 'docx'],
|
||||
['noext', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', 'docx'],
|
||||
['data.csv', 'application/octet-stream', 'csv'],
|
||||
['data', 'text/csv', 'csv'],
|
||||
['data', 'application/csv', 'csv'],
|
||||
['workbook.xlsx', '', 'spreadsheet'],
|
||||
['legacy.xls', '', 'spreadsheet'],
|
||||
['sheet.ods', '', 'spreadsheet'],
|
||||
['noext', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', 'spreadsheet'],
|
||||
['noext', 'application/vnd.ms-excel', 'spreadsheet'],
|
||||
['noext', 'application/vnd.oasis.opendocument.spreadsheet', 'spreadsheet'],
|
||||
['deck.pptx', '', 'pptx'],
|
||||
[
|
||||
'noext',
|
||||
'application/vnd.openxmlformats-officedocument.presentationml.presentation',
|
||||
'pptx',
|
||||
],
|
||||
])('classifies (%s, %s) as %s', (name, mime, expected) => {
|
||||
expect(officeHtmlBucket(name, mime)).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['note.txt', 'text/plain'],
|
||||
['code.py', 'text/x-python'],
|
||||
['data.json', 'application/json'],
|
||||
['photo.jpg', 'image/jpeg'],
|
||||
['doc.pdf', 'application/pdf'],
|
||||
['archive.zip', 'application/zip'],
|
||||
['notes.odt', 'application/vnd.oasis.opendocument.text'],
|
||||
['noext', 'text/plain'],
|
||||
['noext', ''],
|
||||
])('returns null for non-office (%s, %s)', (name, mime) => {
|
||||
expect(officeHtmlBucket(name, mime)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitizeOfficeHtml security', () => {
|
||||
test('strips <script> tags entirely', async () => {
|
||||
const out = await sanitizeOfficeHtml('<p>before<script>alert(1)</script>after</p>');
|
||||
|
|
|
|||
|
|
@ -651,24 +651,34 @@ function extensionOf(name: string): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* Route an office-format buffer to the matching HTML producer. Returns `null`
|
||||
* if the file isn't a recognized office type — caller should fall through to
|
||||
* its existing text/binary handling.
|
||||
* The four buckets the dispatcher can route to. Used by both
|
||||
* `bufferToOfficeHtml` (which actually renders) and `officeHtmlBucket`
|
||||
* (which the upstream gate in `extract.ts` calls to decide whether to
|
||||
* invoke the dispatcher at all). Sharing one source of truth prevents the
|
||||
* gate from going out of sync with the dispatcher — a previous version
|
||||
* had a narrower extension-only gate that missed extensionless office
|
||||
* files identified solely by MIME (e.g. a tool emitting `data` with
|
||||
* `text/csv`), which then routed to the SPREADSHEET bucket on the
|
||||
* client expecting full HTML and got raw text instead.
|
||||
*/
|
||||
export type OfficeHtmlBucket = 'docx' | 'spreadsheet' | 'csv' | 'pptx';
|
||||
|
||||
/**
|
||||
* Classify a file by extension OR MIME into the bucket the office HTML
|
||||
* dispatcher would route it to, or `null` if no producer applies.
|
||||
*
|
||||
* Extension wins over MIME because content sniffing routinely labels these
|
||||
* formats as `application/zip` or `application/octet-stream`.
|
||||
* formats as `application/zip` or `application/octet-stream`. MIME-only
|
||||
* routing is the fallback for extensionless filenames where the upstream
|
||||
* supplied a useful Content-Type header.
|
||||
*/
|
||||
export async function bufferToOfficeHtml(
|
||||
buffer: Buffer,
|
||||
name: string,
|
||||
mimeType: string,
|
||||
): Promise<string | null> {
|
||||
export function officeHtmlBucket(name: string, mimeType: string): OfficeHtmlBucket | null {
|
||||
const ext = extensionOf(name);
|
||||
if (ext === 'docx' || mimeType === DOCX_MIME) {
|
||||
return wordDocToHtml(buffer);
|
||||
return 'docx';
|
||||
}
|
||||
if (ext === 'csv' || CSV_MIME_PATTERN.test(mimeType)) {
|
||||
return csvToHtml(buffer);
|
||||
return 'csv';
|
||||
}
|
||||
if (
|
||||
ext === 'xlsx' ||
|
||||
|
|
@ -677,10 +687,35 @@ export async function bufferToOfficeHtml(
|
|||
excelMimeTypes.test(mimeType) ||
|
||||
mimeType === ODS_MIME
|
||||
) {
|
||||
return excelSheetToHtml(buffer);
|
||||
return 'spreadsheet';
|
||||
}
|
||||
if (ext === 'pptx' || mimeType === PPTX_MIME) {
|
||||
return pptxToSlideListHtml(buffer);
|
||||
return 'pptx';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Route an office-format buffer to the matching HTML producer. Returns `null`
|
||||
* if the file isn't a recognized office type — caller should fall through to
|
||||
* its existing text/binary handling.
|
||||
*/
|
||||
export async function bufferToOfficeHtml(
|
||||
buffer: Buffer,
|
||||
name: string,
|
||||
mimeType: string,
|
||||
): Promise<string | null> {
|
||||
const bucket = officeHtmlBucket(name, mimeType);
|
||||
switch (bucket) {
|
||||
case 'docx':
|
||||
return wordDocToHtml(buffer);
|
||||
case 'csv':
|
||||
return csvToHtml(buffer);
|
||||
case 'spreadsheet':
|
||||
return excelSheetToHtml(buffer);
|
||||
case 'pptx':
|
||||
return pptxToSlideListHtml(buffer);
|
||||
default:
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue