Skip to content

markdown: link/image/autolink conformance improvements - #18

Merged
jonathanKingston merged 2 commits into
mainfrom
claude/issue-10-link-image-edge-cases
Jul 5, 2026
Merged

jonathanKingston merged 2 commits into
mainfrom
claude/issue-10-link-image-edge-cases

Conversation

@jonathanKingston

Copy link
Copy Markdown
Collaborator

Focused, low-risk CommonMark conformance improvements for inline links, link reference definitions, images, and autolinks. Refs #10 (partial batch — does not close the issue).

Per-section conformance (before → after)

Section Before After
Links 72/90 77/90
Images 17/22 18/22
Autolinks 14/19 16/19
Link reference definitions 18/27 18/27 (unchanged)

Total pinned passing examples: 510 → 518, additive only (no section decreased; the passing array only gains numbers).

Newly passing spec examples

  • Autolinks 597, 603
  • Links 547, 548, 551, 552, 568
  • Images 590

What changed

Autolinks (src/inline-autolinks.ts) — rewrote angle-autolink parsing to the CommonMark URI/email grammar:

  • Try the URI (scheme) production before the email production, so <MAILTO:foo@bar.baz> is a scheme autolink with the href kept verbatim rather than being mangled into mailto:MAILTO:... (597).
  • Use the spec's email-address grammar; the local part no longer allows a backslash, so <foo\+@bar.example.com> is not an email autolink (606 — see gaps below).
  • Backslashes are no longer treated as escapes inside an autolink, so <https://example.com/\[\> is a valid autolink (603).

Destination encoding (src/link-references.ts) — percent-encode [ and ] in href output (%5B/%5D), matching the reference renderer (603).

Inline links (src/inline-links.ts) — when a [label](...) inline destination fails to parse, fall through to shortcut-reference resolution instead of bailing out, so [foo](not a link) links [foo] and keeps (not a link) as literal text (568).

Reference label validity (src/link-references.ts) — reject empty, whitespace-only, and bracket-nesting reference labels for both definitions and lookups, so [], [[[foo]]], and [ref[bar]] no longer form links or definitions (547, 548, 551, 552, 590). Escaped brackets ([Foo*bar\]]) remain valid.

Tests

  • Added unit tests for each behavior in inline-autolinks.test.ts, inline-links.test.ts, and link-references.test.ts (not relying solely on the conformance baseline).
  • Regenerated tests/fixtures/commonmark/conformance-baseline.json (additive only).
  • npm run typecheck and npm test (385 tests) fully green.

Remaining gaps (not addressed here)

  • Autolinks 606, 608, 611: 606 still renders the backslash literally (<foo\+@...>&lt;foo\+@...) because encodeBackslashEscapes treats the malformed email as a verbatim region; 608/611 involve the app's bare-URL autolinking, which is a deliberate extension over CommonMark.
  • Reference labels with emphasis (Links 554/558/559, Images 573/576/585/589): these need reference resolution on the raw label before emphasis runs; deferred to avoid a risky pipeline reorder.
  • Link reference definitions (multiline/interrupted defs, defs inside code fences, defs after paragraph text, case-folding SS): these are block-tokenizer concerns and left for a separate change.
  • Raw-HTML precedence cases (Links 491/493/522/524/526/535/536/538) remain divergent by design (the renderer escapes raw HTML).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Cv1iyJb9PHMFG3YGp6ffkT


Generated by Claude Code

Improve CommonMark conformance for inline links, link reference
definitions, images, and autolinks (Refs #10). Focused, low-risk fixes:

- Autolinks: rewrite angle-autolink parsing to the CommonMark URI/email
  grammar. Try the URI (scheme) production before email so
  `<MAILTO:foo@bar.baz>` stays a scheme autolink; use the spec email
  grammar (no backslash in the local part); stop treating backslashes as
  escapes inside autolinks. Percent-encode `[`/`]` in href output.
- Links: when a `[label](...)` inline destination fails to parse, fall
  through to shortcut-reference resolution instead of bailing, so
  `[foo](not a link)` links `[foo]` and keeps the parens as text.
- Links/Images/Reference definitions: reject empty, whitespace-only, and
  bracket-nesting reference labels for both definitions and lookups, so
  `[]`, `[[[foo]]]`, and `[ref[bar]]` no longer form links.

Newly passing spec examples (no regressions):
  Links 72->77 (547, 548, 551, 552, 568)
  Images 17->18 (590)
  Autolinks 14->16 (597, 603)

Added unit tests for each behavior and regenerated the conformance
baseline (510->518 passing, additive only).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cv1iyJb9PHMFG3YGp6ffkT

Copy link
Copy Markdown
Collaborator Author

Code review (high effort) — no correctness bugs found.

Verified:

  • Autolink safety parity: the new autolinkHref keeps the same ^(?:javascript|data|vbscript): blocklist as safeLinkHref, and skips only the escape-decoding/trim (which is correct — autolinks don't process backslash escapes and can't contain whitespace). <javascript:alert(1)>, <data:…>, <vbscript:…> still return null. No security regression.
  • URI-before-email ordering gives <MAILTO:foo@bar> a verbatim scheme href (spec 597), and the anchored [^\s<>]* can't overrun the closing >.
  • isValidReferenceLabel is escape-aware and applied symmetrically to both definition storage and lookup, so []/[[foo]]/[ref[bar]] are consistently rejected; escaped brackets ([foo\[bar]) stay valid.
  • Link fall-through (#568) correctly resolves [foo](not a link) as a shortcut ref with end at the label close, leaving the parens as literal text — the spec-correct behavior, and it doesn't mis-consume for images either.

Low-severity note (non-blocking): dropping [/] from encodeHrefForOutput's allowed set matches cmark (verified), but it now percent-encodes brackets in every href — including an IPv6-literal host like http://[::1]/http://%5B::1%5D/, which browsers reject as a host. It's spec-conformant and IPv6 URLs in chat are vanishingly rare, so this is just a conscious-tradeoff heads-up, not a change request.


Generated by Claude Code

@jonathanKingston
jonathanKingston merged commit f80173e into main Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants