[release/uwp6.2] Port dotnet/runtime#31946 fix for string.Replace infinite loop

[GrabYourPitchforks]

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, the string.Replace logic 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>). This has already impacted at least one first-party application, and they are requesting a backported fix for UWP.

Regression?

Not a regression. This overload of string.Replace is 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

[MichalStrehovsky]

Let me know if you need help with checking this into ProjectN TFS. I have an enlistment of the release branch.

[danmoseley]

does this affect StringBuilder.Replace(..) btw?

[tarekgh]

does this affect StringBuilder.Replace(..) btw?

It shouldn't.

[GrabYourPitchforks]

@MichalStrehovsky I'm not familiar with the process of porting changes in these branches. Do commits to this branch not automatically flow to ProjectN?

[MichalStrehovsky]

@MichalStrehovsky I'm not familiar with the process of porting changes in these branches. Do commits to this branch not automatically flow to ProjectN?

Commits to this branch only affect CoreCLR. This would fix it for UWP in Debug builds (because that one uses CoreCLR), but Release/Store builds would stay broken.

There used to be a mirror for the master branch that went CoreCLR master -> CoreRT master that we then manually moved to CoreRT nmirror branch from which another mirror picked it up and placed it into Project N master in TFS, but all these mirrors are dead. This needs a direct checkin into TFS ProjectN release branch. I can help with that when it's ready.

[jashook]

@GrabYourPitchforks what is the status of this?

[GrabYourPitchforks]

@jashook The fix is already in .NET 5.0.

[asklar]

@jashook The fix is already in .NET 5.0.

@jashook @GrabYourPitchforks we need this fix serviced to existing .net native framework packages

[GrabYourPitchforks]

Sorry, misunderstood. I have some other servicing work I need to get to first, but I should be able to pick this up again next week.

[jashook]

@asklar what versions are you referring to /cc @tommcdon

[asklar]

@jashook @tommcdon I'll add you to the email thread

[jashook]

@GrabYourPitchforks merged, kicking off a build.