Fix weightless code point handling in IndexOf and friends, plus other CompareInfo improvements

src/libraries/Common/src/Interop/Interop.Collation.cs

@@ -23,7 +23,7 @@ internal static partial class Globalization
         internal static extern unsafe int IndexOf(IntPtr sortHandle, char* target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options, int* matchLengthPtr);
 
         [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_LastIndexOf")]
-        internal static extern unsafe int LastIndexOf(IntPtr sortHandle, char* target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options);
+        internal static extern unsafe int LastIndexOf(IntPtr sortHandle, char* target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options, int* matchLengthPtr);
 
         [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOfOrdinalIgnoreCase")]
         internal static extern unsafe int IndexOfOrdinalIgnoreCase(string target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast);

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Globalization.cs

@@ -53,7 +53,7 @@ internal static unsafe partial class Kernel32
         [DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
         internal static extern int LocaleNameToLCID(string lpName, uint dwFlags);
 
-        [DllImport("kernel32.dll", CharSet = CharSet.Unicode)]
+        [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)]
         internal static extern int LCMapStringEx(
                     string? lpLocaleName,
                     uint dwMapFlags,

src/libraries/Common/tests/Tests/System/StringTests.cs

@@ -2516,7 +2516,15 @@ public static void EqualsTest(string s1, object obj, StringComparison comparison
                 Assert.Equal(s1.GetHashCode(), s1.GetHashCode());
             }
 
-            Assert.Equal(expected, s1.AsSpan().Equals(s2.AsSpan(), comparisonType));
+            if (string.IsNullOrEmpty(s1) && string.IsNullOrEmpty(s2))
+            {
+                // null strings are normalized to empty spans
+                Assert.True(s1.AsSpan().Equals(s2.AsSpan(), comparisonType));
+            }
+            else
+            {
+                Assert.Equal(expected, s1.AsSpan().Equals(s2.AsSpan(), comparisonType));
+            }
         }
 
         public static IEnumerable<object[]> Equals_EncyclopaediaData()
@@ -6779,6 +6787,19 @@ public static void StartEndWithTest(string source, string start, string end, str
              Assert.Equal(expected, source.EndsWith(end, ignoreCase, ci));
         }
 
+        [Theory]
+        [InlineData("", StringComparison.InvariantCulture, true)]
+        [InlineData("", StringComparison.Ordinal, true)]
+        [InlineData(ZeroWidthJoiner, StringComparison.InvariantCulture, true)]
+        [InlineData(ZeroWidthJoiner, StringComparison.Ordinal, false)]
+        public static void StartEndWith_ZeroWeightValue(string value, StringComparison comparison, bool expectedStartsAndEndsWithResult)
+        {
+            Assert.Equal(expectedStartsAndEndsWithResult, string.Empty.StartsWith(value, comparison));
+            Assert.Equal(expectedStartsAndEndsWithResult, string.Empty.EndsWith(value, comparison));
+            Assert.Equal(expectedStartsAndEndsWithResult ? 0 : -1, string.Empty.IndexOf(value, comparison));
+            Assert.Equal(expectedStartsAndEndsWithResult ? 0 : -1, string.Empty.LastIndexOf(value, comparison));
+        }
+
         [Fact]
         public static void StartEndNegativeTest()
         {

src/libraries/Native/Unix/System.Globalization.Native/pal_collation.c

@@ -445,6 +445,20 @@ int32_t GlobalizationNative_CompareString(
 
     if (U_SUCCESS(err))
     {
+        // Workaround for https://unicode-org.atlassian.net/projects/ICU/issues/ICU-9396
+        // The ucol_strcoll routine on some older versions of ICU doesn't correctly
+        // handle nullptr inputs. We'll play defensively and always flow a non-nullptr.
+
+        UChar dummyChar = 0;
+        if (lpStr1 == NULL)
+        {
+            lpStr1 = &dummyChar;
+        }
+        if (lpStr2 == NULL)
+        {
+            lpStr2 = &dummyChar; 
+        }
+
         result = ucol_strcoll(pColl, lpStr1, cwStr1Length, lpStr2, cwStr2Length);
     }
 
@@ -464,7 +478,28 @@ int32_t GlobalizationNative_IndexOf(
                         int32_t options,
                         int32_t* pMatchedLength)
 {
+    assert(cwTargetLength > 0);
+
     int32_t result = USEARCH_DONE;
+
+    // It's possible somebody passed us (source = <empty>, target = <non-empty>).
+    // ICU's usearch_* APIs don't handle empty source inputs properly. However,
+    // if this occurs the user really just wanted us to perform an equality check.
+    // We can't short-circuit the operation because depending on the collation in
+    // use, certain code points may have zero weight, which means that empty
+    // strings may compare as equal to non-empty strings.
+
+    if (cwSourceLength == 0)
+    {
+        result = GlobalizationNative_CompareString(pSortHandle, lpTarget, cwTargetLength, lpSource, cwSourceLength, options);
+        if (result == UCOL_EQUAL && pMatchedLength != NULL)
+        {
+            *pMatchedLength = cwTargetLength;
+        }
+
+        return (result == UCOL_EQUAL) ? 0 : -1;
+    }
+
     UErrorCode err = U_ZERO_ERROR;
     const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
@@ -499,9 +534,31 @@ int32_t GlobalizationNative_LastIndexOf(
                         int32_t cwTargetLength,
                         const UChar* lpSource,
                         int32_t cwSourceLength,
-                        int32_t options)
+                        int32_t options,
+                        int32_t* pMatchedLength)
 {
+    assert(cwTargetLength > 0);
+
     int32_t result = USEARCH_DONE;
+
+    // It's possible somebody passed us (source = <empty>, target = <non-empty>).
+    // ICU's usearch_* APIs don't handle empty source inputs properly. However,
+    // if this occurs the user really just wanted us to perform an equality check.
+    // We can't short-circuit the operation because depending on the collation in
+    // use, certain code points may have zero weight, which means that empty
+    // strings may compare as equal to non-empty strings.
+
+    if (cwSourceLength == 0)
+    {
+        result = GlobalizationNative_CompareString(pSortHandle, lpTarget, cwTargetLength, lpSource, cwSourceLength, options);
+        if (result == UCOL_EQUAL && pMatchedLength != NULL)
+        {
+            *pMatchedLength = cwTargetLength;
+        }
+
+        return (result == UCOL_EQUAL) ? 0 : -1;
+    }
+
     UErrorCode err = U_ZERO_ERROR;
     const UCollator* pColl = GetCollatorFromSortHandle(pSortHandle, options, &err);
 
@@ -512,6 +569,13 @@ int32_t GlobalizationNative_LastIndexOf(
         if (U_SUCCESS(err))
         {
             result = usearch_last(pSearch, &err);
+
+            // if the search was successful,
+            // we'll try to get the matched string length.
+            if (result != USEARCH_DONE && pMatchedLength != NULL)
+            {
+                *pMatchedLength = usearch_getMatchedLength(pSearch);
+            }
             usearch_close(pSearch);
         }
     }
@@ -771,14 +835,16 @@ static int32_t ComplexEndsWith(const UCollator* pCollator, UErrorCode* pErrorCod
         int32_t idx = usearch_last(pSearch, pErrorCode);
         if (idx != USEARCH_DONE)
         {
-            if ((idx + usearch_getMatchedLength(pSearch)) == patternLength)
+            int32_t matchEnd = idx + usearch_getMatchedLength(pSearch);
+            assert(matchEnd <= textLength);
+
+            if (matchEnd == textLength)
             {
                 result = TRUE;
             }
             else
             {
-                int32_t matchEnd = idx + usearch_getMatchedLength(pSearch);
-                int32_t remainingStringLength = patternLength - matchEnd;
+                int32_t remainingStringLength = textLength - matchEnd;
 
                 result = CanIgnoreAllCollationElements(pCollator, pText + matchEnd, remainingStringLength);
             }

src/libraries/Native/Unix/System.Globalization.Native/pal_collation.h

@@ -37,7 +37,8 @@ PALEXPORT int32_t GlobalizationNative_LastIndexOf(SortHandle* pSortHandle,
                                                   int32_t cwTargetLength,
                                                   const UChar* lpSource,
                                                   int32_t cwSourceLength,
-                                                  int32_t options);
+                                                  int32_t options,
+                                                  int32_t* pMatchedLength);
 
 PALEXPORT int32_t GlobalizationNative_IndexOfOrdinalIgnoreCase(const UChar* lpTarget,
                                                                int32_t cwTargetLength,

src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.Compare.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Buffers;
 using System.Collections.Generic;
 using Xunit;
 
@@ -207,6 +208,13 @@ public static IEnumerable<object[]> Compare_TestData()
             yield return new object[] { s_invariantCompare, "Test's", null, CompareOptions.None, 1 };
             yield return new object[] { s_invariantCompare, null, null, CompareOptions.None, 0 };
 
+            yield return new object[] { s_invariantCompare, "", "Tests", CompareOptions.None, -1 };
+            yield return new object[] { s_invariantCompare, "Tests", "", CompareOptions.None, 1 };
+
+            yield return new object[] { s_invariantCompare, null, "", CompareOptions.None, -1 };
+            yield return new object[] { s_invariantCompare, "", null, CompareOptions.None, 1 };
+            yield return new object[] { s_invariantCompare, "", "", CompareOptions.None, 0 };
+
             yield return new object[] { s_invariantCompare, new string('a', 5555), new string('a', 5555), CompareOptions.None, 0 };
             yield return new object[] { s_invariantCompare, "foobar", "FooB\u00C0R", CompareOptions.IgnoreNonSpace | CompareOptions.IgnoreCase, 0 };
             yield return new object[] { s_invariantCompare, "foobar", "FooB\u00C0R", CompareOptions.IgnoreNonSpace, -1 };
@@ -362,6 +370,26 @@ public void Compare_Advanced(CompareInfo compareInfo, string string1, int offset
             // Use Compare(string, int, int, string, int, int, CompareOptions)
             Assert.Equal(expected, Math.Sign(compareInfo.Compare(string1, offset1, length1, string2, offset2, length2, options)));
             Assert.Equal(-expected, Math.Sign(compareInfo.Compare(string2, offset2, length2, string1, offset1, length1, options)));
+
+            // Now test the span-based versions - use BoundedMemory to detect buffer overruns
+            // We can't run this test for null inputs since they implicitly convert to empty span
+
+            if (string1 != null && string2 != null)
+            {
+                RunSpanCompareTest(compareInfo, string1.AsSpan(offset1, length1), string2.AsSpan(offset2, length2), options, expected);
+            }
+
+            static void RunSpanCompareTest(CompareInfo compareInfo, ReadOnlySpan<char> string1, ReadOnlySpan<char> string2, CompareOptions options, int expected)
+            {
+                using BoundedMemory<char> string1BoundedMemory = BoundedMemory.AllocateFromExistingData(string1);
+                string1BoundedMemory.MakeReadonly();
+
+                using BoundedMemory<char> string2BoundedMemory = BoundedMemory.AllocateFromExistingData(string2);
+                string2BoundedMemory.MakeReadonly();
+
+                Assert.Equal(expected, Math.Sign(compareInfo.Compare(string1, string2, options)));
+                Assert.Equal(-expected, Math.Sign(compareInfo.Compare(string2, string1, options)));
+            }
         }
 
         [Fact]

src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs

@@ -2,7 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Buffers;
 using System.Collections.Generic;
+using System.Text;
 using Xunit;
 
 namespace System.Globalization.Tests
@@ -63,6 +65,10 @@ public static IEnumerable<object[]> IndexOf_TestData()
             yield return new object[] { s_invariantCompare, "TestFooBA\u0300R", "FooB\u00C0R", 0, 11, CompareOptions.IgnoreNonSpace, 4 };
             yield return new object[] { s_invariantCompare, "o\u0308", "o", 0, 2, CompareOptions.None, -1 };
 
+            // Weightless characters
+            yield return new object[] { s_invariantCompare, "", "\u200d", 0, 0, CompareOptions.None, 0 };
+            yield return new object[] { s_invariantCompare, "hello", "\u200d", 1, 3, CompareOptions.IgnoreCase, 1 };
+
             // Ignore symbols
             yield return new object[] { s_invariantCompare, "More Test's", "Tests", 0, 11, CompareOptions.IgnoreSymbols, 5 };
             yield return new object[] { s_invariantCompare, "More Test's", "Tests", 0, 11, CompareOptions.None, -1 };
@@ -192,7 +198,27 @@ public void IndexOf_String(CompareInfo compareInfo, string source, string value,
                 // Use int MemoryExtensions.IndexOf(this ReadOnlySpan<char>, ReadOnlySpan<char>, StringComparison)
                 Assert.Equal((expected == -1) ? -1 : (expected - startIndex), source.AsSpan(startIndex, count).IndexOf(value.AsSpan(), stringComparison));
             }
-         }
+
+            // Now test the span-based versions - use BoundedMemory to detect buffer overruns
+
+            RunSpanIndexOfTest(compareInfo, source.AsSpan(startIndex, count), value, options, (expected < 0) ? expected : expected - startIndex);
+
+            static void RunSpanIndexOfTest(CompareInfo compareInfo, ReadOnlySpan<char> source, ReadOnlySpan<char> value, CompareOptions options, int expected)
+            {
+                using BoundedMemory<char> sourceBoundedMemory = BoundedMemory.AllocateFromExistingData(source);
+                sourceBoundedMemory.MakeReadonly();
+
+                using BoundedMemory<char> valueBoundedMemory = BoundedMemory.AllocateFromExistingData(value);
+                valueBoundedMemory.MakeReadonly();
+
+                Assert.Equal(expected, compareInfo.IndexOf(sourceBoundedMemory.Span, valueBoundedMemory.Span, options));
+
+                if (TryCreateRuneFrom(value, out Rune rune))
+                {
+                    Assert.Equal(expected, compareInfo.IndexOf(sourceBoundedMemory.Span, rune, options)); // try the Rune-based version
+                }
+            }
+        }
 
         private static void IndexOf_Char(CompareInfo compareInfo, string source, char value, int startIndex, int count, CompareOptions options, int expected)
         {
@@ -331,14 +357,11 @@ public void IndexOf_Invalid()
             AssertExtensions.Throws<ArgumentOutOfRangeException>("count", () => s_invariantCompare.IndexOf("Test", 'a', 2, 4, CompareOptions.None));
         }
 
-        [Fact]
-        public static void IndexOf_MinusOneCompatability()
+        // Attempts to create a Rune from the entirety of a given text buffer.
+        private static bool TryCreateRuneFrom(ReadOnlySpan<char> text, out Rune value)
         {
-            // This behavior was for .NET Framework 1.1 compatability.
-            // Allowing empty source strings with invalid offsets was quickly outed.
-            // with invalid offsets.
-            Assert.Equal(0, s_invariantCompare.IndexOf("", "", -1, CompareOptions.None));
-            Assert.Equal(-1, s_invariantCompare.IndexOf("", "a", -1, CompareOptions.None));
+            return Rune.DecodeFromUtf16(text, out value, out int charsConsumed) == OperationStatus.Done
+                && charsConsumed == text.Length;
         }
     }
 }

src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IsPrefix.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Buffers;
 using System.Collections.Generic;
 using Xunit;
 
@@ -56,6 +57,9 @@ public static IEnumerable<object[]> IsPrefix_TestData()
             yield return new object[] { s_invariantCompare, "o\u0308", "o", CompareOptions.Ordinal, true };
             yield return new object[] { s_invariantCompare, "o\u0000\u0308", "o", CompareOptions.None, true };
 
+            // Weightless comparisons
+            yield return new object[] { s_invariantCompare, "", "\u200d", CompareOptions.None, true };
+
             // Surrogates
             yield return new object[] { s_invariantCompare, "\uD800\uDC00", "\uD800\uDC00", CompareOptions.None, true };
             yield return new object[] { s_invariantCompare, "\uD800\uDC00", "\uD800\uDC00", CompareOptions.IgnoreCase, true };
@@ -102,6 +106,16 @@ public void IsPrefix(CompareInfo compareInfo, string source, string value, Compa
                 Assert.Equal(expected, source.StartsWith(value, stringComparison));
                 Assert.Equal(expected, source.AsSpan().StartsWith(value.AsSpan(), stringComparison));
             }
+
+            // Now test the span version - use BoundedMemory to detect buffer overruns
+
+            using BoundedMemory<char> sourceBoundedMemory = BoundedMemory.AllocateFromExistingData<char>(source);
+            sourceBoundedMemory.MakeReadonly();
+
+            using BoundedMemory<char> valueBoundedMemory = BoundedMemory.AllocateFromExistingData<char>(value);
+            valueBoundedMemory.MakeReadonly();
+
+            Assert.Equal(expected, compareInfo.IsPrefix(sourceBoundedMemory.Span, valueBoundedMemory.Span, options));
         }
 
         [Fact]

src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IsSuffix.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Buffers;
 using System.Collections.Generic;
 using Xunit;
 
@@ -63,6 +64,9 @@ public static IEnumerable<object[]> IsSuffix_TestData()
             yield return new object[] { s_invariantCompare, "o\u0308o", "o", CompareOptions.None, true };
             yield return new object[] { s_invariantCompare, "o\u0308o", "o", CompareOptions.Ordinal, true };
 
+            // Weightless comparisons
+            yield return new object[] { s_invariantCompare, "", "\u200d", CompareOptions.None, true };
+
             // Surrogates
             yield return new object[] { s_invariantCompare, "\uD800\uDC00", "\uD800\uDC00", CompareOptions.None, true };
             yield return new object[] { s_invariantCompare, "\uD800\uDC00", "\uD800\uDC00", CompareOptions.IgnoreCase, true };
@@ -104,6 +108,16 @@ public void IsSuffix(CompareInfo compareInfo, string source, string value, Compa
                 Assert.Equal(expected, source.EndsWith(value, stringComparison));
                 Assert.Equal(expected, source.AsSpan().EndsWith(value.AsSpan(), stringComparison));
             }
+
+            // Now test the span version - use BoundedMemory to detect buffer overruns
+
+            using BoundedMemory<char> sourceBoundedMemory = BoundedMemory.AllocateFromExistingData<char>(source);
+            sourceBoundedMemory.MakeReadonly();
+
+            using BoundedMemory<char> valueBoundedMemory = BoundedMemory.AllocateFromExistingData<char>(value);
+            valueBoundedMemory.MakeReadonly();
+
+            Assert.Equal(expected, compareInfo.IsSuffix(sourceBoundedMemory.Span, valueBoundedMemory.Span, options));
         }
 
         [Fact]