Skip to content

Commit 6a9c198

Browse files
chaxusclaude
andcommitted
fix(editor): the x2t proxy must not fetch a document as if it were a URL
Found reviewing today's changes. Guard 14 turns whatever the vendor passes into bytes before sending it to the worker, and for strings it fetched unconditionally. x2t_helper's own handleFileData does not: it sorts strings into DataURL / BlobURL / FileURL / HttpURL / URL *and* String, and that last case is not an error -- it encodes the text as UTF-8 bytes. So a document handed over as text -- the empty-document template the offline patch uses, or the latin1 DOCY form of #113 -- would have been requested as a relative URL instead of converted. Strings that are not locations now cross untouched, and the worker's own handleFileData decides, exactly as it did before conversion moved out of the frame. Same rule for media entries. looksLikeUrl is exported for the unit test on purpose: it mirrors a classifier in vendor code that this proxy has to agree with, and getting it wrong turns a document into a request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MKNHFQ1kek1cHeqhbH37Pk
1 parent 9c347ea commit 6a9c198

2 files changed

Lines changed: 71 additions & 3 deletions

File tree

‎lib/onlyoffice/guards/x2t-worker.ts‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,24 @@ function fontSources(win: FrameScope): FontSource[] {
197197
return sources;
198198
}
199199

200+
/**
201+
* The string forms x2t_helper treats as somewhere to fetch from.
202+
*
203+
* Exported for the unit test: it mirrors a classifier in vendor code that this
204+
* proxy has to agree with, and getting it wrong turns a document into a
205+
* request. Pinning it directly is cheaper than reaching it through a
206+
* conversion.
207+
*/
208+
export function looksLikeUrl(value: string): boolean {
209+
if (/^(data|blob|file|https?):/i.test(value.trim())) return true;
210+
try {
211+
new URL(value);
212+
return true;
213+
} catch {
214+
return false;
215+
}
216+
}
217+
200218
/**
201219
* Anything the vendor hands us -- blob URL, Blob, typed array -- as bytes.
202220
*
@@ -211,6 +229,13 @@ function fontSources(win: FrameScope): FontSource[] {
211229
async function toBytes(value: unknown): Promise<Uint8Array | null> {
212230
if (value == null) return null;
213231
if (typeof value === 'string') {
232+
// Only strings that are locations. x2t_helper's own handleFileData sorts
233+
// strings into DataURL / BlobURL / FileURL / HttpURL / URL and *String*,
234+
// and the last one is not an error -- it encodes the text as UTF-8 bytes.
235+
// Fetching it instead would turn a document into a bogus request, so
236+
// anything that is not a location is handed over untouched and the vendor
237+
// decides, exactly as it did before conversion moved out of the frame.
238+
if (!looksLikeUrl(value)) return null;
214239
const response = await fetch(value);
215240
if (!response.ok) throw new Error(`Failed to read document data (${response.status})`);
216241
return new Uint8Array(await response.arrayBuffer());
@@ -235,7 +260,9 @@ async function mediaToBytes(medias: unknown): Promise<Record<string, Uint8Array>
235260
for (const [path, url] of Object.entries(medias as Record<string, unknown>)) {
236261
try {
237262
const bytes = await toBytes(url);
263+
// Same rule as the document itself: unresolved forms cross untouched.
238264
if (bytes) out[path] = bytes;
265+
else if (url != null) out[path] = url as Uint8Array;
239266
} catch {
240267
// A medium that cannot be read is one the conversion does without,
241268
// which is what the in-frame path did with it too.
@@ -281,11 +308,14 @@ export function installX2tWorkerProxy(win: Window): boolean {
281308
const channel = new WorkerChannel(frame);
282309

283310
converter.convertToBin = async (data: unknown, fileName?: string, fileExt?: string) => {
284-
const bytes = await toBytes(data);
285-
if (!bytes) throw new Error('Document conversion failed: nothing to convert');
311+
if (data == null) throw new Error('Document conversion failed: nothing to convert');
312+
// Whatever could not be resolved here crosses as it came: the worker runs
313+
// the same handleFileData, so a form this does not know is still the
314+
// vendor's to interpret rather than ours to reject.
315+
const payload = (await toBytes(data)) ?? data;
286316
const result = (await channel.send(
287317
'convertToBin',
288-
{ data: bytes, fileName, fileExt },
318+
{ data: payload, fileName, fileExt },
289319
await fontSourcesWhenReady(frame),
290320
)) as Record<string, unknown>;
291321
return { ...result, media: mintMediaUrls(result.media) };

‎test/unit/x2t-worker-input.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { looksLikeUrl } from '../../lib/onlyoffice/guards/x2t-worker';
3+
4+
/**
5+
* Which strings the worker proxy resolves itself, and which it hands over.
6+
*
7+
* Guard 14 turns whatever the vendor passes into bytes before sending it to
8+
* the x2t worker. For strings that is only correct when the string is a
9+
* location: x2t_helper's own `handleFileData` sorts strings into
10+
* DataURL / BlobURL / FileURL / HttpURL / URL **and `String`**, and that last
11+
* one is not an error -- it encodes the text as UTF-8 bytes. Fetching it
12+
* instead would turn a document into a bogus request.
13+
*
14+
* So anything this does not call a location crosses the boundary untouched and
15+
* the vendor decides, exactly as it did before conversion moved out of the
16+
* frame.
17+
*/
18+
describe('what the x2t proxy treats as a location', () => {
19+
it('resolves the forms the vendor fetches', () => {
20+
for (const url of [
21+
'blob:http://127.0.0.1:4173/2f0a-4d1e',
22+
'http://example.test/report.docx',
23+
'https://example.test/report.docx',
24+
'file:///Users/x/report.docx',
25+
'data:application/octet-stream;base64,AAAA',
26+
]) {
27+
expect(looksLikeUrl(url), url).toBe(true);
28+
}
29+
});
30+
31+
it('leaves document text alone', () => {
32+
// The empty-document template the offline patch hands the editor, and the
33+
// shape #113 was about: a latin1 string carrying its DOCY header.
34+
for (const text of ['DOCY;v5;7372;AAAA', 'plain text', '', ' ', 'Editor.bin']) {
35+
expect(looksLikeUrl(text), JSON.stringify(text)).toBe(false);
36+
}
37+
});
38+
});

0 commit comments

Comments
 (0)