From 507db38404bc49c81dabd19e11eb437f0aab8e8c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 14 Aug 2026 23:00:10 -0400 Subject: [PATCH] fix(#3497): unescape double-quoted scalars on parse so round-trips stop doubling backslashes (#3521) * fix(#3497): unescape double-quoted scalars on parse so round-trips stop doubling backslashes * chore(#3497): add changeset fragment for PR #3521 --------- Co-authored-by: sim --- .../3497-frontmatter-escape-amplification.md | 5 + src/frontmatter.cts | 62 ++++++++++- tests/frontmatter.unit.test.cjs | 103 ++++++++++++++++++ 3 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 .changeset/3497-frontmatter-escape-amplification.md diff --git a/.changeset/3497-frontmatter-escape-amplification.md b/.changeset/3497-frontmatter-escape-amplification.md new file mode 100644 index 000000000..0722ae200 --- /dev/null +++ b/.changeset/3497-frontmatter-escape-amplification.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3521 +--- +**Frontmatter round-trips no longer double backslashes on every state write** — `escapeDoubleQuoted` escaped `\`, `"`, and control characters on each serialize while the parser only stripped the outer quote delimiters, so every read-modify-write cycle doubled existing escapes (2ⁿ−1 backslashes after n cycles). `syncStateFrontmatter` carries `last_activity_desc` through that seam on every state command, growing STATE.md unboundedly — the reported 134 MB file OOMed `state.record-session` after 26 writes. Double-quoted scalars are now un-escaped on parse via the exact inverse of the escaper, making serialize→parse a fixed point; unrecognized escapes are kept literally so hand-authored files parse unchanged. (#3497) diff --git a/src/frontmatter.cts b/src/frontmatter.cts index 3e3fd2cc8..95f0696aa 100644 --- a/src/frontmatter.cts +++ b/src/frontmatter.cts @@ -108,6 +108,64 @@ function isFrontmatterShaped(region: string): boolean { const FULL_LINE_COMMENTS = Symbol('fullLineComments'); type FullLineCommentChannel = { leading: Record; trailing: string[] }; +/** + * Unescape the interior of a YAML double-quoted scalar — the exact inverse of + * `escapeDoubleQuoted` (#3497). The writer has escaped `\`/`"`/`\n`/`\t`/`\r`/ + * `\xHH` since #1779, but the reader only stripped the delimiters, so + * parse(serialize(x)) ≠ x for any quoted scalar carrying a `"` or `\`: each + * read-modify-write round-trip doubled the backslashes (b → 2b+1), growing a + * repeatedly-synced field — and its document — without bound until tooling + * OOMed. Recognized escapes decode per YAML double-quoted semantics; an + * unrecognized `\c` is kept literally (backslash + char), matching the + * strip-only behavior hand-authored files had before this fix. + */ +function unescapeDoubleQuoted(s: string): string { + let out = ''; + for (let i = 0; i < s.length; i++) { + const ch = s[i]; + if (ch !== '\\' || i === s.length - 1) { + out += ch; + continue; + } + const next = s[++i]; + if (next === '\\' || next === '"') { + out += next; + } else if (next === 'n') { + out += '\n'; + } else if (next === 't') { + out += '\t'; + } else if (next === 'r') { + out += '\r'; + } else if (next === 'x') { + const hex = s.slice(i + 1, i + 3); + if (/^[0-9a-fA-F]{2}$/.test(hex)) { + out += String.fromCharCode(parseInt(hex, 16)); + i += 2; + } else { + out += '\\x'; // not \xHH — keep literally + } + } else { + out += '\\' + next; // unrecognized escape — keep literally + } + } + return out; +} + +/** + * Strip the quote delimiters off a parsed YAML scalar, un-escaping the interior + * when the scalar is double-quoted (#3497 — the parse-side complement of + * `escapeDoubleQuoted`). Single-quoted scalars keep the historical strip-only + * behavior (the writer never emits them; `''` → `'` folding is out of scope). + * A scalar wrapped in double quotes un-escapes; anything else keeps the exact + * prior delimiter-strip behavior, including a stray unpaired boundary quote. + */ +function parseQuotedScalar(value: string): string { + if (value.length >= 2 && value.startsWith('"') && value.endsWith('"')) { + return unescapeDoubleQuoted(value.slice(1, -1)); + } + return value.replace(/^["']|["']$/g, ''); +} + /** * Parse one already-delimited YAML region into a Frontmatter object. * @@ -173,12 +231,12 @@ function parseYamlRegion(yaml: string): Frontmatter { current.key = null; } else { // Simple key: value - (current.obj as Record)[key] = value.replace(/^["']|["']$/g, ''); + (current.obj as Record)[key] = parseQuotedScalar(value); current.key = null; } } else if (line.trim().startsWith('- ')) { // Array item - const itemValue = line.trim().slice(2).replace(/^["']|["']$/g, ''); + const itemValue = parseQuotedScalar(line.trim().slice(2)); // If current context is an empty object, convert to array if (typeof current.obj === 'object' && !Array.isArray(current.obj) && Object.keys(current.obj).length === 0) { diff --git a/tests/frontmatter.unit.test.cjs b/tests/frontmatter.unit.test.cjs index 2b1cf20ab..54e7d82dc 100644 --- a/tests/frontmatter.unit.test.cjs +++ b/tests/frontmatter.unit.test.cjs @@ -1254,6 +1254,109 @@ describe('reconstructFrontmatter: strict-YAML round-trip (#1779)', () => { }); }); +// #3497 — escape amplification. `escapeDoubleQuoted` escapes `\`/`"`/control +// chars on every serialize (#1779), but `parseYamlRegion` only stripped the +// outer quote delimiters and never un-escaped the interior, so parse ∘ serialize +// was NOT the identity: every read-modify-write cycle doubled the backslashes +// (b → 2b+1, i.e. 2ⁿ−1 after n round-trips). A `last_activity_desc` containing +// one embedded quote grew STATE.md to 134 MB in 26 state writes and OOMed +// state.record-session. These tests pin the lossy-parser round trip (the actual +// write seam: extractFrontmatter → reconstructFrontmatter/spliceFrontmatter), +// not the strict-YAML path — the amplification lived in the project's own +// parse/serialize pair. +describe('frontmatter round-trip: escape amplification (#3497)', () => { + // One full document round trip through the production write seam: + // parse the frontmatter out of the document, re-serialize it back in. + const roundTripDoc = (content) => { + const fm = extractFrontmatter(content); + return `---\n${reconstructFrontmatter(fm)}\n---\nbody\n`; + }; + + test('single round trip is byte-exact for embedded double quotes', () => { + const original = 'Fixed "the bug" in parser'; + const fm = extractFrontmatter(`---\ndesc: "Fixed \\"the bug\\" in parser"\n---\nbody\n`); + assert.equal(fm.desc, original); + assert.equal(reconstructFrontmatter({ desc: original }), 'desc: "Fixed \\"the bug\\" in parser"'); + }); + + test('single round trip is byte-exact for embedded backslash', () => { + const original = 'path a:\\b\\c plus "quote"'; + const fm = extractFrontmatter(`---\ndesc: "path a:\\\\b\\\\c plus \\"quote\\""\n---\nbody\n`); + assert.equal(fm.desc, original); + assert.equal(extractFrontmatter(`---\n${reconstructFrontmatter({ desc: original })}\n---\n`).desc, original); + }); + + test('single round trip is byte-exact for newline/tab/CR/control chars', () => { + const original = 'l1\nl2\ttab\rCR\x00NUL\x7fDEL'; + const serialized = reconstructFrontmatter({ desc: original }); + assert.equal(extractFrontmatter(`---\n${serialized}\n---\n`).desc, original); + }); + + test('repeated serialize→parse cycles do not grow (26 cycles, the reported OOM window)', () => { + let content = '---\ndesc: "he said \\"hi\\" and \\"bye\\""\n---\nbody\n'; + const firstPass = roundTripDoc(content); + let current = firstPass; + for (let i = 0; i < 26; i++) { + current = roundTripDoc(current); + // After the first cycle the document must be a fixed point: byte-identical + // forever. Before the fix, backslashes followed b → 2b+1 and unbounded + // cycling hit a 134 M-char line within 26 passes (asserting per cycle, + // rather than only after the loop, so the buggy failure is a small clear + // diff on cycle 1 instead of a runner OOM). + assert.equal(current, firstPass, `cycle ${i + 1} changed the document`); + } + assert.equal(extractFrontmatter(current).desc, 'he said "hi" and "bye"'); + }); + + test('block array items with quotes/backslashes round-trip and stay stable', () => { + // 4 items force the block `- "..."` form (inline caps at 3), which has its + // own escape call site and its own quote-strip on parse. + const original = ['a: "1"', 'b:\\path "x"', 'c: "3"', 'd: "4"']; + const serialized = reconstructFrontmatter({ tags: original }); + assert.deepEqual(extractFrontmatter(`---\n${serialized}\n---\n`).tags, original); + const reparsed = extractFrontmatter(`---\n${serialized}\n---\n`); + assert.equal(reconstructFrontmatter(reparsed), serialized); + }); + + test('nested object subvalue with quotes/backslashes round-trips and stays stable', () => { + const original = { meta: { note: 'see: "the \\\\docs\\\\"' } }; + const serialized = reconstructFrontmatter(original); + assert.deepEqual(extractFrontmatter(`---\n${serialized}\n---\n`).meta, original.meta); + const reparsed = extractFrontmatter(`---\n${serialized}\n---\n`); + assert.equal(reconstructFrontmatter(reparsed), serialized); + }); + + test('spliceFrontmatter write path is a fixed point under repeated no-op writes', () => { + // The state.cjs seam: read file → merge → spliceFrontmatter → write. + // Repeating it must not change bytes once the first write lands. + let content = `---\ndesc: "he said \\"hi\\""\nphase: executing\n---\nbody\n`; + content = spliceFrontmatter(content, { ...extractFrontmatter(content), phase: 'executing' }); + const firstWrite = content; + for (let i = 0; i < 10; i++) { + content = spliceFrontmatter(content, { ...extractFrontmatter(content), phase: 'executing' }); + } + assert.equal(content, firstWrite); + assert.equal(extractFrontmatter(content).desc, 'he said "hi"'); + }); + + test('plain scalars that never need quoting still round-trip unquoted and unchanged', () => { + const obj = { name: 'simple-value', wave: 'W1', count: '42' }; + const serialized = reconstructFrontmatter(obj); + assert.equal(serialized, 'name: simple-value\nwave: W1\ncount: 42'); + assert.deepEqual(extractFrontmatter(`---\n${serialized}\n---\n`), obj); + }); + + test('single-quoted scalars keep their existing parse behavior (quotes stripped, no escape processing)', () => { + // The writer never emits single-quoted output; hand-authored files keep + // the historical strip-only behavior. A single-quoted scalar has no + // escape processing in YAML (only '' → '), and changing that is out of + // scope for #3497 — this pins the current contract so the unescape fix + // cannot silently broaden into single-quote handling. + const fm = extractFrontmatter("---\ntitle: 'C:\\real\\path'\n---"); + assert.equal(fm.title, 'C:\\real\\path'); + }); +}); + describe('extractFrontmatter: boundary — dash at start of file', () => { test('--- at byte 0 is treated as frontmatter', () => { const result = extractFrontmatter('---\nkey: val\n---\n');