Skip to content

[release/8.0-staging] Fix regex lazy loop handling of backtracking state at max iteration limit#97927

Merged
stephentoub merged 2 commits intorelease/8.0-stagingfrom
backport/pr-97890-to-release/8.0-staging
Feb 12, 2024
Merged

[release/8.0-staging] Fix regex lazy loop handling of backtracking state at max iteration limit#97927
stephentoub merged 2 commits intorelease/8.0-stagingfrom
backport/pr-97890-to-release/8.0-staging

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Feb 3, 2024

Backport of #97890 to release/8.0-staging

/cc @stephentoub

Customer Impact

  • Customer reported
  • Found internally

Lazy regex loops may end up being incorrectly processed for RegexOptions.Compiled and source generated regexes, in the situation where the expression after the loop fails to match once the loop has reached a maximum number of iterations. When that happens and the handling needs to backtrack, it may erroneously leave state on the backtracking stack, which in turn may cause earlier constructs (e.g. an alternation earlier in the pattern) to pop the wrong state from the backtracking state. The net result is that earlier construct could end up then behaving unexpectedly, such as entering an infinite loop, OOM'ing, or returning an incorrect answer.

Regression

  • Yes
  • No

Introduced in .NET 7 when the compiler was almost entirely rewritten.

Testing

The fix is to pop the backtracking state in one very specific code path that was missed. It requires a very specific form of pattern to hit this condition, and even when it's hit, an even more specific pattern for the impact to be visible. Several patterns were added to the test suite that fit these criteria.

Risk

Low. All existing tests pass, and the extra popping only happens in a case where it was obviously needed but absent.

…imit

Upon entering a lazy loop, state is pushed onto the backtracking stack if the lazy loop might be backtracked into.  That state is then dutifully popped off when unwinding the loop due to failure to match. However, if the loop successfully matches its maximum number of times but still fails because of a failure after the lazy loop, the state still needs to be popped off the stack, but isn't. This fixes that.
@ghost
Copy link
Copy Markdown

ghost commented Feb 3, 2024

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #97890 to release/8.0-staging

/cc @stephentoub

Customer Impact

  • Customer reported
  • Found internally

[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.]

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

[How was the fix verified? How was the issue missed previously? What tests were added?]

Risk

[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.]

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

@stephentoub stephentoub added the Servicing-consider Issue for next servicing release review label Feb 3, 2024
@stephentoub stephentoub added this to the 8.0.x milestone Feb 3, 2024
@stephentoub stephentoub added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 4, 2024
@stephentoub stephentoub requested a review from joperezr February 4, 2024 14:47
@carlossanlop
Copy link
Copy Markdown
Contributor

@stephentoub today is code complete for the March release. If this PR meets all the servicing criteria, please merge it before 4pm PT.

@stephentoub stephentoub merged commit 11ae42b into release/8.0-staging Feb 12, 2024
@stephentoub stephentoub deleted the backport/pr-97890-to-release/8.0-staging branch February 12, 2024 18:33
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants