Skip to content

fix(core): stop the chat link regex backtracking on long words - #1499

Open
ANDROIDHASSAN wants to merge 2 commits into
livekit:mainfrom
ANDROIDHASSAN:fix/url-regex-backtracking
Open

ANDROIDHASSAN wants to merge 2 commits into
livekit:mainfrom
ANDROIDHASSAN:fix/url-regex-backtracking

Conversation

@ANDROIDHASSAN

Copy link
Copy Markdown
Contributor

Summary

ChatEntry runs every received chat message through createDefaultGrammar(), which uses createUrlRegExp.
The host and domain parts are written as (?:x[-_]*)*x+, so a run of letters can be split between the two quantifiers in many ways.
On a long word with no dot in it the regex tries all of them before giving up, and the page freezes while it does.

Measured on main with Node 26, text.match(createUrlRegExp({})):

input main this PR
'a'.repeat(1000) 718 ms 7 ms
'a'.repeat(4000) 42.6 s 105 ms
'x:'.repeat(1000) 2.7 s 8 ms

Any participant can send such a message, and every other participant's tab hangs while rendering it.

The fix writes each label as x+(?:[-_]+x+)*, which matches the same strings but can split them only one way.
(?:\S+(?::\S*)?@)? becomes (?:\S+@)?: : is already in \S, so the optional group only added more ways to split.

Testing

  • New test: a 3000-letter word and 'x:'.repeat(1500) match nothing and finish in under a second. On main it fails (expected 18383.37 to be less than 1000); with this change npx vitest run src/helper passes, 29 tests.
  • Fuzzed old and new regexes against each other on 200,000 random strings built from letters, digits, - _ . : / @ ? # ( ) ,, é, www. and http://, with exact both off and on: 0 differences.

Changeset

Patch for @livekit/components-core.

@vercel

vercel Bot commented Nov 3, 2026

Copy link
Copy Markdown

@ANDROIDHASSAN is attempting to deploy a commit to the LiveKit Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 90c389b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@livekit/components-core Patch
@livekit/components-react Patch
@livekit/agents-ui Patch
@livekit/component-example-next Patch
@livekit/components-js-docs Patch
@livekit/component-docs-storybook Patch
@livekit/components-docs-gen Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

for (const text of ['a'.repeat(3000), 'x:'.repeat(1500)]) {
const start = performance.now();
expect(text.match(createUrlRegExp({}))).toBeNull();
expect(performance.now() - start).toBeLessThan(1000);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: How reliable will this test be across different environments? Is there a different way to test this?

I know 1000 is meant to be very high, but I bring it up because we've run into situations in the past where slow github actions ci workers can cause tests to take much longer than they would normally in the past, and in general I worry about a limit like this resulting in a flaky test when the github actions worker is busy and the job is unable to get a lot of cpu time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. I measured both on a heavily loaded machine (load average ~14): 'a'.repeat(3000) takes 17.6 s with the old regex and 50 ms with the new one; 'x:'.repeat(1500) takes 8.6 s and 17 ms. I raised the bound to 5000 ms in 90c389b, so a starved worker has ~70x headroom and main still fails clearly. JS gives no step counter for a RegExp, so a time bound is the only check I found. Happy to drop the test if you'd prefer.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

2 participants