馃悰 Use the CommonMark whitespace set, not the Python one - #433
Merged
Merged
Conversation
str.strip() and str.split() without arguments use str.isspace(), which treats U+001C, U+001D, U+001E, U+001F and U+0085 as whitespace. CommonMark and String.prototype.trim do not, so upstream markdown-it keeps them. Ten call sites relied on the Python behaviour, while the module already defines the correct set a few lines above them in MD_WHITESPACE. Two consequences, both measured against markdown-it@14.1.0: - normalizeReference folded distinct labels together, so a link reference definition whose label differs from the usage resolved it anyway. The input "[a b]" with a definition "[a\x85b]: http://evil" produced a link in Python and did not in JavaScript. - The characters were dropped from paragraphs, headings, table cells and fence info strings. Add MD_TRIM_CHARS and mdTrim() to common/utils, and use them at the sites that were relying on str. In the fence renderer, replace str.split() with a split over the same set, for the same reason. MD_TRIM_CHARS is String.prototype.trim minus U+FEFF. trim() does remove U+FEFF, and doing the same here would reintroduce exactly the label folding this change removes. Deliberately unchanged: validateLink() in common/normalize_url.py also uses str.strip(), but there the wider Python set is stricter rather than looser, and aligning it would let more URLs through.
Hoist the per-call regex in normalizeReference and RendererHTML.fence to a module-level MD_TRIM_RE, and add the changelog entry for #418.
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
2 participants
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.
Supersedes #418 by @Nexory, whose commit is preserved as the first commit here (the fork does not allow maintainer edits, so it could not be brought up to date in place).
Summary
str.strip()andstr.split()without arguments usestr.isspace(), which treats U+001C, U+001D, U+001E, U+001F and U+0085 as whitespace. CommonMark andString.prototype.trimdo not, so upstream markdown-it keeps those characters. Ten call sites relied on the Python behaviour, with two visible consequences (both measured against markdown-it@14.1.0):normalizeReferencefolded distinct labels together, so[a\x85b]: urlresolved a usage[a b]in Python and not in JavaScript.This adds
MD_TRIM_CHARSandmdTrim()tocommon/utilsand uses them at those sites; the fence renderer splits the info string on the same set.U+FEFFis deliberately excluded, sincetrimremoving it is the same label-folding defect in the other direction. See #418 for the full rationale, including what was deliberately left unchanged (validateLink, the backticks edge rule).Changes on top of #418
re.sub("[" + re.escape(MD_TRIM_CHARS) + "]+", ...)innormalizeReferenceandRendererHTML.fenceis hoisted to a module-level compiledMD_TRIM_RE.Validation
tests/test_port/test_whitespace.pyplus the existing 1017).-Wand nitpicky passes (only the usual egress-blocked stdlib intersphinx refs).