From 50bd5ba4ea38e293d71b6f96c3b8c10b907bdb57 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Mon, 10 Feb 2020 20:29:53 -0800 Subject: [PATCH] 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. --- .../Common/tests/Tests/System/StringTests.cs | 1 + .../src/System/String.Manipulation.cs | 6 +++++- .../System.Runtime/tests/System/StringTests.cs | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/Tests/System/StringTests.cs b/src/libraries/Common/tests/Tests/System/StringTests.cs index 80cee6578e044..e7cc5b6cd1572 100644 --- 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 8e51249f5e50b..50e01e12eba01 100644 --- 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 @@ private unsafe string ReplaceCore(string oldValue, string? newValue, CultureInfo 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 831b7160e5b79..a7c4100ae8807 100644 --- a/src/libraries/System.Runtime/tests/System/StringTests.cs +++ b/src/libraries/System.Runtime/tests/System/StringTests.cs @@ -730,6 +730,21 @@ public void Replace_StringComparison_EmptyOldValue_ThrowsArgumentException() AssertExtensions.Throws("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)]