Fix infinite loop in string.Replace (#31946)

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.

diff --git a/src/libraries/Common/tests/Tests/System/StringTests.cs b/src/libraries/Common/tests/Tests/System/StringTests.cs
index 80cee6578e044db2dc2c2226448b5497c2234eca..e7cc5b6cd1572a664c6aff521cdbc247371796b5 100644 (file)
--- a/src/libraries/Common/tests/Tests/System/StringTests.cs
+++ b/src/libraries/Common/tests/Tests/System/StringTests.cs
@@ -22,6 +22,7 @@ namespace System.Tests
     public partial class StringTests
     {
         private const string SoftHyphen = "\u00AD";
+        private const string ZeroWidthJoiner = "\u200D"; // weightless in both ICU and NLS
         private static readonly char[] s_whiteSpaceCharacters = { '\u0009', '\u000a', '\u000b', '\u000c', '\u000d', '\u0020', '\u0085', '\u00a0', '\u1680' };
 
         [Theory]

diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
index 8e51249f5e50b71adfe0faa2f7cf6bc64cf894ce..50e01e12eba010ccce60757575c72b0cb1c08075 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
@@ -1021,7 +1021,11 @@ namespace System
             do
             {
                 index = ci.IndexOf(this, oldValue, startIndex, this.Length - startIndex, options, &matchLength);
-                if (index >= 0)
+
+                // There's the possibility that 'oldValue' has zero collation weight (empty string equivalent).
+                // If this is the case, we behave as if there are no more substitutions to be made.
+
+                if (index >= 0 && matchLength > 0)
                 {
                     // append the unmodified portion of string
                     result.Append(this.AsSpan(startIndex, index - startIndex));

diff --git a/src/libraries/System.Runtime/tests/System/StringTests.cs b/src/libraries/System.Runtime/tests/System/StringTests.cs
index 831b7160e5b798951246990352e542cd7ca47ee0..a7c4100ae880766934790f9ca753286ba882f031 100644 (file)
--- a/src/libraries/System.Runtime/tests/System/StringTests.cs
+++ b/src/libraries/System.Runtime/tests/System/StringTests.cs
@@ -730,6 +730,21 @@ namespace System.Tests
             AssertExtensions.Throws<ArgumentException>("oldValue", () => "abc".Replace("", "def", true, CultureInfo.CurrentCulture));
         }
 
+        [Fact]
+        public void Replace_StringComparison_WeightlessOldValue_WithOrdinalComparison_Succeeds()
+        {
+            Assert.Equal("abcdef", ("abc" + ZeroWidthJoiner).Replace(ZeroWidthJoiner, "def"));
+            Assert.Equal("abcdef", ("abc" + ZeroWidthJoiner).Replace(ZeroWidthJoiner, "def", StringComparison.Ordinal));
+            Assert.Equal("abcdef", ("abc" + ZeroWidthJoiner).Replace(ZeroWidthJoiner, "def", StringComparison.OrdinalIgnoreCase));
+        }
+
+        [Fact]
+        public void Replace_StringComparison_WeightlessOldValue_WithLinguisticComparison_TerminatesReplacement()
+        {
+            Assert.Equal("abc" + ZeroWidthJoiner + "def", ("abc" + ZeroWidthJoiner + "def").Replace(ZeroWidthJoiner, "xyz", StringComparison.CurrentCulture));
+            Assert.Equal("abc" + ZeroWidthJoiner + "def", ("abc" + ZeroWidthJoiner + "def").Replace(ZeroWidthJoiner, "xyz", true, CultureInfo.CurrentCulture));
+        }
+
         [Theory]
         [InlineData(StringComparison.CurrentCulture - 1)]
         [InlineData(StringComparison.OrdinalIgnoreCase + 1)]