Add USEARCH_DONE check to StartsWith in System.Globalization.Native
authorstephentoub <stoub@microsoft.com>
Sun, 15 Nov 2015 13:03:26 +0000 (08:03 -0500)
committerstephentoub <stoub@microsoft.com>
Sun, 15 Nov 2015 13:03:26 +0000 (08:03 -0500)
The StartsWith ICU wrapper was not checking the result of usearch_first
to see if it was USEARCH_DONE, indicating no match found.  This has two
ramifications:
1. When there isn't a match, USEARCH_DONE (-1) gets passed in as the
textLength argument to ucol_openElements, which treats -1 as meaning
the string isn't null-terminated, and thus ends up walking the string
looking for non-ignorable collation elements.  Our tests have been passing
because they've been using strings containing only non-ignorable
elements, and thus the first character checked causes us to bail and correctly
return false.  If nothing else, this is an unnecessary perf overhead.
2. But on top of that if there are only ignorable collation elements
before the first null character in the string (e.g. if the string begins
with a null character), then because we told ICU that the string ended
at the first null character, it'll stop walking the string and return
a match. e.g. "\0bug".StartsWith("test") returns true incorrectly.

This commit simply adds a check for USEARCH_DONE to StartsWith.
EndsWith already has such a check.

src/corefx/System.Globalization.Native/collation.cpp

index 024e0e9..fadaa73 100644 (file)
@@ -216,38 +216,40 @@ extern "C" int32_t StartsWith(
         if (U_SUCCESS(err))
         {
             idx = usearch_first(pSearch, &err);
-
-            if (idx == 0)
-            {
-                result = TRUE;
-            }
-            else
+            if (idx != USEARCH_DONE)
             {
-                UCollationElements* pCollElem = ucol_openElements(pColl, lpSource, idx, &err);
-
-                if (U_SUCCESS(err))
+                if (idx == 0)
                 {
-                    int32_t curCollElem = UCOL_NULLORDER;
-
                     result = TRUE;
+                }
+                else
+                {
+                    UCollationElements* pCollElem = ucol_openElements(pColl, lpSource, idx, &err);
 
-                    while ((curCollElem = ucol_next(pCollElem, &err)) != UCOL_NULLORDER)
+                    if (U_SUCCESS(err))
                     {
-                        if (curCollElem != 0)
+                        int32_t curCollElem = UCOL_NULLORDER;
+
+                        result = TRUE;
+
+                        while ((curCollElem = ucol_next(pCollElem, &err)) != UCOL_NULLORDER)
+                        {
+                            if (curCollElem != 0)
+                            {
+                                // Non ignorable collation element found between start of the
+                                // string and the first match for lpTarget.
+                                result = FALSE;
+                                break;
+                            }
+                        }
+
+                        if (U_FAILURE(err))
                         {
-                            // Non ignorable collation element found between start of the
-                            // string and the first match for lpTarget.
                             result = FALSE;
-                            break;
                         }
-                    }
 
-                    if (U_FAILURE(err))
-                    {
-                        result = FALSE;
+                        ucol_closeElements(pCollElem);
                     }
-
-                    ucol_closeElements(pCollElem);
                 }
             }