fix: handle backticks in link text for emphasis processing - #3779
fix: handle backticks in link text for emphasis processing#3779sajdakabir wants to merge 10 commits into
Conversation
|
@sajdakabir is attempting to deploy a commit to the MarkedJS Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello @sajdakabir, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a specific parsing bug in the Markdown renderer that caused emphasis (like bold or italic) to fail to close correctly when encountering bracketed text that looked like a link but contained inline code spans (backticks). The core of the fix involves updating a regular expression ( Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where backticks in link text were not handled correctly during emphasis processing. The change updates the blockSkip regular expression to correctly identify and mask code spans within link text. The fix is effective for the described problem. I've added a comment with suggestions to improve the consistency and robustness of the regular expression.
|
|
||
| // sequences em should skip over [title](link), `code`, <html> | ||
| const blockSkip = /\[[^\[\]]*?\]\((?:\\[\s\S]|[^\\\(\)]|\((?:\\[\s\S]|[^\\\(\)])*\))*\)|`[^`]*?`|<(?! )[^<>]*?>/g; | ||
| const blockSkip = /\[(?:[^\[\]\\`]|\\.|`[^`]*`)*?\]\((?:\\.|[^\\\(\)]|\((?:\\.|[^\\\(\)])*\))*\)|`[^`]*?`|<(?! )[^<>]*?>/g; |
There was a problem hiding this comment.
This is a good fix for handling backticks in link text. However, I have a couple of suggestions regarding the new regex pattern.
-
Greedy vs. Non-greedy: The new pattern for code spans within link text is
`[^`]*`(greedy), while the existing pattern for standalone code spans inblockSkipis`[^`]*?`(non-greedy). For consistency and to avoid potential subtle issues, it would be better to use the non-greedy version (*?) here as well. -
Multi-backtick code spans: A more significant point is that this regex pattern (both for standalone code spans and within links) only correctly handles single-backtick code spans. It doesn't support multi-backtick code spans (e.g.,
code). This is a pre-existing limitation, but it's being replicated in the new logic. An unmasked code span containing characters like*could interfere with emphasis parsing, leading to incorrect rendering.
For example, with **[``*text*``](url)**, blockSkip would fail to mask the link and code span correctly, exposing *text* to the emphasis parser.
While a full fix for multi-backtick spans might be out of scope for this PR, I'd recommend at least making the quantifier non-greedy for consistency. The suggestion below applies this change.
| const blockSkip = /\[(?:[^\[\]\\`]|\\.|`[^`]*`)*?\]\((?:\\.|[^\\\(\)]|\((?:\\.|[^\\\(\)])*\))*\)|`[^`]*?`|<(?! )[^<>]*?>/g; | |
| const blockSkip = /\B(?:[^\[\]\\`]|\\.|`[^`]*?`)*?\]\((?:\\.|[^\\\(\)]|\((?:\\.|[^\\\(\)])*\))*\)|`[^`]*?`|<(?! )[^<>]*?>/g; |
UziTech
left a comment
There was a problem hiding this comment.
/\[(?:[^\[\]\\`]|\\.|`[^`]*?`)*?\]\((?:\\.|[^\\\(\)]|\((?:\\.|[^\\\(\)])*\))*\)|`[^`]*?`|<(?! )[^<>]*?>/g
Is vulnerable to ReDOS. You can check it by pasting it into https://makenowjust-labs.github.io/recheck/playground/
@UziTech lmk if any changes are needed.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
UziTech
left a comment
There was a problem hiding this comment.
Can you remove the indentation changes?
…ix-blockskip-backtick-issue-3777
| @@ -0,0 +1,2 @@ | |||
| <p><strong>Really weird edge case: bold around what looks like it might be a link, but is actually a link-looking thing with a code specifier in. [Like <code>this](https://github.com)</code>. It's now impossible to close the bold</strong>.</p> | |||
| <p><strong>Bold text with [unmatched `backtick](url) should close</strong>.</p> No newline at end of file | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
@styfle thanks! I’ve updated the test — this should be resolved now.
There was a problem hiding this comment.
I think @styfle was talking about the second paragraph. That is a different bug. Can we just remove the second paragraph from the test?
The first paragraph isn't actually testing this fix currently
There was a problem hiding this comment.
@UziTech its done , let me know if any other changes are needed.
…abir/marked into fix-blockskip-backtick-issue-3777
|
Looks like this was actually fixed in the latest version by #3785 |

[PR] fix: handle backticks in link-like bracket text during emphasis parsing
Marked version: 16.3.0
Markdown flavor: CommonMark | GitHub Flavored Markdown
Description
blockSkipregex to correctly mask backticks inside link-like bracket text before emphasis processing.Expectation
Bold/italic should close correctly even when the text includes bracketed segments that look like links and contain code spans:
Expected output:
Result (before fix)
Emphasis remained unclosed because backticks within the bracketed segment were not masked, interfering with strong/em parsing.
Technical summary
blockSkipregex (used to mask links/code/HTML before emphasis) didn’t handle backticks inside the bracket text[...].\\..New
blockSkip:Contributor
angle_brackets(ensures<and>don’t break emphasis)link_tick_redos(guards against ReDOS)Committer
In most cases, this should be a different person than the contributor.