Fix BigInteger parsing of substring span (dotnet/corefx#35185)
authorStephen Toub <stoub@microsoft.com>
Sat, 9 Feb 2019 02:24:31 +0000 (21:24 -0500)
committerGitHub <noreply@github.com>
Sat, 9 Feb 2019 02:24:31 +0000 (21:24 -0500)
Commit migrated from https://github.com/dotnet/corefx/commit/73a3cf567a7b02ce4f984f5b791da19812fd8aa9

src/libraries/Common/src/System/Globalization/FormatProvider.Number.cs
src/libraries/System.Runtime.Numerics/tests/BigInteger/parse.netcoreapp.cs

index cda92cf..bdf79c6 100644 (file)
@@ -297,17 +297,17 @@ namespace System.Globalization
                 return (((ch) == 0x20) || ((ch) >= 0x09 && (ch) <= 0x0D));
             }
 
-            private static unsafe char* MatchChars(char* p, string str)
+            private static unsafe char* MatchChars(char* p, char* pEnd, string str)
             {
                 fixed (char* stringPointer = str)
                 {
-                    return MatchChars(p, stringPointer);
+                    return MatchChars(p, pEnd, stringPointer);
                 }
             }
 
-            private static unsafe char* MatchChars(char* p, char* str)
+            private static unsafe char* MatchChars(char* p, char* pEnd, char* str)
             {
-                Debug.Assert(p != null && str != null);
+                Debug.Assert(p != null && pEnd != null && p <= pEnd && str != null);
 
                 if (*str == '\0')
                 {
@@ -317,8 +317,13 @@ namespace System.Globalization
                 // We only hurt the failure case
                 // This fix is for French or Kazakh cultures. Since a user cannot type 0xA0 as a
                 // space character we use 0x20 space character instead to mean the same.
-                while (*p == *str || (*str == '\u00a0' && *p == '\u0020'))
+                while (true)
                 {
+                    char cp = p < pEnd ? *p : '\0';
+                    if (cp != *str && !(*str == '\u00a0' && cp == '\u0020'))
+                    {
+                        break;
+                    }
                     p++;
                     str++;
                     if (*str == '\0') return p;
@@ -326,8 +331,12 @@ namespace System.Globalization
                 return null;
             }
 
-            private static unsafe bool ParseNumber(ref char* str, NumberStyles options, ref NumberBuffer number, StringBuilder sb, NumberFormatInfo numfmt, bool parseDecimal)
+            private static unsafe bool ParseNumber(ref char* str, char* strEnd, NumberStyles options, ref NumberBuffer number, StringBuilder sb, NumberFormatInfo numfmt, bool parseDecimal)
             {
+                Debug.Assert(str != null);
+                Debug.Assert(strEnd != null);
+                Debug.Assert(str <= strEnd);
+
                 const int StateSign = 0x0001;
                 const int StateParens = 0x0002;
                 const int StateDigits = 0x0004;
@@ -362,7 +371,7 @@ namespace System.Globalization
                 int maxParseDigits = bigNumber ? int.MaxValue : NumberMaxDigits;
 
                 char* p = str;
-                char ch = *p;
+                char ch = p < strEnd ? *p : '\0';
                 char* next;
 
                 char* dig = number.digits;
@@ -373,7 +382,7 @@ namespace System.Globalization
                     // "-Kr 1231.47" is legal but "- 1231.47" is not.
                     if (!IsWhite(ch) || (options & NumberStyles.AllowLeadingWhite) == 0 || ((state & StateSign) != 0 && ((state & StateCurrency) == 0 && numfmt.NumberNegativePattern != 2)))
                     {
-                        if ((((options & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, numfmt.PositiveSign)) != null || ((next = MatchChars(p, numfmt.NegativeSign)) != null && (number.sign = true))))
+                        if ((((options & NumberStyles.AllowLeadingSign) != 0) && (state & StateSign) == 0) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || ((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null && (number.sign = true))))
                         {
                             state |= StateSign;
                             p = next - 1;
@@ -383,7 +392,7 @@ namespace System.Globalization
                             state |= StateSign | StateParens;
                             number.sign = true;
                         }
-                        else if (currSymbol != null && (next = MatchChars(p, currSymbol)) != null)
+                        else if (currSymbol != null && (next = MatchChars(p, strEnd, currSymbol)) != null)
                         {
                             state |= StateCurrency;
                             currSymbol = null;  
@@ -397,7 +406,7 @@ namespace System.Globalization
                             break;
                         }
                     }
-                    ch = *++p;
+                    ch = ++p < strEnd ? *p : '\0';
                 }
 
                 int digCount = 0;
@@ -437,12 +446,12 @@ namespace System.Globalization
                             number.scale--;
                         }
                     }
-                    else if (((options & NumberStyles.AllowDecimalPoint) != 0) && ((state & StateDecimal) == 0) && ((next = MatchChars(p, decSep)) != null || ((parsingCurrency) && (state & StateCurrency) == 0) && (next = MatchChars(p, numfmt.NumberDecimalSeparator)) != null))
+                    else if (((options & NumberStyles.AllowDecimalPoint) != 0) && ((state & StateDecimal) == 0) && ((next = MatchChars(p, strEnd, decSep)) != null || ((parsingCurrency) && (state & StateCurrency) == 0) && (next = MatchChars(p, strEnd, numfmt.NumberDecimalSeparator)) != null))
                     {
                         state |= StateDecimal;
                         p = next - 1;
                     }
