From 32e10ca6d392b59c95a0d095de48d3a41121662a Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 00:09:21 +0300 Subject: [PATCH 1/2] =?UTF-8?q?fix(drawio):=20content=3D=20=D0=BF=D0=B8?= =?UTF-8?q?=D1=88=D0=B5=D1=82=D1=81=D1=8F=20entity-XML,=20=D0=BD=D0=B5=20b?= =?UTF-8?q?ase64=20=E2=80=94=20=D0=BA=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=D0=B8?= =?UTF-8?q?=D1=86=D0=B0=20=D0=BD=D0=B5=20=D0=BC=D0=BE=D0=B9=D0=B1=D0=B5?= =?UTF-8?q?=D0=B9=D0=BA=D0=B0=D0=B5=D1=82=20=D0=B2=20=D1=80=D0=B5=D0=B4?= =?UTF-8?q?=D0=B0=D0=BA=D1=82=D0=BE=D1=80=D0=B5=20(#507)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Реviewer-filed баг: draw.io-редактор декодит base64 content= как Latin-1 (atob-семантика) → кириллица разваливается в мойбейк, автосейв редактора персистит порчу и убивает превью. Нативная форма draw.io — entity-encoded mxfile-XML (content="<mxfile…"), DOM-декодится как UTF-8. Фикс двух write-путей: buildDrawioSvg (mcp, все create/update) и createDrawioSvg (server Confluence-импорт) теперь XML-эскейпят content= вместо base64. Декодер уже различает startsWith("<") vs base64 — обе формы читаются, старые base64-файлы открываются (back-compat), byte-stable round-trip. CHANGELOG + заметка про лечение старых диаграмм (drawioGet→Update). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 15 +++ ...port-attachment.service.drawio-svg.spec.ts | 96 +++++++++++++++++++ .../services/import-attachment.service.ts | 21 +++- packages/mcp/src/lib/drawio-xml.ts | 22 +++-- packages/mcp/test/unit/drawio-xml.test.mjs | 81 +++++++++++++++- 5 files changed, 224 insertions(+), 11 deletions(-) create mode 100644 apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e33e1149..0a6a51a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -392,6 +392,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `@docmost/token-estimate` package) so they can never diverge. Deferred-tool activation is also cached in the chat metadata to avoid re-resolving it each turn. (#490) +- **Cyrillic (and any non-ASCII) draw.io labels no longer turn into mojibake + when a diagram is opened in the draw.io editor.** Agent-created diagrams + (`drawioCreate`) and Confluence-imported diagrams stored their model in the + SVG's `content=` attribute as base64; the draw.io editor decodes that via + Latin-1 `atob` (no UTF-8 step), so every non-ASCII char (e.g. `Старт-бит`, + `ё`, `—`) split into garbage and the editor's autosave then persisted the + corrupted model, breaking the page preview too. Both write paths + (`buildDrawioSvg`, the import service's `createDrawioSvg`) now write `content=` + as XML-entity-escaped mxfile XML — draw.io's own native form, decoded by the + DOM as UTF-8 — so labels open intact. The decoder reads both the new + entity-encoded form and the old base64 form, so existing diagrams still open. + *Healing pre-fix diagrams:* any agent-created cyrillic diagram written before + this fix is repaired in place by `drawioGet` → `drawioUpdate` with the same + XML (rewrites the attachment in the new form); no migration script is needed. + (#507) - **A chat with one malformed message part no longer 500s on every turn, and a failed send no longer duplicates the user's message.** Incoming client parts are now whitelisted to `text` (a forged tool-result part can no longer reach diff --git a/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts b/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts new file mode 100644 index 00000000..00735ea2 --- /dev/null +++ b/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts @@ -0,0 +1,96 @@ +// p-limit and @sindresorhus/slugify are ESM-only and not in jest's transform +// allowlist; both are irrelevant to createDrawioSvg (a pure fs + string method), +// so they are mocked out to keep the module graph loadable under ts-jest. +jest.mock('p-limit', () => ({ + __esModule: true, + default: () => (fn: () => unknown) => fn(), +})); +jest.mock('@sindresorhus/slugify', () => ({ + __esModule: true, + default: (input: string) => String(input), +})); + +import { promises as fs } from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { ImportAttachmentService } from './import-attachment.service'; + +/** + * Unit test for ImportAttachmentService.createDrawioSvg (issue #507). + * + * The Confluence import wraps a `.drawio` file into a `.drawio.svg` attachment. + * The `content=` payload MUST be the mxfile XML entity-escaped (draw.io's native + * form), NOT base64 — draw.io's editor decodes a base64 content= via Latin-1 + * atob, mangling every non-ASCII char into mojibake. createDrawioSvg touches no + * injected dependency, so the service is built with placeholder deps. + */ +describe('ImportAttachmentService.createDrawioSvg (#507)', () => { + const service = new ImportAttachmentService( + {} as any, + {} as any, + {} as any, + ); + const call = (p: string): Promise => + (service as any).createDrawioSvg(p); + + let tmpDir: string; + beforeAll(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'drawio-507-')); + }); + afterAll(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + const writeDrawio = async (name: string, xml: string): Promise => { + const p = path.join(tmpDir, name); + await fs.writeFile(p, xml, 'utf-8'); + return p; + }; + + it('writes content= as entity-encoded XML, not base64', async () => { + const drawio = + '' + + '' + + '' + + ''; + const p = await writeDrawio('cyrillic.drawio', drawio); + + const svg = (await call(p)).toString('utf-8'); + const content = /content="([^"]*)"/.exec(svg)?.[1]; + expect(content).toBeDefined(); + + // Entity-encoded XML form, starting with <mxfile — never a base64 blob. + expect(content).toMatch(/^<mxfile/); + expect(content).toContain('<'); + // Non-ASCII survives as raw UTF-8, with no Latin-1 mojibake. + expect(content).toContain('Старт-бит'); + expect(content).toContain('Схема — ёж'); + expect(content).not.toContain('Ð'); + + // Decoding the attribute (un-escaping) yields the original drawio file. + const decoded = content! + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/&/g, '&'); + expect(decoded).toBe(drawio); + }); + + it('escapes XML metacharacters in the drawio payload', async () => { + const drawio = '"q" <x>'; + const p = await writeDrawio('meta.drawio', drawio); + + const svg = (await call(p)).toString('utf-8'); + const content = /content="([^"]*)"/.exec(svg)?.[1]; + expect(content).toBeDefined(); + // The attribute value must contain no bare `<`, `>` or `"` that would break + // out of the content="..." attribute or the SVG element. + expect(content).not.toMatch(/[<>"]/); + const decoded = content! + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/&/g, '&'); + expect(decoded).toBe(drawio); + }); +}); diff --git a/apps/server/src/integrations/import/services/import-attachment.service.ts b/apps/server/src/integrations/import/services/import-attachment.service.ts index 9100149b..1b47ce4e 100644 --- a/apps/server/src/integrations/import/services/import-attachment.service.ts +++ b/apps/server/src/integrations/import/services/import-attachment.service.ts @@ -849,7 +849,12 @@ export class ImportAttachmentService { ): Promise { try { const drawioContent = await fs.readFile(drawioPath, 'utf-8'); - const drawioBase64 = Buffer.from(drawioContent).toString('base64'); + // Write the mxfile XML XML-entity-escaped (draw.io's native content= form), + // NOT base64. draw.io's editor decodes a base64 content= via Latin-1 atob + // (no UTF-8 step), turning every non-ASCII char (Cyrillic, ё, —) into + // mojibake; the entity-encoded form is decoded by the DOM as UTF-8 and + // opens intact. Docmost's own decoder reads both forms. + const drawioEscaped = this.xmlEscapeAttr(drawioContent); let imageElement = ''; // If we have a PNG, include it in the SVG @@ -875,7 +880,7 @@ export class ImportAttachmentService { width="600" height="400" viewBox="0 0 600 400" - content="${drawioBase64}">${imageElement}`; + content="${drawioEscaped}">${imageElement}`; return Buffer.from(svgContent, 'utf-8'); } catch (error) { @@ -884,6 +889,18 @@ export class ImportAttachmentService { } } + /** + * Escape a string so it is safe as the value of a double-quoted XML attribute + * (the `content=` payload of a `.drawio.svg`). Order matters: `&` first. + */ + private xmlEscapeAttr(s: string): string { + return s + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); + } + private async uploadWithRetry(opts: { abs: string; storageFilePath: string; diff --git a/packages/mcp/src/lib/drawio-xml.ts b/packages/mcp/src/lib/drawio-xml.ts index 84b2c5a3..f0a7e526 100644 --- a/packages/mcp/src/lib/drawio-xml.ts +++ b/packages/mcp/src/lib/drawio-xml.ts @@ -178,10 +178,11 @@ function sliceModel(xml: string): string | null { // --- decode chain ---------------------------------------------------------- /** - * Read the `content=` attribute out of a `.drawio.svg` string. Docmost stores a - * base64 payload there (createDrawioSvg); draw.io's own SVG export may store the - * XML entity-encoded instead. The DOM decodes entities for us, so the caller - * only has to distinguish "starts with '<'" (raw XML) from base64. + * Read the `content=` attribute out of a `.drawio.svg` string. Docmost writes + * the mxfile XML entity-encoded there (buildDrawioSvg / createDrawioSvg), which + * is also how draw.io's own SVG export stores it; older attachments stored a + * base64 payload instead. The DOM decodes entities for us, so the caller only + * has to distinguish "starts with '<'" (raw XML) from base64. */ export function extractContentAttr(svg: string): string { const { doc, error } = parseXml(svg); @@ -307,9 +308,16 @@ export function encodeDrawioFile(modelXml: string, title = "Page-1"): string { /** * Build the `diagram.drawio.svg` attachment. Mirrors the import service's * createDrawioSvg contract exactly: - * ${inner} + * ${inner} * plus width/height/viewBox from the diagram bounding box and the schematic * preview as the visible children (`inner`). + * + * The `content=` value is the mxfile XML XML-entity-escaped (draw.io's own + * native form), NOT base64. draw.io's editor decodes a base64 content= via + * Latin-1 atob (no UTF-8 step), turning every non-ASCII char (e.g. Cyrillic, + * ё, —) into mojibake; the entity-encoded form is decoded by the DOM as UTF-8 + * and opens intact. Our decoder (decodeDrawioSvg) reads both forms, so old + * base64 attachments still round-trip. */ export function buildDrawioSvg( modelXml: string, @@ -318,14 +326,14 @@ export function buildDrawioSvg( title = "Page-1", ): string { const file = encodeDrawioFile(modelXml, title); - const base64 = Buffer.from(file, "utf-8").toString("base64"); + const content = xmlEscape(file); const w = Math.max(1, Math.round(bbox.width)); const h = Math.max(1, Math.round(bbox.height)); return ( `${inner}` + `content="${content}">${inner}` ); } diff --git a/packages/mcp/test/unit/drawio-xml.test.mjs b/packages/mcp/test/unit/drawio-xml.test.mjs index 8d6e83ec..e5e5f746 100644 --- a/packages/mcp/test/unit/drawio-xml.test.mjs +++ b/packages/mcp/test/unit/drawio-xml.test.mjs @@ -51,6 +51,14 @@ const hasRule = (issues, rule, cellId) => (i) => i.rule === rule && (cellId === undefined || i.cellId === cellId), ); +// Reverse the attribute-value XML escaping used in the `content=` payload. +const unescapeAttr = (s) => + s + .replace(/</g, "<") + .replace(/>/g, ">") + .replace(/"/g, '"') + .replace(/&/g, "&"); + // --- style parsing --------------------------------------------------------- test("parseStyle: base stylename + key=value pairs", () => { @@ -335,16 +343,20 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG // The outer content="..." attribute must not be broken by the title: the raw // title metacharacters never appear literally in the SVG markup (they are - // base64-encoded inside content=, and escaped inside the file XML). + // entity-escaped inside content=, doubly so where the file XML already escaped + // them inside name="..."). const contentMatch = /content="([^"]*)"/.exec(svg); assert.ok(contentMatch, "SVG has a single well-formed content= attribute"); // The diagram model still decodes losslessly despite the exotic title. assert.equal(decodeDrawioSvg(svg), model); + // content= is now entity-encoded XML (draw.io's native form), never base64. + assert.match(contentMatch[1], /^<mxfile/); + // The file XML is well-formed: the title lives in name="..." as escaped // entities, so unescaping recovers the original title byte-for-byte. - const fileXml = Buffer.from(contentMatch[1], "base64").toString("utf-8"); + const fileXml = unescapeAttr(contentMatch[1]); const nameMatch = //.exec(fileXml); assert.ok(nameMatch, "the diagram name attribute is intact and quote-safe"); const decodedTitle = nameMatch[1] @@ -358,3 +370,68 @@ test("encode/build: a title with < > \" & round-trips without corrupting the SVG const file = encodeDrawioFile(model, title); assert.match(file, /name="A < B > C " D & E">/); }); + +// --- #507: content= is entity-encoded XML, never base64 -------------------- + +// A model whose cell values carry Cyrillic, ё and an em dash — exactly the +// characters draw.io's Latin-1 atob mangles when content= is base64. +const CYRILLIC_MODEL = + "" + + '' + + '' + + '' + + ""; + +test("#507: buildDrawioSvg writes content= as entity-encoded XML (not base64)", () => { + const model = normalizeXml(CYRILLIC_MODEL); + const svg = buildDrawioSvg(model, "", { width: 200, height: 120 }, "Диаграмма"); + + const contentMatch = /content="([^"]*)"/.exec(svg); + assert.ok(contentMatch, "SVG has a single well-formed content= attribute"); + const content = contentMatch[1]; + + // The content= value is the entity-encoded mxfile XML — starts with `<mxfile`. + assert.match(content, /^<mxfile/, "content= is entity-encoded mxfile XML"); + // It must NOT be a base64 blob: base64 has no XML entities and no literal `<`. + assert.ok(content.includes("<"), "content= carries XML entities, not base64"); + + // Cyrillic / ё / — survive verbatim in the attribute (raw UTF-8, not atob-mangled). + assert.ok(content.includes("Старт-бит — ёж"), "non-ASCII value is raw UTF-8 in content="); + assert.ok(content.includes("Диаграмма"), "non-ASCII title is raw UTF-8 in content="); + // The mojibake that base64+atob would have produced must be absent. + assert.ok(!content.includes("Ð"), "no Latin-1 mojibake in content="); +}); + +test("#507: Cyrillic model round-trips byte-stable through buildDrawioSvg -> decodeDrawioSvg", () => { + const model = normalizeXml(CYRILLIC_MODEL); + const svg = buildDrawioSvg(model, "", { width: 200, height: 120 }, "Заголовок — ё"); + assert.equal(decodeDrawioSvg(svg), model); +}); + +test("#507 back-compat: an OLD base64-form .drawio.svg still decodes losslessly", () => { + const model = normalizeXml(CYRILLIC_MODEL); + // Reproduce the pre-fix write path: encodeDrawioFile -> base64 in content=. + const file = encodeDrawioFile(model, "Старая диаграмма"); + const base64 = Buffer.from(file, "utf-8").toString("base64"); + const svg = + ``; + // No XML entities, purely base64 alphabet — this is the legacy form. + assert.ok(!base64.includes("<") && !base64.includes("&")); + assert.equal(decodeDrawioSvg(svg), model); +}); + +test("#507 negative: non-ASCII in a cell value AND in the title round-trip clean", () => { + const model = normalizeXml(CYRILLIC_MODEL); + const title = "Тест — ёмкость № 5"; + const svg = buildDrawioSvg(model, "", { width: 200, height: 120 }, title); + + // Model recovered byte-for-byte. + assert.equal(decodeDrawioSvg(svg), model); + + // Title lands in the of the decoded file XML, intact. + const content = /content="([^"]*)"/.exec(svg)[1]; + const fileXml = unescapeAttr(content); + const nameMatch = //.exec(fileXml); + assert.ok(nameMatch, "diagram name attribute present"); + assert.equal(unescapeAttr(nameMatch[1]), title); +}); From 846341d7d476d5903b59e6e24d5350592b2d592f Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 03:44:50 +0300 Subject: [PATCH 2/2] fix(drawio): encode literal tab/newline/CR in content= as numeric char-refs (#507 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up to the base64→entity-XML content= switch. The whole mxfile XML now lives in one content="..." attribute; XML attribute-value normalization collapses a LITERAL tab/newline/CR to a single space on DOM read (jsdom and the real draw.io editor alike), silently flattening multi-line labels and tab-bearing values that the old base64 form stored verbatim. Finding 1 (data-loss): both encode paths — buildDrawioSvg's xmlEscape (mcp) and the import service's escape — now append / / after the four &<>" replaces (numeric char-refs survive normalization, as draw.io's own export does). The extractContentAttr regex fallback now decodes those char-refs (hex case-insensitive plus decimal / / ) so it agrees with the DOM path; & stays decoded last so an escaped &#x9; reads back as literal text. Finding 2 (dedup): the server's private xmlEscapeAttr is replaced by the shared htmlEscape helper (& < > " ' — a strict superset, the extra ' is harmless in a "-delimited value) wrapped in xmlEscapeContent, which adds the three control-char char-refs on top (htmlEscape does not escape them). Finding 3 (docs): narrow the CHANGELOG healing claim — only a diagram still holding its original correct-UTF-8 base64 (not yet opened/autosaved) is recoverable; one already opened in the editor persisted mojibake at rest and its text is lost. Tests: new mcp round-trip test with literal tab/newline/CR in a value (DOM path, byte-stable) plus a fallback-branch test forcing a malformed wrapper so both decode paths are proven to agree; new server spec asserting char-ref encoding. Mutation-checked: dropping the encode replaces reddens both new mcp tests; dropping only the fallback decode reddens just the fallback test. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 12 +++-- ...port-attachment.service.drawio-svg.spec.ts | 35 +++++++++++++++ .../services/import-attachment.service.ts | 23 ++++++---- packages/mcp/src/lib/drawio-xml.ts | 23 +++++++++- packages/mcp/test/unit/drawio-xml.test.mjs | 44 +++++++++++++++++++ 5 files changed, 123 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a6a51a0..53dfdcd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -403,10 +403,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 as XML-entity-escaped mxfile XML — draw.io's own native form, decoded by the DOM as UTF-8 — so labels open intact. The decoder reads both the new entity-encoded form and the old base64 form, so existing diagrams still open. - *Healing pre-fix diagrams:* any agent-created cyrillic diagram written before - this fix is repaired in place by `drawioGet` → `drawioUpdate` with the same - XML (rewrites the attachment in the new form); no migration script is needed. - (#507) + *Healing pre-fix diagrams:* only a diagram that still holds its original + (correct-UTF-8) base64 — i.e. one not yet opened/autosaved in the draw.io + editor — can be repaired in place by `drawioGet` → `drawioUpdate` with the + same XML (rewrites the attachment in the new form); no migration script is + needed. A diagram that was already opened in the editor persisted the + mojibake at rest, so `drawioGet` reads the already-corrupted text and + `drawioUpdate` faithfully rewrites it — that text is lost and is not + recoverable by a rewrite. (#507) - **A chat with one malformed message part no longer 500s on every turn, and a failed send no longer duplicates the user's message.** Incoming client parts are now whitelisted to `text` (a forged tool-result part can no longer reach diff --git a/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts b/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts index 00735ea2..ebe67431 100644 --- a/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts +++ b/apps/server/src/integrations/import/services/import-attachment.service.drawio-svg.spec.ts @@ -76,6 +76,41 @@ describe('ImportAttachmentService.createDrawioSvg (#507)', () => { expect(decoded).toBe(drawio); }); + it('encodes literal tab/newline/CR as numeric char-refs, not literal control chars (#507 F1)', async () => { + // A literal tab/newline/CR inside the mxfile XML would be collapsed to a + // single space by XML attribute-value normalization when the draw.io editor + // reads content=, silently flattening multi-line labels and tab-bearing + // values. They must be emitted as numeric char-refs instead. + const drawio = + '' + + '' + + '' + + '' + + ''; + const p = await writeDrawio('ctrl.drawio', drawio); + + const svg = (await call(p)).toString('utf-8'); + const content = /content="([^"]*)"/.exec(svg)?.[1]; + expect(content).toBeDefined(); + // No literal control chars survive in the attribute value. + expect(content).not.toMatch(/[\t\n\r]/); + // They round-trip as numeric char-refs. + expect(content).toContain(' '); + expect(content).toContain(' '); + expect(content).toContain(' '); + // Decoding (char-refs back to literal, entities back) recovers the file. + const decoded = content! + .replace(/</g, '<') + .replace(/>/g, '>') + .replace(/"/g, '"') + .replace(/'/g, "'") + .replace(/ /gi, '\t') + .replace(/ /gi, '\n') + .replace(/ /gi, '\r') + .replace(/&/g, '&'); + expect(decoded).toBe(drawio); + }); + it('escapes XML metacharacters in the drawio payload', async () => { const drawio = '"q" <x>'; const p = await writeDrawio('meta.drawio', drawio); diff --git a/apps/server/src/integrations/import/services/import-attachment.service.ts b/apps/server/src/integrations/import/services/import-attachment.service.ts index 1b47ce4e..3fa94838 100644 --- a/apps/server/src/integrations/import/services/import-attachment.service.ts +++ b/apps/server/src/integrations/import/services/import-attachment.service.ts @@ -8,6 +8,7 @@ import { createReadStream } from 'node:fs'; import { promises as fs } from 'fs'; import { Readable } from 'stream'; import { getMimeType, sanitizeFileName } from '../../../common/helpers'; +import { htmlEscape } from '../../../common/helpers/html-escaper'; import { v7 } from 'uuid'; import { FileTask } from '@docmost/db/types/entity.types'; import { getAttachmentFolderPath } from '../../../core/attachment/attachment.utils'; @@ -854,7 +855,7 @@ export class ImportAttachmentService { // (no UTF-8 step), turning every non-ASCII char (Cyrillic, ё, —) into // mojibake; the entity-encoded form is decoded by the DOM as UTF-8 and // opens intact. Docmost's own decoder reads both forms. - const drawioEscaped = this.xmlEscapeAttr(drawioContent); + const drawioEscaped = this.xmlEscapeContent(drawioContent); let imageElement = ''; // If we have a PNG, include it in the SVG @@ -891,14 +892,20 @@ export class ImportAttachmentService { /** * Escape a string so it is safe as the value of a double-quoted XML attribute - * (the `content=` payload of a `.drawio.svg`). Order matters: `&` first. + * (the `content=` payload of a `.drawio.svg`). The shared `htmlEscape` covers + * `& < > " '` (a strict superset of what this attribute needs; the extra `'` + * escape is harmless in a `"`-delimited value). On top of that, the numeric + * char-refs for tab/newline/CR are required: a literal tab/newline/CR inside + * an attribute value is collapsed to a single space by XML attribute-value + * normalization on DOM read (both our decoder and the real draw.io editor), + * silently flattening multi-line labels and tab-bearing values. Char-refs + * survive that normalization (#507). */ - private xmlEscapeAttr(s: string): string { - return s - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"'); + private xmlEscapeContent(s: string): string { + return htmlEscape(s) + .replace(/\t/g, ' ') + .replace(/\n/g, ' ') + .replace(/\r/g, ' '); } private async uploadWithRetry(opts: { diff --git a/packages/mcp/src/lib/drawio-xml.ts b/packages/mcp/src/lib/drawio-xml.ts index f0a7e526..620b12ea 100644 --- a/packages/mcp/src/lib/drawio-xml.ts +++ b/packages/mcp/src/lib/drawio-xml.ts @@ -196,12 +196,23 @@ export function extractContentAttr(svg: string): string { // value itself never contains a double-quote (base64 / entity-encoded XML). const m = /content="([^"]*)"/.exec(svg); if (m) { - // Decode the handful of XML entities a raw regex would leave encoded. + // Decode the handful of XML entities a raw regex would leave encoded. The + // numeric char-refs for tab/newline/CR MUST be decoded here too: the DOM + // path above turns them back into the literal control chars, so this + // regex fallback has to agree or the two decode paths diverge (#507). + // `&` is decoded last so an escaped `&#x9;` reads back as the + // literal text ` `, not a tab. return m[1] .replace(/</g, "<") .replace(/>/g, ">") .replace(/"/g, '"') .replace(/'/g, "'") + .replace(/ /gi, "\t") + .replace(/ /g, "\t") + .replace(/ /gi, "\n") + .replace(/ /g, "\n") + .replace(/ /gi, "\r") + .replace(/ /g, "\r") .replace(/&/g, "&"); } throw new Error("drawio: SVG has no content= attribute to decode"); @@ -342,7 +353,15 @@ function xmlEscape(s: string): string { .replace(/&/g, "&") .replace(//g, ">") - .replace(/"/g, """); + .replace(/"/g, """) + // A literal tab/newline/CR inside an attribute value is collapsed to a + // single space by XML attribute-value normalization on DOM read (both jsdom + // here and the real draw.io editor), silently flattening multi-line labels + // and tab-bearing values. Numeric char-refs survive that normalization, so + // emit them the way draw.io's own native export does (#507). + .replace(/\t/g, " ") + .replace(/\n/g, " ") + .replace(/\r/g, " "); } // --- normalization + hash -------------------------------------------------- diff --git a/packages/mcp/test/unit/drawio-xml.test.mjs b/packages/mcp/test/unit/drawio-xml.test.mjs index e5e5f746..3295b0f8 100644 --- a/packages/mcp/test/unit/drawio-xml.test.mjs +++ b/packages/mcp/test/unit/drawio-xml.test.mjs @@ -435,3 +435,47 @@ test("#507 negative: non-ASCII in a cell value AND in the title round-trip clean assert.ok(nameMatch, "diagram name attribute present"); assert.equal(unescapeAttr(nameMatch[1]), title); }); + +// --- #507 F1: literal tab/newline/CR in an attribute value survive the +// content= round-trip. The whole mxfile XML lives in one content="..." attr; +// XML attribute-value normalization collapses a LITERAL tab/newline/CR to a +// single space on DOM read, so the escape must emit numeric char-refs instead. +const CTRL_MODEL = + "" + + '' + + '' + + '' + + '' + + '' + + ""; + +test("#507 F1: literal tab/newline/CR in a value round-trip byte-stable (DOM decode)", () => { + const svg = buildDrawioSvg(CTRL_MODEL, "", { width: 200, height: 200 }); + const content = /content="([^"]*)"/.exec(svg)[1]; + // The tab/newline/CR must be emitted as numeric char-refs, never as literal + // control chars (which DOM attribute-value normalization would eat). + assert.ok( + !/[\t\n\r]/.test(content), + "no literal tab/newline/CR survive in the content= attribute", + ); + assert.ok( + content.includes(" ") && + content.includes(" ") && + content.includes(" "), + "tab/newline/CR are emitted as numeric char-refs", + ); + // Full DOM decode recovers the model byte-for-byte, control chars intact. + assert.equal(decodeDrawioSvg(svg), CTRL_MODEL); +}); + +test("#507 F1: regex fallback decodes tab/newline/CR char-refs (agrees with DOM path)", () => { + const svg = buildDrawioSvg(CTRL_MODEL, "", { width: 200, height: 200 }); + const content = /content="([^"]*)"/.exec(svg)[1]; + // A bare `&` makes the SVG wrapper malformed, forcing extractContentAttr onto + // its regex fallback branch. That branch must decode the tab/newline/CR + // char-refs exactly like the DOM path, or the two decoders diverge. + const malformedSvg = `&`; + assert.equal(decodeDrawioSvg(malformedSvg), CTRL_MODEL); + // The well-formed (DOM) path yields the identical result. + assert.equal(decodeDrawioSvg(svg), CTRL_MODEL); +});