From 19b4d3ff8b193467fb3f2731f695ab367429d515 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 03:34:38 +0100 Subject: [PATCH 01/11] Remove more unsafe code from IPAddress parsing --- .../System/Net/IPv4AddressHelper.Common.cs | 42 +++++++++++++------ .../System/Net/IPv6AddressHelper.Common.cs | 10 +++-- .../src/System/Net/IPAddressParser.cs | 37 ++++++---------- .../src/System/IPv4AddressHelper.cs | 12 +----- 4 files changed, 48 insertions(+), 53 deletions(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 0f57374b49616f..7256a117590b96 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -97,20 +97,32 @@ internal static int ParseHostNumber(ReadOnlySpan str, int start, i // //Remark: MUST NOT be used unless all input indexes are verified and trusted. - internal static unsafe bool IsValid(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static bool IsValid(ReadOnlySpan name, out int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme) where TChar : unmanaged, IBinaryInteger { // 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; } } + // IsValid wrapper for unsafe pointer use (TODO: remove when callers are migrated to Span) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal static unsafe bool IsValid(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme) + where TChar : unmanaged, IBinaryInteger + { + ReadOnlySpan span = new ReadOnlySpan(name + start, end - start); + bool result = IsValid(span, out int localEnd, allowIPv6, notImplicitFile, unknownScheme); + end = start + localEnd; + return result; + } + // // IsValidCanonical // @@ -124,17 +136,20 @@ internal static unsafe bool IsValid(TChar* name, int start, ref int end, // / "2" %x30-34 DIGIT ; 200-249 // / "25" %x30-35 ; 250-255 // - internal static unsafe bool IsValidCanonical(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile) + internal static bool IsValidCanonical(ReadOnlySpan name, out int end, bool allowIPv6, bool notImplicitFile) where TChar : unmanaged, IBinaryInteger { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); + end = name.Length; + int start = 0; + int dots = 0; long number = 0; bool haveNumber = false; bool firstCharIsZero = false; - while (start < end) + while (start < name.Length) { int ch = ToUShort(name[start]); @@ -162,7 +177,7 @@ internal static unsafe bool IsValidCanonical(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; @@ -210,11 +225,12 @@ internal static unsafe bool IsValidCanonical(TChar* name, int start, ref // 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* name, int start, ref int end, bool notImplicitFile) + internal static long ParseNonCanonical(ReadOnlySpan name, out int end, bool notImplicitFile) where TChar : unmanaged, IBinaryInteger { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); + end = name.Length; int numberBase = IPv4AddressHelper.Decimal; int ch = 0; Span parts = stackalloc long[3]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue. @@ -223,9 +239,9 @@ internal static unsafe long ParseNonCanonical(TChar* name, int start, ref // 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 +255,7 @@ internal static unsafe long ParseNonCanonical(TChar* name, int start, ref { current++; atLeastOneChar = true; - if (current < end) + if (current < name.Length) { ch = ToUShort(name[current]); @@ -258,7 +274,7 @@ internal static unsafe long ParseNonCanonical(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 +293,7 @@ internal static unsafe long ParseNonCanonical(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,7 +316,7 @@ internal static unsafe long ParseNonCanonical(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 } diff --git a/src/libraries/Common/src/System/Net/IPv6AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv6AddressHelper.Common.cs index d27cd18b8d56c3..79bbf162aef6c7 100644 --- a/src/libraries/Common/src/System/Net/IPv6AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv6AddressHelper.Common.cs @@ -95,7 +95,7 @@ internal static bool ShouldHaveIpv4Embedded(ReadOnlySpan 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* name, int start, int end) + internal static bool IsValidStrict(ReadOnlySpan name) where TChar : unmanaged, IBinaryInteger { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); @@ -109,6 +109,8 @@ internal static unsafe bool IsValidStrict(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* 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; diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs index e9485113f75826..3d0e9f38436be1 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs +++ b/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(ReadOnlySpan ipSpan) + public static bool IsValid(ReadOnlySpan ipSpan) where TChar : unmanaged, IBinaryInteger { - 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; } } @@ -66,13 +62,7 @@ public static unsafe bool IsValid(ReadOnlySpan ipSpan) private static unsafe bool TryParseIpv4(ReadOnlySpan ipSpan, out long address) where TChar : unmanaged, IBinaryInteger { - 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) { @@ -93,13 +83,10 @@ private static unsafe bool TryParseIPv6(ReadOnlySpan ipSpan, Span< 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 scopeIdSpan); diff --git a/src/libraries/System.Private.Uri/src/System/IPv4AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv4AddressHelper.cs index dc926f40b6a0fb..38083dc3bbafec 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv4AddressHelper.cs +++ b/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}"); From 79d0700555598386b32378aa70d5df9fe0f3952d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 04:05:36 +0100 Subject: [PATCH 02/11] Remove more unsafe code from IPAddress parsing --- .../System/Net/IPv4AddressHelper.Common.cs | 11 ---- .../src/System/Net/IPAddressParser.cs | 4 +- .../src/System/IPv6AddressHelper.cs | 23 ++++--- .../System.Private.Uri/src/System/Uri.cs | 65 ++++++++----------- 4 files changed, 42 insertions(+), 61 deletions(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 7256a117590b96..14f21a2539ba63 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -112,17 +112,6 @@ internal static bool IsValid(ReadOnlySpan name, out int end, bool } } - // IsValid wrapper for unsafe pointer use (TODO: remove when callers are migrated to Span) - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static unsafe bool IsValid(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme) - where TChar : unmanaged, IBinaryInteger - { - ReadOnlySpan span = new ReadOnlySpan(name + start, end - start); - bool result = IsValid(span, out int localEnd, allowIPv6, notImplicitFile, unknownScheme); - end = start + localEnd; - return result; - } - // // IsValidCanonical // diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs index 3d0e9f38436be1..52eb062fd9dc4c 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs @@ -59,7 +59,7 @@ public static bool IsValid(ReadOnlySpan ipSpan) throw new FormatException(SR.dns_bad_ip_address, new SocketException(SocketError.InvalidArgument)); } - private static unsafe bool TryParseIpv4(ReadOnlySpan ipSpan, out long address) + private static bool TryParseIpv4(ReadOnlySpan ipSpan, out long address) where TChar : unmanaged, IBinaryInteger { long tmpAddr = IPv4AddressHelper.ParseNonCanonical(ipSpan, out int end, notImplicitFile: true); @@ -77,7 +77,7 @@ private static unsafe bool TryParseIpv4(ReadOnlySpan ipSpan, out l return false; } - private static unsafe bool TryParseIPv6(ReadOnlySpan ipSpan, Span numbers, int numbersLength, out uint scope) + private static bool TryParseIPv6(ReadOnlySpan ipSpan, Span numbers, int numbersLength, out uint scope) where TChar : unmanaged, IBinaryInteger { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); diff --git a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs index 05dc91d1d808d0..b8ad8f52a356ba 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs +++ b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs @@ -144,8 +144,10 @@ private static bool IsLoopback(ReadOnlySpan 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 name, out int end, bool validateStrictAddress) { + int start = 0; + end = name.Length; int sequenceCount = 0; int sequenceLength = 0; bool haveCompressor = false; @@ -155,13 +157,13 @@ 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[start] == ':' && (start + 1 >= name.Length || name[start + 1] != ':')) { return false; } int i; - for (i = start; i < end; ++i) + for (i = start; 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 name, out int end) { - return InternalIsValid(name, start, ref end, false); + return InternalIsValid(name, out end, false); } } } diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 6e3777358beb99..3994bc09bdcaa3 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -1318,48 +1318,35 @@ public static UriHostNameType CheckHostName(string? name) return UriHostNameType.Unknown; } - int end = name.Length; - unsafe + if (name.StartsWith('[') && name.EndsWith(']')) { - fixed (char* fixedName = name) + // we require that _entire_ name is recognized as ipv6 address + if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int endSeq) && endSeq == name.Length) { - if (name.StartsWith('[') && name.EndsWith(']')) - { - // we require that _entire_ name is recognized as ipv6 address - if (IPv6AddressHelper.IsValid(fixedName, 1, ref end) && end == name.Length) - { - return UriHostNameType.IPv6; - } - } - - end = name.Length; - if (IPv4AddressHelper.IsValid(fixedName, 0, ref end, false, false, false) && end == name.Length) - { - return UriHostNameType.IPv4; - } + return UriHostNameType.IPv6; } + } - if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out int length) && length == name.Length) - { - return UriHostNameType.Dns; - } + if (IPv4AddressHelper.IsValid(name.AsSpan(), out int ipv4End, false, false, false) && ipv4End == name.Length) + { + return UriHostNameType.IPv4; + } - if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out length) && length == name.Length) - { - return UriHostNameType.Dns; - } + if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out int length) && length == name.Length) + { + return UriHostNameType.Dns; + } - //This checks the form without [] - end = name.Length + 2; - // we require that _entire_ name is recognized as ipv6 address - name = "[" + name + "]"; - fixed (char* newFixedName = name) - { - if (IPv6AddressHelper.IsValid(newFixedName, 1, ref end) && end == name.Length) - { - return UriHostNameType.IPv6; - } - } + if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out length) && length == name.Length) + { + return UriHostNameType.Dns; + } + + //This checks the form without [] + // we require that _entire_ name is recognized as ipv6 address + if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int ipv6End) && ipv6End == name.Length) + { + return UriHostNameType.IPv6; } return UriHostNameType.Unknown; } @@ -3886,8 +3873,9 @@ private unsafe int CheckAuthorityHelper(char* pString, int idx, int length, } if (ch == '[' && syntax.InFact(UriSyntaxFlags.AllowIPv6Host) && - IPv6AddressHelper.IsValid(pString, start + 1, ref end)) + IPv6AddressHelper.IsValid(new ReadOnlySpan(pString + (start + 1), end - (start + 1)), out int seqEnd)) { + end = start + 1 + seqEnd; if (end < length && pString[end] is not (':' or '/' or '?' or '#')) { // A valid IPv6 address wasn't followed by a valid delimiter (e.g. http://[::]extra). @@ -3904,8 +3892,9 @@ private unsafe int CheckAuthorityHelper(char* pString, int idx, int length, } } else if (char.IsAsciiDigit(ch) && syntax.InFact(UriSyntaxFlags.AllowIPv4Host) && - IPv4AddressHelper.IsValid(pString, start, ref end, false, StaticNotAny(flags, Flags.ImplicitFile), syntax.InFact(UriSyntaxFlags.V1_UnknownUri))) + IPv4AddressHelper.IsValid(new ReadOnlySpan(pString + start, end - start), out int endSeq, false, StaticNotAny(flags, Flags.ImplicitFile), syntax.InFact(UriSyntaxFlags.V1_UnknownUri))) { + end = start + endSeq; flags |= Flags.IPv4HostType; if (hasUnicode) From 925c972553af25367bd923d3099c4cface7e6001 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 14:41:09 +0100 Subject: [PATCH 03/11] fix perf regression --- .../Common/src/System/Net/IPv4AddressHelper.Common.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 14f21a2539ba63..23cbaf2c84240a 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -220,9 +220,9 @@ internal static long ParseNonCanonical(ReadOnlySpan name, out int Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); end = name.Length; - int numberBase = IPv4AddressHelper.Decimal; + int numberBase; int ch = 0; - Span parts = stackalloc long[3]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue. + Span 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; From 5eea4894228172a15418a4c9bc56f00e50b5d3db Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 17:31:34 +0100 Subject: [PATCH 04/11] fix tests --- src/libraries/System.Private.Uri/src/System/Uri.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 3994bc09bdcaa3..5c95f8ed96e71b 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -1318,33 +1318,34 @@ public static UriHostNameType CheckHostName(string? name) return UriHostNameType.Unknown; } + int end; if (name.StartsWith('[') && name.EndsWith(']')) { // we require that _entire_ name is recognized as ipv6 address - if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int endSeq) && endSeq == name.Length) + if (IPv6AddressHelper.IsValid(name.AsSpan(1), out end) && end == name.Length - 1) { return UriHostNameType.IPv6; } } - if (IPv4AddressHelper.IsValid(name.AsSpan(), out int ipv4End, false, false, false) && ipv4End == name.Length) + if (IPv4AddressHelper.IsValid(name.AsSpan(), out end, false, false, false) && end == name.Length) { return UriHostNameType.IPv4; } - if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out int length) && length == name.Length) + if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out end) && end == name.Length) { return UriHostNameType.Dns; } - if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out length) && length == name.Length) + if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out end) && end == name.Length) { return UriHostNameType.Dns; } - //This checks the form without [] + // This checks the form without [] // we require that _entire_ name is recognized as ipv6 address - if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int ipv6End) && ipv6End == name.Length) + if (IPv6AddressHelper.IsValid(name + "]", out end) && end - 1 == name.Length) { return UriHostNameType.IPv6; } From ac1340618f30ecca96ab3d222cc7b2af6bc4bbb3 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 31 Oct 2025 17:41:22 +0100 Subject: [PATCH 05/11] Update IPv4AddressHelper.Common.cs --- src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 23cbaf2c84240a..afdc368b4e4e21 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -97,7 +97,6 @@ internal static int ParseHostNumber(ReadOnlySpan str, int start, i // //Remark: MUST NOT be used unless all input indexes are verified and trusted. - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static bool IsValid(ReadOnlySpan name, out int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme) where TChar : unmanaged, IBinaryInteger { From bbf7a3255800ae73093179ddd4b024761e8f1148 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 18:18:03 +0100 Subject: [PATCH 06/11] cleanup --- .../Common/src/System/Net/IPv4AddressHelper.Common.cs | 10 +++------- .../System.Private.Uri/src/System/IPv6AddressHelper.cs | 8 ++++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index afdc368b4e4e21..81aa8de6c1716d 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -129,13 +129,12 @@ internal static bool IsValidCanonical(ReadOnlySpan name, out int e { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); - end = name.Length; - int start = 0; - + end = 0; int dots = 0; long number = 0; bool haveNumber = false; bool firstCharIsZero = false; + int start = 0; while (start < name.Length) { @@ -202,10 +201,7 @@ internal static bool IsValidCanonical(ReadOnlySpan name, out int e ++start; } bool res = (dots == 3) && haveNumber; - if (res) - { - end = start; - } + end = res ? start : 0; return res; } diff --git a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs index b8ad8f52a356ba..620cbf3f5e2675 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs +++ b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs @@ -146,8 +146,7 @@ private static bool IsLoopback(ReadOnlySpan numbers) // start must be next to '[' position, or error is reported private static bool InternalIsValid(ReadOnlySpan name, out int end, bool validateStrictAddress) { - int start = 0; - end = name.Length; + end = 0; int sequenceCount = 0; int sequenceLength = 0; bool haveCompressor = false; @@ -157,13 +156,14 @@ private static bool InternalIsValid(ReadOnlySpan name, out int end, bool v int lastSequence = 1; // Starting with a colon character is only valid if another colon follows. - if (name[start] == ':' && (start + 1 >= name.Length || name[start + 1] != ':')) + if (name[0] == ':' && (name.Length <= 1 || name[1] != ':')) { return false; } + int start = 0; int i; - for (i = start; i < name.Length; ++i) + for (i = 0; i < name.Length; ++i) { if (havePrefix ? char.IsAsciiDigit(name[i]) : char.IsAsciiHexDigit(name[i])) { From 5010008cb188aca59547a5d2ea827c4c9c355a36 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 18:22:45 +0100 Subject: [PATCH 07/11] cleanup --- src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 81aa8de6c1716d..2bb9694f98c503 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -214,7 +214,7 @@ internal static long ParseNonCanonical(ReadOnlySpan name, out int { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); - end = name.Length; + end = 0; int numberBase; int ch = 0; Span parts = [0, 0, 0]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue. From 6c661c00276287b71fa3664010a5d49337a796f6 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 20:07:32 +0100 Subject: [PATCH 08/11] fix tests --- src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 2bb9694f98c503..7be827c0378b24 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -303,12 +303,12 @@ internal static long ParseNonCanonical(ReadOnlySpan name, out int 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 From 643734bb7cc0f8c2093a2415c8c28ef1f64071f7 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 31 Oct 2025 20:09:32 +0100 Subject: [PATCH 09/11] add a comment --- .../Common/src/System/Net/IPv4AddressHelper.Common.cs | 4 ++-- .../System.Private.Uri/src/System/IPv6AddressHelper.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs index 7be827c0378b24..6a0d7f35f97592 100644 --- a/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs +++ b/src/libraries/Common/src/System/Net/IPv4AddressHelper.Common.cs @@ -129,7 +129,7 @@ internal static bool IsValidCanonical(ReadOnlySpan name, out int e { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); - end = 0; + end = 0; // Default value in case of failure int dots = 0; long number = 0; bool haveNumber = false; @@ -214,7 +214,7 @@ internal static long ParseNonCanonical(ReadOnlySpan name, out int { Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte)); - end = 0; + end = 0; // Default value in case of failure int numberBase; int ch = 0; Span parts = [0, 0, 0]; // One part per octet. Final octet doesn't have a terminator, so is stored in currentValue. diff --git a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs index 620cbf3f5e2675..dad3197d0d0134 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs +++ b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs @@ -146,7 +146,7 @@ private static bool IsLoopback(ReadOnlySpan numbers) // start must be next to '[' position, or error is reported private static bool InternalIsValid(ReadOnlySpan name, out int end, bool validateStrictAddress) { - end = 0; + end = 0; // Default value in case of failure int sequenceCount = 0; int sequenceLength = 0; bool haveCompressor = false; From b86e36818262c392f7a31680059a733dcd2a1ee8 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 1 Nov 2025 00:51:04 +0100 Subject: [PATCH 10/11] fix OOB exception --- .../System.Private.Uri/src/System/IPv6AddressHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs index dad3197d0d0134..22bf3dc54605a7 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs +++ b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs @@ -156,7 +156,7 @@ private static bool InternalIsValid(ReadOnlySpan name, out int end, bool v int lastSequence = 1; // Starting with a colon character is only valid if another colon follows. - if (name[0] == ':' && (name.Length <= 1 || name[1] != ':')) + if ((name.Length == 0) || (name[0] == ':' && (name.Length <= 1 || name[1] != ':'))) { return false; } From 74d090fcd3dab868cae3a7374e54d79b664799d0 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 1 Nov 2025 20:06:06 +0100 Subject: [PATCH 11/11] Update src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs Co-authored-by: Miha Zupan --- .../System.Private.Uri/src/System/IPv6AddressHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs index 22bf3dc54605a7..a93638779cbd6b 100644 --- a/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs +++ b/src/libraries/System.Private.Uri/src/System/IPv6AddressHelper.cs @@ -156,7 +156,7 @@ private static bool InternalIsValid(ReadOnlySpan name, out int end, bool v int lastSequence = 1; // Starting with a colon character is only valid if another colon follows. - if ((name.Length == 0) || (name[0] == ':' && (name.Length <= 1 || name[1] != ':'))) + if (name.Length < 2 || (name[0] == ':' && name[1] != ':')) { return false; }