From c48c9a6ce9d087a3562be46d5e31fc22968b8f90 Mon Sep 17 00:00:00 2001 From: "heecheol.park" Date: Thu, 23 Jul 2026 23:40:16 +0900 Subject: [PATCH 1/2] fix: escape passthrough tags restored into attribute values Whitelisted inline tags (br|u|sub|sup|mark|details|summary) are stashed as placeholders before markdown rendering and restored across the whole HTML string. When a placeholder rendered into a link/image title attribute, the raw tag was restored verbatim, injecting unescaped <>"& into the attribute value and producing storage XML that Confluence rejects. Detect the attribute context at restore time (nearest preceding angle bracket opens a tag) and XML-escape the restored value there. Body-text placeholders are left raw, so passthrough output stays byte-identical. Covers all seven whitelisted tags. Adds regression tests for link title
, image title , a mixed body+title case, and a well-formed XML assertion. Co-Authored-By: Claude Opus 4.8 --- lib/macro-converter.js | 31 +++++++++++++++- tests/macro-converter.test.js | 70 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/lib/macro-converter.js b/lib/macro-converter.js index ab4c977..49504db 100644 --- a/lib/macro-converter.js +++ b/lib/macro-converter.js @@ -30,6 +30,19 @@ const PASSTHROUGH_BLOCK_RE = /<(svg|div)(?:\s[^>]*)?>[\s\S]*?<\/\1>/gi; // continuation that happens to align to four spaces. const INLINE_CODE_RE = /`[^`\n]+`/g; +// Escapes a restored passthrough tag for safe inclusion inside a double-quoted +// XML attribute value. Only used when a stash placeholder resolved into an +// attribute context (see `_renderMarkdownToHtml`); text-node placeholders are +// left raw. `&` is escaped first so the entity ampersands below are not +// double-escaped. +function escapeXmlAttr(str) { + return str + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"'); +} + class MacroConverter { constructor({ isCloud = false, webUrlPrefix = '', buildUrl = null, linkStyle = null } = {}) { this._isCloud = isCloud; @@ -128,7 +141,23 @@ class MacroConverter { const html = this.markdown.render(src); return html.replace( new RegExp(`${STASH_DELIM}H(\\d+)${STASH_DELIM}`, 'g'), - (m, i) => htmlStash[+i] ?? m, + (m, i, offset, str) => { + const raw = htmlStash[+i]; + if (raw == null) return m; + // A placeholder can render into an attribute value (e.g. a link/image + // `title` built from `[x](url "
")`). Restoring the raw tag there + // would inject unescaped `<>"&` into an attribute, producing invalid + // storage XML that Confluence rejects. Detect the attribute context by + // checking whether the nearest preceding angle bracket opens a tag + // (`<` after the last `>`), and XML-escape the restored value if so. + // Text-node placeholders keep their raw tag, so body passthrough is + // byte-identical. + const before = str.slice(0, offset); + if (before.lastIndexOf('<') > before.lastIndexOf('>')) { + return escapeXmlAttr(raw); + } + return raw; + }, ); } diff --git a/tests/macro-converter.test.js b/tests/macro-converter.test.js index 527b9a8..93be57f 100644 --- a/tests/macro-converter.test.js +++ b/tests/macro-converter.test.js @@ -1143,6 +1143,76 @@ describe('MacroConverter
passthrough', () => { }); }); +describe('MacroConverter passthrough tags in link/image title attributes', () => { + // Whitelisted inline tags (br|u|sub|sup|mark|details|summary) are stashed as + // placeholders before rendering and restored across the whole HTML string. + // When a placeholder lands inside an attribute value (a link/image `title`), + // restoring the raw tag would inject unescaped `<>"&` and produce storage XML + // that Confluence rejects. The restore step must XML-escape those, while + // leaving body-text passthrough byte-identical (see the
passthrough suite + // above, which must stay green). + const converter = new MacroConverter({ isCloud: true }); + + // Focused well-formedness check for this defect class: every attribute value + // must be free of raw `<`/`>`. htmlparser2 (xmlMode) is too lenient to flag + // these, so we scan attribute values directly. + const assertAttrsWellFormed = (xml) => { + const attrRe = /\b[\w:-]+\s*=\s*"([^"]*)"/g; + let m; + while ((m = attrRe.exec(xml)) !== null) { + expect(m[1]).not.toMatch(/[<>]/); + } + }; + + test('link title with
is XML-escaped, not restored raw', () => { + const result = converter.markdownToStorage('[x](https://example.com "
")'); + expect(result).toContain('title="<br>"'); + expect(result).not.toContain('title="
"'); + assertAttrsWellFormed(result); + }); + + test('image title with ... is XML-escaped, not restored raw', () => { + const result = converter.markdownToStorage('![img](https://example.com/i.png "capx")'); + expect(result).toContain('title="cap<sub>x</sub>"'); + expect(result).not.toContain('title="cap'); + assertAttrsWellFormed(result); + }); + + test('same tag in body AND title: body stays raw XHTML, title gets escaped', () => { + const result = converter.markdownToStorage('Line a
b then [x](https://example.com "
")'); + // body passthrough unchanged (self-closed by markdown/storage pipeline) + expect(result).toContain('a
b'); + // attribute context escaped + expect(result).toContain('title="<br>"'); + assertAttrsWellFormed(result); + }); + + test('all seven whitelisted tags in a title attribute are escaped', () => { + const tags = ['br', 'u', 'sub', 'sup', 'mark', 'details', 'summary']; + for (const tag of tags) { + const result = converter.markdownToStorage(`[x](https://example.com "a<${tag}>b")`); + expect(result).toContain(`title="a<${tag}>b"`); + assertAttrsWellFormed(result); + } + }); + + test('output with escaped title parses as well-formed XML', () => { + const htmlparser2 = require('htmlparser2'); + const result = converter.markdownToStorage('![img](https://example.com/i.png "capx") and text a
b'); + let error = null; + const parser = new htmlparser2.Parser( + { onerror: (e) => { error = e; } }, + { xmlMode: true, recognizeSelfClosing: true } + ); + parser.write(`${result}`); + parser.end(); + expect(error).toBeNull(); + // xmlMode is lenient about attribute contents, so also assert no raw + // angle brackets survive inside any attribute value. + assertAttrsWellFormed(result); + }); +}); + describe('MacroConverter storageToMarkdown panel formatting', () => { const converter = new MacroConverter({ isCloud: true }); From b869b1ba9dc75f07904163d90a82bba97e9670a7 Mon Sep 17 00:00:00 2001 From: "heecheol.park" Date: Thu, 23 Jul 2026 23:44:52 +0900 Subject: [PATCH 2/2] no-mistakes(review): Optimize passthrough placeholder context scanning --- lib/macro-converter.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/macro-converter.js b/lib/macro-converter.js index 49504db..c47f154 100644 --- a/lib/macro-converter.js +++ b/lib/macro-converter.js @@ -139,9 +139,17 @@ class MacroConverter { src += stashHtml(markdown.slice(pos)); const html = this.markdown.render(src); + let contextOffset = 0; + let isInsideTag = false; return html.replace( new RegExp(`${STASH_DELIM}H(\\d+)${STASH_DELIM}`, 'g'), - (m, i, offset, str) => { + (m, i, offset) => { + for (; contextOffset < offset; contextOffset++) { + if (html[contextOffset] === '<') isInsideTag = true; + else if (html[contextOffset] === '>') isInsideTag = false; + } + contextOffset = offset + m.length; + const raw = htmlStash[+i]; if (raw == null) return m; // A placeholder can render into an attribute value (e.g. a link/image @@ -152,11 +160,7 @@ class MacroConverter { // (`<` after the last `>`), and XML-escape the restored value if so. // Text-node placeholders keep their raw tag, so body passthrough is // byte-identical. - const before = str.slice(0, offset); - if (before.lastIndexOf('<') > before.lastIndexOf('>')) { - return escapeXmlAttr(raw); - } - return raw; + return isInsideTag ? escapeXmlAttr(raw) : raw; }, ); }