-
-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes catastrophic backtracking for long strings of spaces #629
base: main
Are you sure you want to change the base?
Fixes catastrophic backtracking for long strings of spaces #629
Conversation
…trings of spaces
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Could you add a changeset please?
Hey @quantizor, thanks for the review! Unfortunately, I've discovered that the new regex doesn't conform to the CommonMark specs and fails the test case below. it('should not interrupt paragraphs', () => {
render(compiler('foo\n bar'))
expect(root.innerHTML).toMatchInlineSnapshot(`
<p>
foo
bar
</p>
`)
})
- Snapshot - 4
+ Received + 10
- <p>
+ <div>
+ <p>
- foo
+ foo
- bar
- </p>
+ </p>
+ <pre>
+ <code>
+ bar
+ </code>
+ </pre>
+ </div> I think changing the regex won't be the final solution here, and the block parsing algorithm should be improved to handle indented code blocks. It looks like we have two options:
|
I'm pretty sure there is already code to handle indentation |
Sorry for the confusion. I meant using block parsing to identify indented code blocks, instead of using regex. |
Solves #473
With a very long input of spaces and the current
CODE_BLOCK_R
regex, the number of possible combinations becomes huge, causing the regex engine to take an extremely long time to test the input. This small change to the regex reduces the backtracking and prevents catastrophic backtracking when faced with a long string of spaces.Tester for catastrophic backtracing: https://regex101.com/r/85LH2r/1
Reproduction: https://stackblitz.com/edit/markdown-to-jsx-reproduction-473?file=src%2FApp.tsx