-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[improve] Use Google re2/j library for user provided regexes #22829
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.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #22829 +/- ##
============================================
- Coverage 73.57% 73.26% -0.32%
- Complexity 32624 32955 +331
============================================
Files 1877 1888 +11
Lines 139502 141764 +2262
Branches 15299 15559 +260
============================================
+ Hits 102638 103858 +1220
- Misses 28908 29880 +972
- Partials 7956 8026 +70
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@merlimat which branches do we target with this change? do you think that it's safe to cherry-pick this to maintenance branches? |
I think it should be fairly safe, given all "normal" regexes should behave in same way as before. At the same time, I don't think there's urgency to patch in stable branches, so I'd say to skip it unless it's needed later. |
Motivation
Google RE2/J library provide regex library that has worst case linear time processing of pattern, as opposed to standard JDK regexes which could have recursive processing. It's better to avoid these corner cases by just swapping the
Pattern
class implementation. It's a drop-in replacement for the basic methods we're using, and RE2/J is already a transitive dependency.Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: