This repository was archived by the owner on Jan 23, 2023. It is now read-only.
[release/3.1] Port dotnet/runtime#31946 fix for string.Replace infinite loop #28014
Conversation
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
When string.Replace is given a target string with zero collation weight, it would enter an infinite loop. It is now changed so that the call to Replace terminates when such a condition is encountered.
GrabYourPitchforks
added
area-System.Runtime
Servicing-consider
tarekgh
approved these changes
Feb 12, 2020
|
@GrabYourPitchforks is this for 3.1.3? If so please merge by EOD today (with green CI). CC @danmosemsft |
|
Also CC @jeffschwMSFT |
leecow
added
Servicing-approved
|
Yes, it should go into 3.1.3 today. |
|
@GrabYourPitchforks I didn't find a matching corefx PR porting the tests. Please can you throw up a PR today for those - you can mark it with the same labels/milestone as this one and @Anipik can merge it. |
GrabYourPitchforks
added a commit
to GrabYourPitchforks/corefx
that referenced
this pull request
Feb 18, 2020
|
Unit tests at dotnet/corefx#42862 |
|
Looks like there are some R2R test failures. I don't believe they're related to this PR since I'm not touching any JIT code paths. |
|
Merging this change as failures are unrelated to this. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 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.
Issue dotnet/runtime#1060 reports that in a linguistic (non-ordinal) call to
string.Replace, if the target string contains code points with zero collation weight, thestring.Replacelogic can enter an infinite loop.This has already been fixed in .NET 5 (see dotnet/runtime#31946). This PR ports that fix down to release/3.1.
Customer Impact
Infinite loop if calling
string.Replace("<something-with-zero-collation-weight>", "...", StringComparison.<any-non-ordinal-comparison>).Regression?
Not a regression. This overload of
string.Replaceis the only one affected, and it wasn't introduced until .NET Core 2.0. It appears to have very little usage outside of domain-specific scenarios, like text editors.Testing
The .NET 5 fix at dotnet/runtime#31946 includes tests for the fix.
Risk
Low. A code path which previously went into an infinite loop now terminates successfully.
Code Reviewer
@tarekgh