Performance issue with long lines

[llimllib]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)

Affected packages and versions

at least the latest version

Link to runnable example

https://gist.github.com/llimllib/4b87fd4042359b4812350038562dba03

Steps to reproduce

1. Run the test.mjs file from the gist above in the root of this repository. Its output on my machine is:

$ node test.mjs
parsing a file with breaks: 32.809ms
parsing a file without breaks: 10.127s

In this test case, the "file with breaks" is 8000 lines of 100 a characters. The "file without breaks" is 800,000 'a' characters without line breaks, so actually a slightly smaller file but with no line breaks.

here's an observable notebook that demonstrates the behavior, you can play around with the regular expression there and see how performance looks

Expected behavior

parsing a file with a long line should not have such severe performance effects.

(In the terms that affect my team, our customers should not be able to easily DoS us by providing markdown files with long lines)

Actual behavior

The findEmail regular expression here takes a very long time to parse a long line.

(I think this is due to using nested quantifiers, but I am not an expert here)

I noticed this when investigating a slowdown in our app, developed a test case, and profiled it. This regular expression jumped out:

Affected runtime and version

node 20.16.0

Affected package manager and version

all

Affected OS and version

all

Build and bundle tools

No response

[llimllib]

for what it's worth, I tried finding a regular expression that worked better and was not successful; perhaps it is sensible to do something like check the line for an @ before running the regular expression?

That would still leave a long line full of @ as a DDoS vector, but it'd be an improvement at least?

[ChristianMurphy]

Welcome @llimllib! 👋
Performance improvements are welcome!

as a DDoS vector

Respectfully a major of "DDoS vectors", including this, are a nothingburger.
Longer content takes longer, if you let users put content of unrestricted length in, it could be slow.

[llimllib]

@ChristianMurphy we render mdx on our servers for our clients; if somebody were to start spamming us with long strings, it would cost a lot of money and slow down our system for everybody

[llimllib]

Limiting the length of + matches to 1024 speeds up the test file, but it also makes tests fail and I don't quote have a handle on why:

// Use {0,1024} instead of `+` to avoid pathological regular expression
// behavior; see
// https://github.com/syntax-tree/mdast-util-gfm-autolink-literal/issues/8
[
  /([-.\w+]{0,1024})@([-\w]{0,1024}(?:\.[-\w]{0,1024}){0,1024})/g,
  findEmail
]

That expression matches emails the same as the current one:

> "bill@billmill.org".match(/([-.\w+]{0,1024})@([-\w]{0,1024}(?:\.[-\w]{0,1024}){0,1024})/g)
Array [ "bill@billmill.org" ]

ah ha! it's matching things it ought not to, and of course the proper replacement for + is {1,1024} not {0,...}, that's what + means

[ChristianMurphy]

we render mdx on our servers for our clients; if somebody were to start spamming us with long strings, it would cost a lot of money and slow down our system for everybody

Yes, that is how programming languages work.
Please read the MDX docs https://mdxjs.com/docs/getting-started/#security this has already been addressed.

[llimllib]

@ChristianMurphy I truly have no idea why you're being mean here

[ChristianMurphy]

MDX is a full programming language, it compiles to JavaScript.
If you are, as you imply, letting your untrusted user run content without any restrictions.
You are letting untrusted users run arbitrary JS on your server, and all the security and performance risks that go along with it. That is your choice, but that isn't a "vulnerability" or a "security issue" of MDX.

---

To use MDX safely you either:

1. trust your users
2. or, sandbox and timeout content

In both circumstances this perf issue you bring up, is a non-issue from a security perspective.

---

Again, happy to discuss improving parser speed.
I'm mostly bringing this up because you seem to fundamentally misunderstand what is and is not a security concern.

[llimllib]

yes I know that! I do not see why you are spamming this issue with unrelated stuff

[llimllib]

@ChristianMurphy I presented a fix in #9

• I understand exactly what this is
• For us it is a security issue, I apologize if that bothers you, I guess? I'm not going to be filing a CVE or anything, just trying to make it so that long lines process in a reasonable period of time so we're not vulnerable to CPU usage attacks