Fix String.Replace implementation (#11039)
authorTarek Mahmoud Sayed <tarekms@microsoft.com>
Tue, 18 Apr 2017 05:02:08 +0000 (22:02 -0700)
committerGitHub <noreply@github.com>
Tue, 18 Apr 2017 05:02:08 +0000 (22:02 -0700)
* Fix String.Replace implementation

String.Replace was callin ga low level CompareInfo method IndexOfCore which cannot be called in some conditions like when using ordinal ignore case option or when running in invariant mode.
The fix is to call a higher level method in CompareInfo which can handle such cases becfore calling the lower level method

* fix typo

src/mscorlib/src/System/Globalization/CompareInfo.cs
src/mscorlib/src/System/String.Manipulation.cs

index 285a81d..b2c2208 100644 (file)
@@ -752,7 +752,6 @@ namespace System.Globalization
             return IndexOfCore(source, new string(value, 1), startIndex, count, options, null);
         }
 
-
         public unsafe virtual int IndexOf(string source, string value, int startIndex, int count, CompareOptions options)
         {
             // Validate inputs
@@ -802,6 +801,53 @@ namespace System.Globalization
             return IndexOfCore(source, value, startIndex, count, options, null);
         }
 
+        // The following IndexOf overload is mainly used by String.Replace. This overload assumes the parameters are already validated
+        // and the caller is passing a valid matchLengthPtr pointer.
+        internal unsafe int IndexOf(string source, string value, int startIndex, int count, CompareOptions options, int* matchLengthPtr)
+        {
+            Debug.Assert(source != null);
+            Debug.Assert(value != null);
+            Debug.Assert(startIndex >= 0);
+            Debug.Assert(matchLengthPtr != null);
+            *matchLengthPtr = 0;
+
+            if (source.Length == 0)
+            {
+                if (value.Length == 0)
+                {
+                    return 0;
+                }
+                return -1;
+            }
+
+            if (startIndex >= source.Length)
+            {
+                return -1;
+            }
+
+            if (options == CompareOptions.OrdinalIgnoreCase)
+            {
+                int res = IndexOfOrdinal(source, value, startIndex, count, ignoreCase: true);
+                if (res >= 0)
+                {
+                    *matchLengthPtr = value.Length;
+                }
+                return res;
+            }
+
+            if (_invariantMode)
+            {
+                int res = IndexOfOrdinal(source, value, startIndex, count, ignoreCase: (options & (CompareOptions.IgnoreCase | CompareOptions.OrdinalIgnoreCase)) != 0);
+                if (res >= 0)
+                {
+                    *matchLengthPtr = value.Length;
+                }
+                return res;
+            }
+
+            return IndexOfCore(source, value, startIndex, count, options, matchLengthPtr);
+        }
+
         internal int IndexOfOrdinal(string source, string value, int startIndex, int count, bool ignoreCase)
         {
             if (_invariantMode)
index 2b1f5a6..b71ffa2 100644 (file)
@@ -1025,7 +1025,7 @@ namespace System
             if (oldValue.Length == 0)
                 throw new ArgumentException(SR.Argument_StringZeroLength, nameof(oldValue));
 
-            // If they asked to replace oldValue with a null, replace all occurences
+            // If they asked to replace oldValue with a null, replace all occurrences
             // with the empty string.
             if (newValue == null)
                 newValue = string.Empty;
@@ -1039,10 +1039,11 @@ namespace System
             int matchLength = 0;
 
             bool hasDoneAnyReplacements = false;
+            CompareInfo ci = referenceCulture.CompareInfo;
 
             do
             {
-                index = referenceCulture.CompareInfo.IndexOfCore(this, oldValue, startIndex, m_stringLength - startIndex, options, &matchLength);
+                index = ci.IndexOf(this, oldValue, startIndex, m_stringLength - startIndex, options, &matchLength);
                 if (index >= 0)
                 {
                     // append the unmodified portion of string