Skip to content
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

[release/5.0] Fix handling of \G in Regex.Split/Replace #44985

Merged
merged 1 commit into from
Nov 21, 2020

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 20, 2020

Backport of #44975 to release/5.0

/cc @stephentoub

Customer Impact

Broken Replace/Split operations where the regex uses the \G anchor. The \G anchor in a .NET regex says that it should match the ending location of the last match. However, our optimized regex scan loop we use for handling Regex.Replace and Regex.Split wasn’t updating the relevant field to indicate that position; as a result, \G used in Regex.Replace and Regex.Split was failing to match after the first occurrence.

Testing

We didn’t have any tests that exercised \G in replace/splits (only in other operations like Regex.Match). I added several.

Risk

Low. It’s a one-line addition to update a tracking field that's only relevant to the processing of \G anchors.

In our optimized Regex.Split loop, we failed to update runtextstart, which means the \G anchor (aka starting where the previous match ended).
@ghost
Copy link

ghost commented Nov 20, 2020

Tagging subscribers to this area: @eerhardt, @pgovind, @jeffhandley
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #44975 to release/5.0

/cc @stephentoub

Customer Impact

Testing

Risk

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

area-System.Text.RegularExpressions

Milestone: -

@stephentoub stephentoub added the Servicing-consider Issue for next servicing release review label Nov 20, 2020
@stephentoub stephentoub added this to the 5.0.x milestone Nov 20, 2020
@stephentoub stephentoub added Servicing-approved Approved for servicing release auto-merge and removed Servicing-consider Issue for next servicing release review labels Nov 20, 2020
@ghost
Copy link

ghost commented Nov 20, 2020

Hello @stephentoub!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@stephentoub stephentoub merged commit 90abcef into release/5.0 Nov 21, 2020
@stephentoub stephentoub deleted the backport/pr-44975-to-release/5.0 branch November 21, 2020 01:08
@ghost ghost locked as resolved and limited conversation to collaborators Dec 21, 2020
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.

2 participants