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
fix(regex): limit regex manager iterations to 10k to avoid OoM #22084
Conversation
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.
We should document and log this behaviour.
Do we document to anything like that detail today? I'm not sure where to add it where it makes sense |
I would put a warning in the regex manager docs. We document limitations as I'm aware. @HonkingGoose Thoughts? |
It's not only a regex manager limitation, it's an re2 bug. see my comment on the issue. I would open a bug on re2. |
I agree we should open an issue on RE2, but with this PR we are introducing a limitation and this should be documented or at least logged as it is happening. |
I'll go with a warn |
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.
I would put a warning in the regex manager docs.
We should add a warning to the regexManager
config option docs.
And maybe explain it in our "Known Limitations" page as well?
🎉 This PR is included in version 35.79.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Changes
Limits matchStrings matching to 10k to reduce change of hanging/crashing. The test completes in 16ms for me locally.
Context
Fixes #22083
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: