Fix offset + globalization issues in StringSegment

src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs

@@ -146,24 +146,20 @@ public char this[int index]
         /// </returns>
         public static int Compare(StringSegment a, StringSegment b, StringComparison comparisonType)
         {
-            int minLength = Math.Min(a.Length, b.Length);
-            int diff = string.Compare(a.Buffer, a.Offset, b.Buffer, b.Offset, minLength, comparisonType);
-            if (diff == 0)
+            if (a.HasValue && b.HasValue)
             {
-                diff = a.Length - b.Length;
+                return a.AsSpan().CompareTo(b.AsSpan(), comparisonType);
+            }
+            else
+            {
+                CheckStringComparison(comparisonType); // must arg check before returning
+                return !a.HasValue ? (b.HasValue ? -1 : 0) : 1; // null sorts less than non-null, and two nulls sort as equal
             }
-
-            return diff;
         }
 
         /// <inheritdoc />
         public override bool Equals(object obj)
         {
-            if (obj is null)
-            {
-                return false;
-            }
-
             return obj is StringSegment segment && Equals(segment);
         }
 
@@ -182,12 +178,15 @@ public override bool Equals(object obj)
         /// <returns><see langword="true" /> if the current object is equal to the other parameter; otherwise, <see langword="false" />.</returns>
         public bool Equals(StringSegment other, StringComparison comparisonType)
         {
-            if (Length != other.Length)
+            if (HasValue && other.HasValue)
             {
-                return false;
+                return AsSpan().Equals(other.AsSpan(), comparisonType);
+            }
+            else
+            {
+                CheckStringComparison(comparisonType); // must arg check before returning
+                return !HasValue && !other.HasValue; // only return true if both are null
             }
-
-            return string.Compare(Buffer, Offset, other.Buffer, other.Offset, other.Length, comparisonType) == 0;
         }
 
         // This handles StringSegment.Equals(string, StringSegment, StringComparison) and StringSegment.Equals(StringSegment, string, StringComparison)
@@ -232,20 +231,20 @@ public bool Equals(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            int textLength = text.Length;
-            if (!HasValue || Length != textLength)
+            if (!HasValue)
             {
+                CheckStringComparison(comparisonType); // must arg check before returning
                 return false;
             }
 
-            return string.Compare(Buffer, Offset, text, 0, textLength, comparisonType) == 0;
+            return AsSpan().Equals(text.AsSpan(), comparisonType);
         }
 
         /// <inheritdoc />
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public override int GetHashCode()
         {
-#if NETCOREAPP || NETSTANDARD2_1
+#if NETCOREAPP
             return string.GetHashCode(AsSpan());
 #elif (NETSTANDARD2_0 || NETFRAMEWORK)
             // This GetHashCode is expensive since it allocates on every call.
@@ -255,7 +254,6 @@ public override int GetHashCode()
 #else
 #error Target frameworks need to be updated.
 #endif
-
         }
 
         /// <summary>
@@ -310,15 +308,13 @@ public bool StartsWith(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            bool result = false;
-            int textLength = text.Length;
-
-            if (HasValue && Length >= textLength)
+            if (!HasValue)
             {
-                result = string.Compare(Buffer, Offset, text, 0, textLength, comparisonType) == 0;
+                CheckStringComparison(comparisonType); // must arg check before returning
+                return false;
             }
 
-            return result;
+            return AsSpan().StartsWith(text.AsSpan(), comparisonType);
         }
 
         /// <summary>
@@ -338,16 +334,13 @@ public bool EndsWith(string text, StringComparison comparisonType)
                 ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
             }
 
-            bool result = false;
-            int textLength = text.Length;
-            int comparisonLength = Offset + Length - textLength;
-
-            if (HasValue && comparisonLength > 0)
+            if (!HasValue)
             {
-                result = string.Compare(Buffer, comparisonLength, text, 0, textLength, comparisonType) == 0;
+                CheckStringComparison(comparisonType); // must arg check before returning
+                return false;
             }
 
-            return result;
+            return AsSpan().EndsWith(text.AsSpan(), comparisonType);
         }
 
         /// <summary>
@@ -445,10 +438,10 @@ public int IndexOf(char c, int start, int count)
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.count);
             }
 
