Skip to content

fix(markdown): restore placeholder blocks with a function replacer#5664

Open
wbaxterh wants to merge 1 commit into
odysseus-dev:devfrom
wbaxterh:fix/markdown-placeholder-dollar-restore
Open

fix(markdown): restore placeholder blocks with a function replacer#5664
wbaxterh wants to merge 1 commit into
odysseus-dev:devfrom
wbaxterh:fix/markdown-placeholder-dollar-restore

Conversation

@wbaxterh

Copy link
Copy Markdown
Contributor

Summary

The placeholder-restore pass at the end of mdToHtml in static/js/markdown.js puts extracted blocks back with String.prototype.replace and a string replacement:

codeBlocks.forEach((block, index) => { s = s.replace(`___CODE_BLOCK_${index}___`, block); });

A string replacement makes String.replace interpret $&, $`, $', $$, and $1..$99 as special patterns, so any restored code / allowed-HTML / math / mermaid block containing them is corrupted ($& re-inserts the placeholder, $` inserts everything before it, $' everything after, $$ collapses to $, $1 becomes an empty back-reference). These sequences are extremely common in fenced code (shell "$&", sed 's/$/…', echo $1, $$USD`), so code samples render mangled in chat, notes, and documents.

The inline-code restore site right below (line ~793) was already fixed to use a function replacer () => block with a comment explaining exactly this hazard. This PR applies the same () => block form to the four sibling sites (allowed-HTML, math, mermaid, code) that were missed. A function replacer inserts the content verbatim.

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #5663

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.

How to Test

python -m pytest tests/test_markdown_placeholder_restore_js.py -v
# or directly:
node tests/markdown_codefence_placeholder_regression.mjs

Extends the existing tests/markdown_codefence_placeholder_regression.mjs harness with a fenced code block for each $ sequence ($&, $`, $', $$, $1), asserting the surrounding marker/trailing text survives and no ___CODE_BLOCK_ placeholder leaks. Adds tests/test_markdown_placeholder_restore_js.py so CI's pytest actually runs the node harness (it wasn't wrapped before). Verified the new assertions fail on the pre-fix markdown.js and pass after.

The allowed-HTML, math, mermaid, and code-block restore sites in mdToHtml
put content back with String.replace and a *string* replacement, so `$&`,
`$\``, `$'`, `$$`, and `$1` inside the restored block were interpreted as
special replacement patterns and corrupted the output. These sequences are
common in fenced code (shell, sed, regex), so code samples rendered wrong in
chat, notes, and documents.

Use the function-replacer form `() => block` at all four sites, which inserts
the content verbatim — the same fix already applied to the inline-code site
right below them.

Fixes #

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(markdown): code/HTML/math blocks containing $&, $`, $', $$, or $1 render corrupted

1 participant