fix(drawio): encode literal tab/newline/CR in content= as numeric char-refs (#507 review)

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) <noreply@anthropic.com>
2026-07-12 03:44:50 +03:00
parent 355bb81a20
commit 1edc8b8dad
5 changed files with 123 additions and 14 deletions
@@ -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 =
+      '<mxfile><diagram name="p">' +
+      '<mxGraphModel><root><mxCell id="0"/>' +
+      '<mxCell id="2" value="col1\tcol2" style="html=1;\nshadow=0" vertex="1" parent="0"/>' +
+      '<mxCell id="3" value="Line1\nLine2\rLine3" vertex="1" parent="0"/>' +
+      '</root></mxGraphModel></diagram></mxfile>';
+    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('&#x9;');
+    expect(content).toContain('&#xa;');
+    expect(content).toContain('&#xd;');
+    // Decoding (char-refs back to literal, entities back) recovers the file.
+    const decoded = content!
+      .replace(/&lt;/g, '<')
+      .replace(/&gt;/g, '>')
+      .replace(/&quot;/g, '"')
+      .replace(/&#39;/g, "'")
+      .replace(/&#x9;/gi, '\t')
+      .replace(/&#xa;/gi, '\n')
+      .replace(/&#xd;/gi, '\r')
+      .replace(/&amp;/g, '&');
+    expect(decoded).toBe(drawio);
+  });
+
  it('escapes XML metacharacters in the drawio payload', async () => {
    const drawio = '<mxfile><diagram name="a &amp; b">"q" &lt;x&gt;</diagram></mxfile>';
    const p = await writeDrawio('meta.drawio', drawio);
@@ -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, '&amp;')
-      .replace(/</g, '&lt;')
-      .replace(/>/g, '&gt;')
-      .replace(/"/g, '&quot;');
+  private xmlEscapeContent(s: string): string {
+    return htmlEscape(s)
+      .replace(/\t/g, '&#x9;')
+      .replace(/\n/g, '&#xa;')
+      .replace(/\r/g, '&#xd;');
  }

  private async uploadWithRetry(opts: {