Remove more unsafe code from IPAddress parsing

src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs

@@ -97,17 +97,17 @@ internal static int ParseHostNumber<TChar>(ReadOnlySpan<TChar> str, int start, i
         //
 
         //Remark: MUST NOT be used unless all input indexes are verified and trusted.
-        internal static unsafe bool IsValid<TChar>(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme)
+        internal static bool IsValid<TChar>(ReadOnlySpan<TChar> name, out int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
             // IPv6 can only have canonical IPv4 embedded. Unknown schemes will not attempt parsing of non-canonical IPv4 addresses.
             if (allowIPv6 || unknownScheme)
             {
-                return IsValidCanonical(name, start, ref end, allowIPv6, notImplicitFile);
+                return IsValidCanonical(name, out end, allowIPv6, notImplicitFile);
             }
             else
             {
-                return ParseNonCanonical(name, start, ref end, notImplicitFile) != Invalid;
+                return ParseNonCanonical(name, out end, notImplicitFile) != Invalid;
             }
         }
 
@@ -124,17 +124,19 @@ internal static unsafe bool IsValid<TChar>(TChar* name, int start, ref int end,
         //                 / "2" %x30-34 DIGIT     ; 200-249
         //                 / "25" %x30-35          ; 250-255
         //
-        internal static unsafe bool IsValidCanonical<TChar>(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile)
+        internal static bool IsValidCanonical<TChar>(ReadOnlySpan<TChar> name, out int end, bool allowIPv6, bool notImplicitFile)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
             Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte));
 
+            end = 0; // Default value in case of failure
             int dots = 0;
             long number = 0;
             bool haveNumber = false;
             bool firstCharIsZero = false;
+            int start = 0;
 
