Merge pull request umbraco#367 from lctjohn/7.1.1-U4-2635

Shazwazza · Shazwazza · commit e1d99855f6af · 2014-04-30T11:10:28.000+10:00
fix for: U4-2635 Replace [...] method only replaces first ocurrence of oldString
diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs
@@ -1191,17 +1191,18 @@ public static string ToSafeFileName(this string text, CultureInfo culture)
         /// <returns>Updated string</returns>
         public static string Replace(this string source, string oldString, string newString, StringComparison stringComparison)
         {
-            var index = source.IndexOf(oldString, stringComparison);
+            // 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 we found a match
-            var matchFound = index >= 0;
-
-            if (matchFound)
+            // 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
+                // Remove the old text.
                 source = source.Remove(index, oldString.Length);
 
-                // Add the replacemenet text
+                // Add the replacemenet text.
                 source = source.Insert(index, newString);
             }
 
diff --git a/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs b/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs
@@ -80,6 +80,18 @@ public void TrimStart(string input, string forTrimming, string shouldBe)
             Assert.AreEqual(shouldBe, trimmed);
         }
 
+        [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);
+            Assert.AreEqual(shouldBe, replaced);
+        }
+
         [TestCase(null, null)]
         [TestCase("", "")]
         [TestCase("x", "X")]
