Skip to content
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

Parsing emphasis takes quadratic time #172

Closed
rlidwka opened this issue Aug 30, 2019 · 3 comments
Closed

Parsing emphasis takes quadratic time #172

rlidwka opened this issue Aug 30, 2019 · 3 comments

Comments

@rlidwka
Copy link
Contributor

rlidwka commented Aug 30, 2019

A bug was reported in markdown-it:
markdown-it/markdown-it#583

Similar thing exists here, just for more complex pattern.

This takes 1 second (as well as multiple other similar combinations):

$ node -e 'console.log("*_* _".repeat(40000))' | time ./commonmark > /dev/null
1.12user 0.10system 0:00.72elapsed 169%CPU (0avgtext+0avgdata 150168maxresident)k
0inputs+0outputs (0major+32913minor)pagefaults 0swaps

But this one takes 45 seconds (progressively worse with more data):

$ node -e 'console.log("*_* _ ".repeat(40000))' | time ./commonmark > /dev/null
45.13user 0.17system 0:44.73elapsed 101%CPU (0avgtext+0avgdata 152156maxresident)k
0inputs+0outputs (0major+37771minor)pagefaults 0swaps

Expected result is <em>_</em> _ <em>_</em> _ <em>_</em> _ <em>_</em> _ <em>_</em>, and I suspect the issue is with _ character in between emphases trying to be paired with every single previous one.

I don't know at the moment how or whether it can be fixed.

@jgm
Copy link
Member

jgm commented Aug 30, 2019

Note: cmark does NOT have this problem (nor does jgm/commonmark-hs).

@jgm
Copy link
Member

jgm commented Aug 30, 2019

I think this is the same as commonmark/cmark#178
which was fixed by
commonmark/cmark@a8d5bd4

It should be possible to port this commit to commonmark.js, which otherwise follows cmark's algorithm exactly.

@jgm
Copy link
Member

jgm commented Aug 30, 2019

I backported the fix to cmark#178, but that didn't fix this case.
Hm.

@jgm jgm closed this as completed in a45a752 Aug 31, 2019
taku0 added a commit to taku0/cmark-el that referenced this issue Dec 28, 2019
rpokorny added a commit to rpokorny/react-commonmark that referenced this issue Jan 4, 2021
The old version, 0.27.0, has a vulnerability as described here:
commonmark/commonmark.js#172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants