From b4127294e51a3ffc3971bda30ec0d5cec8a1049c Mon Sep 17 00:00:00 2001 From: John Date: Tue, 29 Apr 2014 13:54:26 +0200 Subject: [PATCH] U4-2635 Skip past replaced word when checking Avoids infinite loop if search and replacement are equal --- src/Umbraco.Core/StringExtensions.cs | 10 ++++++---- src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs | 3 +++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index c7ea0c43dd..6e1b367669 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -1191,11 +1191,13 @@ namespace Umbraco.Core /// Updated string public static string Replace(this string source, string oldString, string newString, StringComparison stringComparison) { - // Initialised to zero so it can be used as startIndex on first iteration. - int index = 0; + // This initialisation ensures the first check starts at index zero of the source. On successive checks for + // a match, the source is skipped to immediately after the last replaced occurrence for efficiency + // and to avoid infinite loops when oldString and newString compare equal. + int index = -1 * newString.Length; - // Determine if there are any matches left in source, starting from last found match. - while((index = source.IndexOf(oldString, index, stringComparison)) >= 0) + // Determine if there are any matches left in source, starting from just after the result of replacing the last match. + while((index = source.IndexOf(oldString, index + newString.Length, stringComparison)) >= 0) { // Remove the old text. source = source.Remove(index, oldString.Length); diff --git a/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs b/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs index d1c7efa489..89d03baa2d 100644 --- a/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs +++ b/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs @@ -83,6 +83,9 @@ namespace Umbraco.Tests.CoreStrings [TestCase("Hello this is my string", "hello", "replaced", "replaced this is my string", StringComparison.CurrentCultureIgnoreCase)] [TestCase("Hello this is hello my string", "hello", "replaced", "replaced this is replaced my string", StringComparison.CurrentCultureIgnoreCase)] [TestCase("Hello this is my string", "nonexistent", "replaced", "Hello this is my string", StringComparison.CurrentCultureIgnoreCase)] + [TestCase("Hellohello this is my string", "hello", "replaced", "replacedreplaced this is my string", StringComparison.CurrentCultureIgnoreCase)] + // Ensure replacing with the same string doesn't cause infinite loop. + [TestCase("Hello this is my string", "hello", "hello", "hello this is my string", StringComparison.CurrentCultureIgnoreCase)] public void ReplaceWithStringComparison(string input, string oldString, string newString, string shouldBe, StringComparison stringComparison) { var replaced = input.Replace(oldString, newString, stringComparison);