-            int index = Buffer.IndexOf(c, offset, count);
-            if (index != -1)
+            int index = AsSpan().Slice(start, count).IndexOf(c);
+            if (index >= 0)
             {
-                index -= Offset;
+                index += start;
             }
 
             return index;
@@ -552,18 +545,7 @@ public int IndexOfAny(char[] anyOf)
         /// <returns>The zero-based index position of value if that character is found, or -1 if it is not.</returns>
         public int LastIndexOf(char value)
         {
-            int index = -1;
-
-            if (HasValue)
-            {
-                index = Buffer.LastIndexOf(value, Offset + Length - 1, Length);
-                if (index != -1)
-                {
-                    index -= Offset;
-                }
-            }
-
-            return index;
+            return AsSpan().LastIndexOf(value);
         }
 
         /// <summary>
@@ -576,54 +558,40 @@ public int LastIndexOf(char value)
         /// Removes all leading whitespaces.
         /// </summary>
         /// <returns>The trimmed <see cref="StringSegment"/>.</returns>
-        public unsafe StringSegment TrimStart()
+        public StringSegment TrimStart()
         {
-            int trimmedStart = Offset;
-            int length = Offset + Length;
+            ReadOnlySpan<char> span = AsSpan();
 
-            fixed (char* p = Buffer)
+            int i;
+            for (i = 0; i < span.Length; i++)
             {
-                while (trimmedStart < length)
+                if (!char.IsWhiteSpace(span[i]))
                 {
-                    char c = p[trimmedStart];
-
-                    if (!char.IsWhiteSpace(c))
-                    {
-                        break;
-                    }
-
-                    trimmedStart++;
+                    break;
                 }
             }
 
-            return new StringSegment(Buffer, trimmedStart, length - trimmedStart);
+            return Subsegment(i);
         }
 
         /// <summary>
         /// Removes all trailing whitespaces.
         /// </summary>
         /// <returns>The trimmed <see cref="StringSegment"/>.</returns>
-        public unsafe StringSegment TrimEnd()
+        public StringSegment TrimEnd()
         {
-            int offset = Offset;
-            int trimmedEnd = offset + Length - 1;
+            ReadOnlySpan<char> span = AsSpan();
 
-            fixed (char* p = Buffer)
+            int i;
+            for (i = span.Length - 1; i >= 0; i--)
             {
-                while (trimmedEnd >= offset)
+                if (!char.IsWhiteSpace(span[i]))
                 {
-                    char c = p[trimmedEnd];
-
-                    if (!char.IsWhiteSpace(c))
-                    {
-                        break;
-                    }
-
-                    trimmedEnd--;
+                    break;
                 }
             }
 
-            return new StringSegment(Buffer, offset, trimmedEnd - offset + 1);
+            return Subsegment(0, i + 1);
         }
 
         /// <summary>
@@ -664,6 +632,15 @@ public override string ToString()
             return Value ?? string.Empty;
         }
 
+        private static void CheckStringComparison(StringComparison comparisonType)
+        {
+            // Single comparison to check if comparisonType is within [CurrentCulture .. OrdinalIgnoreCase]
+            if ((uint)comparisonType > (uint)StringComparison.OrdinalIgnoreCase)
+            {
+                ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.comparisonType);
+            }
+        }
+
         // Methods that do no return (i.e. throw) are not inlined
         // https://github.com/dotnet/coreclr/pull/6103
         private static void ThrowInvalidArguments(string buffer, int offset, int length)
@@ -716,5 +693,14 @@ Exception GetInvalidArgumentsException(bool hasValue)
                 return ThrowHelper.GetArgumentException(ExceptionResource.Argument_InvalidOffsetLengthStringSegment);
             }
         }
+
+        /// <inheritdoc />
+        bool IEquatable<string>.Equals(string other)
+        {
+            // Explicit interface implementation for IEquatable<string> because
+            // the interface's Equals method allows null strings, which we return
+            // as not-equal.
+            return other != null && Equals(other);
+        }
     }
 }

src/libraries/Microsoft.Extensions.Primitives/src/ThrowHelper.cs

@@ -83,7 +83,8 @@ internal enum ExceptionArgument
         index,
         value,
         capacity,
-        separators
+        separators,
+        comparisonType
     }
 
     internal enum ExceptionResource