Fix StackOverflow in RELEASE_PATTERN by reducing regex branch nodes#7231
Merged
Jenson3210 merged 9 commits intomainfrom Apr 1, 2026
Merged
Fix StackOverflow in RELEASE_PATTERN by reducing regex branch nodes#7231Jenson3210 merged 9 commits intomainfrom
Jenson3210 merged 9 commits intomainfrom
Conversation
Reproduces NumberFormatException in LatestRelease.compare() when version strings have 6+ numeric parts, as the loop exceeds the 5 regex groups.
LatestRelease.compare() iterates based on countVersionParts(), which can return >5 for versions with many numeric segments (e.g. 1.2.3.4.5.6). However, RELEASE_PATTERN only has 5 numeric capturing groups, so group(6) returns the qualifier suffix (e.g. ".6") causing NumberFormatException in Long.parseLong(). Cap the loop at 5 to stay within the regex groups; any remaining parts are handled by the string comparison fallback. Fixes moderneinc/customer-requests#2121
Instead of using RELEASE_PATTERN's 5 numeric capturing groups, parse numeric version parts and qualifier suffix directly from the version string. This supports versions with any number of numeric parts (e.g. 1.2.3.4.5.6.7) and correctly extracts qualifiers like -RC1 even when they follow more than 5 numeric segments.
Verify that comparing versions with different numbers of numeric parts works correctly when either side has more than 5 parts (e.g. 3 vs 6, 5 vs 6), including the case where trailing zeros make versions equal.
RELEASE_PATTERN only had 5 numeric capturing groups, which caused isValid() to reject 6+ part versions by misclassifying the 6th numeric part as a qualifier suffix. Extend to 9 groups and use a named group for the qualifier so checkVersion() is independent of the group count.
…groups The extractVersionParts/extractQualifierSuffix helpers are no longer needed since RELEASE_PATTERN has enough numeric groups. Restore the original compare() logic with just the cap removed and group(6) replaced by the named "qualifier" group.
.*?$ is non-greedy but still captures to end of string, so it works correctly with both .matches() and .find() callers.
…erflow The 9 optional capturing groups in RELEASE_PATTERN created too many Branch nodes in Java's regex engine, causing StackOverflowError. Instead, keep 5 capturing groups (for XRange compatibility) and use a non-capturing `(?:\.\d+)*` to consume any additional numeric parts. The compare() method parses version parts directly from the string rather than relying on regex groups. Fixes moderneinc/customer-requests#2142
timtebeek
approved these changes
Apr 1, 2026
Member
timtebeek
left a comment
timtebeek
left a comment
There was a problem hiding this comment.
Approved already; still see some conflicts.
github-project-automation
bot
moved this from In Progress
to Ready to Review
in OpenRewrite
Contributor
Author
Verification against customer-requests#2142I tested this PR's approach by shadowing Results
AssessmentThe fix correctly eliminates the regex At very small stack sizes (≤272KB), a second recursive component becomes the bottleneck: For the customer's actual environment (Moderne platform workers, likely ≥512KB stack), this fix should be sufficient. |
Member
|
Ok to merge then I think, right? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
(?:\.\d+)*consumercompare()instead of relying on regex groups(?<qualifier>...)group for qualifier extractionProblem
RELEASE_PATTERNfrom 5 to 9 optional capturing groups to support versions with 6+ numeric parts. Each optional(?:\.(\d+))?compiles to aBranchnode in Java's regex engine, which uses recursion to evaluate. The 9 branches causedStackOverflowErrorin environments with limited stack depth.Solution
Keep 5 explicit capturing groups (needed by
XRange) and add a single non-capturing(?:\.\d+)*to consume any additional numeric parts. This creates a simple loop node instead of multiple Branch nodes, eliminating the stack overflow risk. Thecompare()method parses version parts directly from the string rather than relying on regex group access, so it handles any number of parts.Test plan
All
LatestReleaseTesttests pass (including 6+ part versions)All semver tests pass
RemoveRedundantDependencyVersionstests passVerified no StackOverflow with 256k stack size
Fixes moderneinc/customer-requests#2142