Skip to content

@mdxld/frame: escapeMarkdown is not injective, and leaves backticks, HTML, emphasis and links unescaped #11

Open

Description

@nathanclevenger

Follow-up from the verification review of #4.

The package advertises the escape as "unambiguous and reversible". It is neither.

Not injective — verified

escapeMarkdown('#foo')    // '\#foo'
escapeMarkdown('\\#foo')  // '\#foo'     ← two different values, one byte string

escapeMarkdown('a\nb')    // 'a\nb'  (literal backslash-n)
escapeMarkdown('a\\nb')   // 'a\nb'  ← a real newline and the two characters \ n collide

escapeMarkdown('a\r\nb')  // 'a\nb'
escapeMarkdown('a\rb')    // 'a\nb'  ← CRLF, CR and LF all collapse to the same output

A reader cannot recover the original value from the markdown face. This is the same "two facts, one byte string" defect the presence-marker fix addressed (glyph.ts quotes a present value whose text would imitate — / withheld(...) / a trailing ~, precisely so an agent parsing the face can tell them apart), reintroduced one layer further out. renderGlyph hands the sink a string in which the four presence states are recoverable; escapeMarkdown then destroys recoverability of the value itself.

Under-escaped — verified

escapeMarkdown handles |, newlines, and a leading #>-+ only. It passes through:

escapeMarkdown('`code`')                      // '`code`'
escapeMarkdown('<script>alert(1)</script>')   // unchanged
escapeMarkdown('**bold** [text](http://evil)')// unchanged

So a counterparty string — and in a shared-record domain a Field's value is very often a counterparty string, which is the entire premise of the escape's own docstring — can forge emphasis, a live link, and raw HTML in the agent face. Face parity does not catch it: the emitted value is exactly what the Frame held; it is the markup around it that was authored by the supplier.

Fix

  • Make the transform injective. Escape the escape character first (\ → \\) before anything else, and use distinct sequences for CR, LF and CRLF rather than collapsing all three.
  • Extend the character class: backtick, *, _, [, ], <, >, & at minimum. The set should be derived from "what can author structure or a live reference in GFM", not enumerated by hand from memory.
  • Ship the inverse. unescapeMarkdown is what makes "reversible" a testable claim rather than a docstring, and the wrote/text relation issue wants it anyway.
  • Property test: unescape(escape(s)) === s over a generated corpus including every markdown metacharacter, all three newline forms, and strings that are already escaped.

Ref: #4

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions