From 5615d90fb5c0553f199405d4b08d81401ca16937 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Mon, 17 Apr 2017 22:02:08 -0700 Subject: [PATCH] Fix String.Replace implementation (dotnet/coreclr#11039) * 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 Commit migrated from https://github.com/dotnet/coreclr/commit/cb371832f635664f74f99b9e0fa405eef2148a2f --- .../src/System/Globalization/CompareInfo.cs | 48 +++++++++++++++++++++- .../src/mscorlib/src/System/String.Manipulation.cs | 5 ++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/coreclr/src/mscorlib/src/System/Globalization/CompareInfo.cs b/src/coreclr/src/mscorlib/src/System/Globalization/CompareInfo.cs index 285a81d..b2c2208 100644 --- a/src/coreclr/src/mscorlib/src/System/Globalization/CompareInfo.cs +++ b/src/coreclr/src/mscorlib/src/System/Globalization/CompareInfo.cs @@ -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) diff --git a/src/coreclr/src/mscorlib/src/System/String.Manipulation.cs b/src/coreclr/src/mscorlib/src/System/String.Manipulation.cs index 2b1f5a6..b71ffa2 100644 --- a/src/coreclr/src/mscorlib/src/System/String.Manipulation.cs +++ b/src/coreclr/src/mscorlib/src/System/String.Manipulation.cs @@ -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 -- 2.7.4