-            while (start < end)
+            while (start < name.Length)
             {
                 int ch = ToUShort(name[start]);
 
@@ -162,7 +164,7 @@ internal static unsafe bool IsValidCanonical<TChar>(TChar* name, int start, ref
                     // A number starting with zero should be interpreted in base 8 / octal
                     if (!haveNumber && parsedCharacter == 0)
                     {
-                        if ((start + 1 < end) && name[start + 1] == TChar.CreateTruncating('0'))
+                        if ((start + 1 < name.Length) && name[start + 1] == TChar.CreateTruncating('0'))
                         {
                             // 00 is not allowed as a prefix.
                             return false;
@@ -199,33 +201,31 @@ internal static unsafe bool IsValidCanonical<TChar>(TChar* name, int start, ref
                 ++start;
             }
             bool res = (dots == 3) && haveNumber;
-            if (res)
-            {
-                end = start;
-            }
+            end = res ? start : 0;
             return res;
         }
 
         // Parse any canonical or noncanonical IPv4 formats and return a long between 0 and MaxIPv4Value.
         // Return Invalid (-1) for failures.
         // If the address has less than three dots, only the rightmost section is assumed to contain the combined value for
         // the missing sections: 0xFF00FFFF == 0xFF.0x00.0xFF.0xFF == 0xFF.0xFFFF
-        internal static unsafe long ParseNonCanonical<TChar>(TChar* name, int start, ref int end, bool notImplicitFile)
+        internal static long ParseNonCanonical<TChar>(ReadOnlySpan<TChar> name, out int end, bool notImplicitFile)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
             Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte));
 
-            int numberBase = IPv4AddressHelper.Decimal;
+            end = 0; // Default value in case of failure
+            int numberBase;
             int ch = 0;
-            Span<long> parts = stackalloc long[3]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue.
+            Span<long> parts = [0, 0, 0]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue.
             long currentValue = 0;
             bool atLeastOneChar = false;
 
             // Parse one dotted section at a time
             int dotCount = 0; // Limit 3
-            int current = start;
+            int current = 0;
 
-            for (; current < end; current++)
+            for (; current < name.Length; current++)
             {
                 ch = ToUShort(name[current]);
                 currentValue = 0;
@@ -239,7 +239,7 @@ internal static unsafe long ParseNonCanonical<TChar>(TChar* name, int start, ref
                 {
                     current++;
                     atLeastOneChar = true;
-                    if (current < end)
+                    if (current < name.Length)
                     {
                         ch = ToUShort(name[current]);
 
@@ -258,7 +258,7 @@ internal static unsafe long ParseNonCanonical<TChar>(TChar* name, int start, ref
                 }
 
                 // Parse this section
-                for (; current < end; current++)
+                for (; current < name.Length; current++)
                 {
                     ch = ToUShort(name[current]);
                     int digitValue = HexConverter.FromChar(ch);
@@ -277,7 +277,7 @@ internal static unsafe long ParseNonCanonical<TChar>(TChar* name, int start, ref
                     atLeastOneChar = true;
                 }
 
-                if (current < end && ch == '.')
+                if (current < name.Length && ch == '.')
                 {
                     if (dotCount >= 3 // Max of 3 dots and 4 segments
                         || !atLeastOneChar // No empty segments: 1...1
@@ -300,15 +300,15 @@ internal static unsafe long ParseNonCanonical<TChar>(TChar* name, int start, ref
             {
                 return Invalid;  // Empty trailing segment: 1.1.1.
             }
-            else if (current >= end)
+            else if (current >= name.Length)
             {
                 // end of string, allowed
+                end = name.Length;
             }
             else if (ch == '/' || ch == '\\' || (notImplicitFile && (ch == ':' || ch == '?' || ch == '#')))
             {
                 // For a normal IPv4 address, the terminator is the prefix ('/' or its counterpart, '\'). If notImplicitFile is set, the terminator
                 // is one of the characters which signify the start of the rest of the URI - the port number (':'), query string ('?') or fragment ('#')
-
                 end = current;
             }
             else

src/libraries/Common/src/System/Net/IPv6AddressHelper.Common.cs

@@ -95,7 +95,7 @@ internal static bool ShouldHaveIpv4Embedded(ReadOnlySpan<ushort> numbers)
 
         //  Remarks: MUST NOT be used unless all input indexes are verified and trusted.
         //           start must be next to '[' position, or error is reported
-        internal static unsafe bool IsValidStrict<TChar>(TChar* name, int start, int end)
+        internal static bool IsValidStrict<TChar>(ReadOnlySpan<TChar> name)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
             Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte));
@@ -109,6 +109,8 @@ internal static unsafe bool IsValidStrict<TChar>(TChar* name, int start, int end
             bool expectingNumber = true;
             // Start position of the previous component
             int lastSequence = 1;
+            int start = 0;
+            int end = name.Length;
 
             bool needsClosingBracket = false;
 
@@ -244,15 +246,15 @@ internal static unsafe bool IsValidStrict<TChar>(TChar* name, int start, int end
                                 return false;
                             }
 
-                            i = end;
-                            if (!IPv4AddressHelper.IsValid(name, lastSequence, ref i, true, false, false))
+                            if (!IPv4AddressHelper.IsValid(name.Slice(lastSequence, end - lastSequence), out int seqEnd, true, false, false))
                             {
                                 return false;
                             }
+                            i = lastSequence + seqEnd;
+
                             // An IPv4 address takes 2 slots in an IPv6 address. One was just counted meeting the '.'
                             ++sequenceCount;
                             lastSequence = i - sequenceLength;
-                            sequenceLength = 0;
                             haveIPv4Address = true;
                             --i;            // it will be incremented back on the next loop
                             break;

src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs

@@ -16,21 +16,17 @@ internal static class IPAddressParser
         internal const int MaxIPv4StringLength = 15; // 4 numbers separated by 3 periods, with up to 3 digits per number
         internal const int MaxIPv6StringLength = 65;
 
-        public static unsafe bool IsValid<TChar>(ReadOnlySpan<TChar> ipSpan)
+        public static bool IsValid<TChar>(ReadOnlySpan<TChar> ipSpan)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
-            fixed (TChar* ipStringPtr = &MemoryMarshal.GetReference(ipSpan))
+            if (ipSpan.Contains(TChar.CreateTruncating(':')))
             {
-                if (ipSpan.Contains(TChar.CreateTruncating(':')))
-                {
-                    return IPv6AddressHelper.IsValidStrict(ipStringPtr, 0, ipSpan.Length);
-                }
-                else
-                {
-                    int end = ipSpan.Length;
-                    long address = IPv4AddressHelper.ParseNonCanonical(ipStringPtr, 0, ref end, notImplicitFile: true);
-                    return address != IPv4AddressHelper.Invalid && end == ipSpan.Length;
-                }
+                return IPv6AddressHelper.IsValidStrict(ipSpan);
+            }
+            else
+            {
+                long address = IPv4AddressHelper.ParseNonCanonical(ipSpan, out int end, notImplicitFile: true);
+                return address != IPv4AddressHelper.Invalid && end == ipSpan.Length;
             }
         }
 
@@ -63,16 +59,10 @@ public static unsafe bool IsValid<TChar>(ReadOnlySpan<TChar> ipSpan)
             throw new FormatException(SR.dns_bad_ip_address, new SocketException(SocketError.InvalidArgument));
         }
 
-        private static unsafe bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out long address)
+        private static bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out long address)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
-            int end = ipSpan.Length;
-            long tmpAddr;
-
-            fixed (TChar* ipStringPtr = &MemoryMarshal.GetReference(ipSpan))
-            {
-                tmpAddr = IPv4AddressHelper.ParseNonCanonical(ipStringPtr, 0, ref end, notImplicitFile: true);
-            }
+            long tmpAddr = IPv4AddressHelper.ParseNonCanonical(ipSpan, out int end, notImplicitFile: true);
 
             if (tmpAddr != IPv4AddressHelper.Invalid && end == ipSpan.Length)
             {
@@ -87,19 +77,16 @@ private static unsafe bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out l
             return false;
         }
 
-        private static unsafe bool TryParseIPv6<TChar>(ReadOnlySpan<TChar> ipSpan, Span<ushort> numbers, int numbersLength, out uint scope)
+        private static bool TryParseIPv6<TChar>(ReadOnlySpan<TChar> ipSpan, Span<ushort> numbers, int numbersLength, out uint scope)
             where TChar : unmanaged, IBinaryInteger<TChar>
         {
             Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte));
             Debug.Assert(numbersLength >= IPAddressParserStatics.IPv6AddressShorts);
 
-            fixed (TChar* ipStringPtr = &MemoryMarshal.GetReference(ipSpan))
+            if (!IPv6AddressHelper.IsValidStrict(ipSpan))
             {
-                if (!IPv6AddressHelper.IsValidStrict(ipStringPtr, 0, ipSpan.Length))
-                {
-                    scope = 0;
-                    return false;
-                }
+                scope = 0;
+                return false;
             }
 
             IPv6AddressHelper.Parse(ipSpan, numbers, out ReadOnlySpan<TChar> scopeIdSpan);

src/libraries/System.Private.Uri/src/System/IPv4AddressHelper.cs

@@ -13,17 +13,7 @@ internal static partial class IPv4AddressHelper
         // Parse and canonicalize
         internal static string ParseCanonicalName(string str, int start, int end, ref bool isLoopback)
         {
-            // end includes ports, so changedEnd may be different from end
-            int changedEnd = end;
-            long result;
-
-            unsafe
-            {
-                fixed (char* ipString = str)
-                {
-                    result = ParseNonCanonical(ipString, start, ref changedEnd, true);
-                }
-            }
+            long result = ParseNonCanonical(str.AsSpan(start), out _, true);
 
             Debug.Assert(result != Invalid, $"Failed to parse after already validated: {str}");
 

src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs

@@ -144,8 +144,9 @@ private static bool IsLoopback(ReadOnlySpan<ushort> numbers)
 
         //  Remarks: MUST NOT be used unless all input indexes are verified and trusted.
         //           start must be next to '[' position, or error is reported
-        private static unsafe bool InternalIsValid(char* name, int start, ref int end, bool validateStrictAddress)
+        private static bool InternalIsValid(ReadOnlySpan<char> name, out int end, bool validateStrictAddress)
         {
+            end = 0; // Default value in case of failure
             int sequenceCount = 0;
             int sequenceLength = 0;
             bool haveCompressor = false;
@@ -155,13 +156,14 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
             int lastSequence = 1;
 
             // Starting with a colon character is only valid if another colon follows.
-            if (name[start] == ':' && (start + 1 >= end || name[start + 1] != ':'))
+            if (name.Length < 2 || (name[0] == ':' && name[1] != ':'))
             {
                 return false;
             }
 
+            int start = 0;
             int i;
-            for (i = start; i < end; ++i)
+            for (i = 0; i < name.Length; ++i)
             {
                 if (havePrefix ? char.IsAsciiDigit(name[i]) : char.IsAsciiHexDigit(name[i]))
                 {
@@ -185,7 +187,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
                             while (true)
                             {
                                 //accept anything in scopeID
-                                if (++i == end)
+                                if (++i == name.Length)
                                 {
                                     // no closing ']', fail
                                     return false;
@@ -201,7 +203,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
                             }
                         case ']':
                             start = i;
-                            i = end;
+                            i = name.Length;
                             //this will make i = end+1
                             continue;
                         case ':':
@@ -243,11 +245,12 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
                                 return false;
                             }
 
-                            i = end;
-                            if (!IPv4AddressHelper.IsValid(name, lastSequence, ref i, true, false, false))
+                            i = name.Length;
+                            if (!IPv4AddressHelper.IsValid(name.Slice(lastSequence, i - lastSequence), out int seqEnd, true, false, false))
                             {
                                 return false;
                             }
+                            i = lastSequence + seqEnd;
                             // ipv4 address takes 2 slots in ipv6 address, one was just counted meeting the '.'
                             ++sequenceCount;
                             haveIPv4Address = true;
@@ -278,7 +281,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
 
             if (!expectingNumber && (sequenceLength <= 4) && (haveCompressor ? (sequenceCount < expectedSequenceCount) : (sequenceCount == expectedSequenceCount)))
             {
-                if (i == end + 1)
+                if (i == name.Length + 1)
                 {
                     // ']' was found
                     end = start + 1;
@@ -321,9 +324,9 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
         //  Remarks: MUST NOT be used unless all input indexes are verified and trusted.
         //           start must be next to '[' position, or error is reported
 
-        internal static unsafe bool IsValid(char* name, int start, ref int end)
+        internal static bool IsValid(ReadOnlySpan<char> name, out int end)
         {
-            return InternalIsValid(name, start, ref end, false);
+            return InternalIsValid(name, out end, false);
         }
     }
 }