fix: escape square brackets in link text and image alt (#1302) - #2269
Open
UditDewan wants to merge 1 commit into
Open
fix: escape square brackets in link text and image alt (#1302)#2269UditDewan wants to merge 1 commit into
UditDewan wants to merge 1 commit into
Conversation
Link text and image alt text were emitted unescaped inside `[...](...)`
and ``. When the text contains an unbalanced `[` or `]`, the
bracket terminates the label early and the link breaks:
<a href="https://example.com">Unbalanced ] close</a>
previously converted to
[Unbalanced ] close](https://example.com)
which CommonMark does not parse as a link at all -- parsers emit the
whole thing as literal text, so both the link and its target are lost.
An unbalanced opening bracket instead truncates the label.
Escape `[` and `]` in the two `_CustomMarkdownify` overrides that build
Markdown link syntax. Escaping is applied unconditionally: balanced
brackets already round-trip correctly, and escaped brackets render
identically to them, so this avoids having to detect balance. A negative
lookbehind keeps the substitution idempotent for callers that enable
markdownify's `escape_misc` option, which pre-escapes brackets.
Keeping the escape local to these two methods avoids flipping
`escape_misc` globally, which would also escape `&<>~=+|` and change
output well beyond links.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1302.
Problem
Link text and image alt text are emitted unescaped inside
[...](...)/. When the text contains an unbalanced[or], the bracket terminates the label early and the link is destroyed.Repro on
main:That output is not a link. Per CommonMark, brackets are permitted in a link label only when backslash-escaped or appearing as a matched pair, so parsers emit the whole string as literal text — both the link and its target are lost:
An unbalanced opening bracket fails differently, truncating the label:
Image alt text breaks the same way (
![Bad ] alt](pic.png)renders as literal text).Since this lives in
_CustomMarkdownify, it affects every HTML-backed converter — HTML, DOCX, EPUB, RSS, Wikipedia, and Bing SERP.One clarification on the issue as filed: it reports
[Learn [GPT]](url)as broken, but balanced brackets are actually legal CommonMark and already round-trip correctly today. The genuine defect is the unbalanced case.Fix
Escape
[and]in the two_CustomMarkdownifyoverrides that build Markdown link syntax (convert_aandconvert_img).Escaping is applied unconditionally rather than only to unbalanced input: escaped brackets render identically to balanced ones, so this avoids having to detect balance for no behavioural gain. A negative lookbehind keeps the substitution idempotent for callers who enable markdownify's
escape_misc, which pre-escapes brackets.Keeping the escape local to these two methods avoids flipping
escape_miscon globally, which would also escape&<>~=+|and change output well beyond links.Brackets in ordinary text are left untouched — they aren't link syntax there.
Verification
Escaped output was checked against the CommonMark reference implementation (
commonmark) andmistletoe; all cases now render as intended links/images with the brackets preserved in the visible text:[Learn \[GPT\]](u)<a href="u">Learn [GPT]</a>[Unbalanced \] close](u)<a href="u">Unbalanced ] close</a>[Unbalanced \[ open](u)<a href="u">Unbalanced [ open</a>![Bad \] alt](p.png)<img src="p.png" alt="Bad ] alt">Tests
Adds
test_link_text_bracket_escaping, covering balanced, unbalanced-open, and unbalanced-close brackets in both link text and image alt, plus assertions that plain-text brackets are untouched and that escaping is idempotent.Full suite run locally: no regressions. The pre-existing failures on my Windows machine (cp1252 stdout encoding, missing
ffmpeg, missing Azure credentials) are identical with and without this change.black(pre-commit, 23.7.0) is clean.