-                    else if (((options & NumberStyles.AllowThousands) != 0) && ((state & StateDigits) != 0) && ((state & StateDecimal) == 0) && ((next = MatchChars(p, groupSep)) != null || ((parsingCurrency) && (state & StateCurrency) == 0) && (next = MatchChars(p, numfmt.NumberGroupSeparator)) != null))
+                    else if (((options & NumberStyles.AllowThousands) != 0) && ((state & StateDigits) != 0) && ((state & StateDecimal) == 0) && ((next = MatchChars(p, strEnd, groupSep)) != null || ((parsingCurrency) && (state & StateCurrency) == 0) && (next = MatchChars(p, strEnd, numfmt.NumberGroupSeparator)) != null))
                     {
                         p = next - 1;
                     }
@@ -450,7 +459,7 @@ namespace System.Globalization
                     {
                         break;
                     }
-                    ch = *++p;
+                    ch = ++p < strEnd ? *p : '\0';
                 }
 
                 bool negExp = false;
@@ -464,14 +473,14 @@ namespace System.Globalization
                     if ((ch == 'E' || ch == 'e') && ((options & NumberStyles.AllowExponent) != 0))
                     {
                         char* temp = p;
-                        ch = *++p;
-                        if ((next = MatchChars(p, numfmt.PositiveSign)) != null)
+                        ch = ++p < strEnd ? *p : '\0';
+                        if ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null)
                         {
-                            ch = *(p = next);
+                            ch = (p = next) < strEnd ? *p : '\0';
                         }
-                        else if ((next = MatchChars(p, numfmt.NegativeSign)) != null)
+                        else if ((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null)
                         {
-                            ch = *(p = next);
+                            ch = (p = next) < strEnd ? *p : '\0';
                             negExp = true;
                         }
                         if (ch >= '0' && ch <= '9')
@@ -480,13 +489,13 @@ namespace System.Globalization
                             do
                             {
                                 exp = exp * 10 + (ch - '0');
-                                ch = *++p;
+                                ch = ++p < strEnd ? *p : '\0';
                                 if (exp > 1000)
                                 {
                                     exp = 9999;
                                     while (ch >= '0' && ch <= '9')
                                     {
-                                        ch = *++p;
+                                        ch = ++p < strEnd ? *p : '\0';
                                     }
                                 }
                             } while (ch >= '0' && ch <= '9');
@@ -499,14 +508,14 @@ namespace System.Globalization
                         else
                         {
                             p = temp;
-                            ch = *p;
+                            ch = p < strEnd ? *p : '\0';
                         }
                     }
                     while (true)
                     {
                         if (!IsWhite(ch) || (options & NumberStyles.AllowTrailingWhite) == 0)
                         {
-                            if (((options & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0)) && ((next = MatchChars(p, numfmt.PositiveSign)) != null || (((next = MatchChars(p, numfmt.NegativeSign)) != null) && (number.sign = true))))
+                            if (((options & NumberStyles.AllowTrailingSign) != 0 && ((state & StateSign) == 0)) && ((next = MatchChars(p, strEnd, numfmt.PositiveSign)) != null || (((next = MatchChars(p, strEnd, numfmt.NegativeSign)) != null) && (number.sign = true))))
                             {
                                 state |= StateSign;
                                 p = next - 1;
@@ -515,7 +524,7 @@ namespace System.Globalization
                             {
                                 state &= ~StateParens;
                             }
-                            else if (currSymbol != null && (next = MatchChars(p, currSymbol)) != null)
+                            else if (currSymbol != null && (next = MatchChars(p, strEnd, currSymbol)) != null)
                             {
                                 currSymbol = null;
                                 p = next - 1;
@@ -525,7 +534,7 @@ namespace System.Globalization
                                 break;
                             }
                         }
-                        ch = *++p;
+                        ch = ++p < strEnd ? *p : '\0';
                     }
                     if ((state & StateParens) == 0)
                     {
@@ -568,7 +577,7 @@ namespace System.Globalization
                 fixed (char* stringPointer = &MemoryMarshal.GetReference(str))
                 {
                     char* p = stringPointer;
-                    if (!ParseNumber(ref p, options, ref number, sb, numfmt, parseDecimal)
+                    if (!ParseNumber(ref p, p + str.Length, options, ref number, sb, numfmt, parseDecimal)
                         || (p - stringPointer < str.Length && !TrailingZeros(str, (int)(p - stringPointer))))
                     {
                         return false;
index 0fab12e..0ddb376 100644 (file)
@@ -9,6 +9,24 @@ namespace System.Numerics.Tests
 {
     public partial class parseTest
     {
+        [Theory]
+        [InlineData("123456789", 0, 9, "123456789")]
+        [InlineData("123456789", 0, 1, "1")]
+        [InlineData("123456789", 1, 3, "234")]
+        public void Parse_Subspan_Success(string input, int offset, int length, string expected)
+        {
+            Eval(BigInteger.Parse(input.AsSpan(offset, length)), expected);
+            Assert.True(BigInteger.TryParse(input.AsSpan(offset, length), out BigInteger test));
+            Eval(test, expected);
+        }
+
+        [Fact]
+        public void Parse_EmptySubspan_Fails()
+        {
+            Assert.False(BigInteger.TryParse("12345".AsSpan(0, 0), out BigInteger result));
+            Assert.Equal(0, result);
+        }
+
         static partial void VerifyParseSpanToString(string num1, NumberStyles ns, bool failureNotExpected, string expected)
         {
             if (failureNotExpected)