fix: Avoid RegExp lookbehind assertions (#3816) #3817

ziadkh0 · 2025-10-28T10:49:55Z

RegExp lookbehind assertions are supported in Safari >= 16.5, avoiding them keeps support for older Safari versions.

Marked version: 16.4.1

Markdown flavor: n/a

Description

Fixes Marked is incompatible with Safari < 16.5 #3816

Contributor

Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
no tests required for this PR.
If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

CI is green (no forced merge required).
Squash and Merge PR following conventional commit guidelines.

RegExp lookbehind assertions are supported in Safari >= 16.5, avoiding them keeps support for older Safari versions.

vercel · 2025-10-28T10:50:02Z

@ziadkh0 is attempting to deploy a commit to the MarkedJS Team on Vercel.

A member of the Team first needs to authorize it.

gemini-code-assist · 2025-10-28T10:50:09Z

Summary of Changes

Hello @ziadkh0, I'm Gemini Code Assist¹! 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 addresses a browser compatibility issue by refactoring regular expressions to remove lookbehind assertions, which are not universally supported across all browser versions. The primary goal is to ensure that the markdown parsing library remains functional and compatible with older Safari versions (pre-16.5) while accurately handling inline code blocks during the lexing process.

Highlights

RegExp Lookbehind Assertion Removal: The pull request refactors regular expressions to eliminate the use of lookbehind assertions, specifically within the blockSkip rule for inline code. This change is crucial for maintaining compatibility with older browser versions, particularly Safari versions prior to 16.5, which do not support these assertions.
Lexer Masking Logic Adjustment: The Lexer class's masking logic has been updated to correctly process the modified blockSkip regular expression. It now conditionally utilizes the second capturing group (match[2]) for masking inline code blocks, ensuring accurate parsing after the regex structure change.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request aims to remove RegExp lookbehind assertions to improve compatibility with older Safari versions. The changes in src/rules.ts and src/Lexer.ts are a good step in this direction. However, the fix is incomplete as a lookbehind assertion remains in the link regex. Additionally, there appears to be a typo in the new code regex. My review includes comments to address these issues.

src/rules.ts

ziadkh0 · 2025-10-28T11:27:03Z

I noticed npm run test:redos uses recheck which doesn't appear to support the d flag.
Should I update test/recheck.js to ignore this flag?

vercel · 2025-10-29T13:27:24Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
marked-website	Ready	Preview	Comment	Oct 30, 2025 4:05pm

UziTech · 2025-10-29T13:50:39Z

ya updating recheck.js would be helpful. I don't think the d flag would change whether or not the regexp is vulnerable to redos.

UziTech · 2025-10-29T14:03:26Z

the full regexp looks safe

/\[(?:[^\[\]`]|(?<a>`+)[^`]+\k<a>(?!`))*?\]\((?:\\[\s\S]|[^\\\(\)]|\((?:\\[\s\S]|[^\\\(\)])*\))*\)|(?:^|[^`])((?<b>`+)[^`]+\k<b>(?!`))|<(?! )[^<>]*?>/dg

UziTech · 2025-10-29T14:11:31Z

It looks like this does slow marked down significantly.

npm run bench for this PR:

marked completed in 5407ms and passed 97.70%
commonmark completed in 3798ms and passed 100.00%
markdown-it completed in 4457ms and passed 88.96%
42% slower than commonmark

npm run bench for master:

marked completed in 5102ms and passed 97.70%
commonmark completed in 3787ms and passed 100.00%
markdown-it completed in 4452ms and passed 88.96%
35% slower than commonmark

UziTech · 2025-10-29T14:28:21Z

@styfle @calculuschild is it worth it to slow marked down to support ~0.25% of users?

I'm on the fence

calculuschild · 2025-10-29T15:49:56Z

I would say not worth it. The core marked should prioritize speed for the 99.75% of users.

If somebody really needs to support old browsers, I recommend an older version or making a fork.

ziadkh0 · 2025-10-29T17:27:15Z

I can feature detect support and make the lookbehind-less version a fallback, that way it's win-win. What do you think?

ziadkh0 · 2025-10-29T17:44:52Z

Only marginal difference on a M1 MacBook Air. 🤷‍♂️

npm run bench for this PR:

marked completed in 1911ms and passed 97.70%
commonmark completed in 1410ms and passed 100.00%
markdown-it completed in 1670ms and passed 88.96%
36% slower than commonmark

npm run bench for master:

marked completed in 1914ms and passed 97.70%
commonmark completed in 1407ms and passed 100.00%
markdown-it completed in 1678ms and passed 88.96%
36% slower than commonmark

ziadkh0 · 2025-10-29T18:16:18Z

I'm getting big run to run variance when running npm run bench 😢
@UziTech are your bench results consistent from run to run?

UziTech · 2025-10-29T18:26:14Z

The times are not consistent due to other things taking up the processor during part of it but the time for marked relative to the other in one run should be fairly consistent. Mine is usually within 2% so a 7% change seems significant

ziadkh0 · 2025-10-29T18:29:02Z

I have seen a difference of 10% between slowest run and fastest run (e.g. 36% slower than commonmark and 46% slowerthan commonmark)!

UziTech · 2025-10-29T18:32:17Z

I can feature detect support and make the lookbehind-less version a fallback, that way it's win-win. What do you think?

That seems like unnecessary maintenance burden to make sure the two version of the regexp + code are in sync.

ziadkh0 · 2025-10-29T19:19:54Z

For what it's worth, in the usage stats you forgot to include iOS Safari ~1% of users

calculuschild · 2025-10-29T19:22:34Z

Any chance this could just be made as an extension?

ziadkh0 · 2025-10-29T20:44:34Z

I ran npm run bench for master on an idling machine 11 times and I still got a big variance from run to run

41% slower than commonmark
47% slower than commonmark
51% slower than commonmark
41% slower than commonmark
47% slower than commonmark
38% slower than commonmark
36% slower than commonmark
31% slower than commonmark
43% slower than commonmark
40% slower than commonmark
42% slower than commonmark

Am I the only one unable to get a consistent run to run results?

ziadkh0 · 2025-10-30T12:29:16Z

I made the lookbehind-less version a fallback for engines that don't support them, and changed the implementation a bit to improve performance.

@UziTech, can you please rerun the benchmark on the most recent commit? 🤞

As for the maintenance burden, I'm willing to maintain support for Safari 15.6. 😄

UziTech

With this new version it looks like the benchmarks are the same. Thanks for sticking with this! 💯

src/rules.ts

RegExp lookbehind assertions are supported in Safari >= 16.5. But the lookbehind-less version has a performance penalty. So having a fallback keeps support for older Safari versions while maintaining performance for modern engines.

ziadkh0 · 2025-10-31T10:30:30Z

@UziTech Thank you for testing and approving this PR! 😄

Would you prefer I squash the two commits into a single one? (I'm not quite sure what the commit guidelines are 😅)
The new version no longer uses the d RegExp flag, should I update recheck.js in another PR or there's no need?

UziTech · 2025-10-31T13:26:14Z

We use squash and merge whenever we merge a PR. You don't need to do anything.
No need, we can do it later if needed.

styfle · 2025-11-04T19:58:55Z

src/rules.ts

-  .replace('code', /(?<!`)(?<b>`+)[^`]+\k<b>(?!`)/)
+const blockSkip = edit(/link|precode-code|html/, 'g')
+  .replace('link', /\[(?:[^\[\]`]|(?<a>`+)[^`]+\k<a>(?!`))*?\]\((?:\\[\s\S]|[^\\\(\)]|\((?:\\[\s\S]|[^\\\(\)])*\))*\)/)
+  .replace('precode-', supportsLookbehind ? '(?<!`)()' : '(^^|[^`])')


My only thought now is about testing. Are we testing both possibilities (with lookbehind and without)?

No, only with lookbehind as Node supports it since version 8.10.
I don't know which approach is best suited to test without lookbehind support. 🤔
I think ideally, tests should be run in an environment that doesn't support lookbehind (like Safari < 16.4) but I'm not sure its possible...

fix: Avoid RegExp lookbehind assertions (markedjs#3816)

aebb5a0

RegExp lookbehind assertions are supported in Safari >= 16.5, avoiding them keeps support for older Safari versions.

gemini-code-assist bot reviewed Oct 28, 2025

View reviewed changes

src/rules.ts Show resolved Hide resolved

src/rules.ts Outdated Show resolved Hide resolved

ziadkh0 mentioned this pull request Oct 28, 2025

Marked is incompatible with Safari < 16.5 #3816

Open

vercel bot deployed to Preview October 29, 2025 13:27 View deployment

UziTech approved these changes Oct 30, 2025

View reviewed changes

UziTech requested review from calculuschild and styfle October 30, 2025 15:18

vercel bot deployed to Preview October 30, 2025 15:18 View deployment

UziTech reviewed Oct 30, 2025

View reviewed changes

src/rules.ts Outdated Show resolved Hide resolved

Improve perf of blockSkip on modern engines

9397604

RegExp lookbehind assertions are supported in Safari >= 16.5. But the lookbehind-less version has a performance penalty. So having a fallback keeps support for older Safari versions while maintaining performance for modern engines.

ziadkh0 force-pushed the issue-3816 branch from 70e5d94 to 9397604 Compare October 30, 2025 15:51

vercel bot deployed to Preview October 30, 2025 16:05 View deployment

UziTech added the hacktoberfest-accepted label Oct 30, 2025

styfle reviewed Nov 4, 2025

View reviewed changes

fix: Avoid RegExp lookbehind assertions (#3816) #3817

Are you sure you want to change the base?

fix: Avoid RegExp lookbehind assertions (#3816) #3817

Conversation

ziadkh0 commented Oct 28, 2025

Description

Contributor

Committer

Uh oh!

vercel bot commented Oct 28, 2025

Uh oh!

gemini-code-assist bot commented Oct 28, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

ziadkh0 commented Oct 28, 2025

Uh oh!

vercel bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UziTech commented Oct 29, 2025

Uh oh!

UziTech commented Oct 29, 2025

Uh oh!

UziTech commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UziTech commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calculuschild commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

UziTech commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

UziTech commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

calculuschild commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 29, 2025

Uh oh!

ziadkh0 commented Oct 30, 2025

Uh oh!

UziTech left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ziadkh0 commented Oct 31, 2025

Uh oh!

UziTech commented Oct 31, 2025

Uh oh!

styfle Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

ziadkh0 Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

vercel bot commented Oct 29, 2025 •

edited

Loading

UziTech commented Oct 29, 2025 •

edited

Loading

UziTech commented Oct 29, 2025 •

edited

Loading

calculuschild commented Oct 29, 2025 •

edited

Loading