Skip to content

Commit 79d0700

Browse files
EgorBoEgorBo
committed
Remove more unsafe code from IPAddress parsing
1 parent 19b4d3f commit 79d0700

File tree

4 files changed

+42
-61
lines changed

4 files changed

+42
-61
lines changed

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,17 +112,6 @@ internal static bool IsValid<TChar>(ReadOnlySpan<TChar> name, out int end, bool
112112
}
113113
}
114114

115-
// IsValid wrapper for unsafe pointer use (TODO: remove when callers are migrated to Span)
116-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
117-
internal static unsafe bool IsValid<TChar>(TChar* name, int start, ref int end, bool allowIPv6, bool notImplicitFile, bool unknownScheme)
118-
where TChar : unmanaged, IBinaryInteger<TChar>
119-
{
120-
ReadOnlySpan<TChar> span = new ReadOnlySpan<TChar>(name + start, end - start);
121-
bool result = IsValid(span, out int localEnd, allowIPv6, notImplicitFile, unknownScheme);
122-
end = start + localEnd;
123-
return result;
124-
}
125-
126115
//
127116
// IsValidCanonical
128117
//

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public static bool IsValid<TChar>(ReadOnlySpan<TChar> ipSpan)
5959
throw new FormatException(SR.dns_bad_ip_address, new SocketException(SocketError.InvalidArgument));
6060
}
6161

62-
private static unsafe bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out long address)
62+
private static bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out long address)
6363
where TChar : unmanaged, IBinaryInteger<TChar>
6464
{
6565
long tmpAddr = IPv4AddressHelper.ParseNonCanonical(ipSpan, out int end, notImplicitFile: true);
@@ -77,7 +77,7 @@ private static unsafe bool TryParseIpv4<TChar>(ReadOnlySpan<TChar> ipSpan, out l
7777
return false;
7878
}
7979

80-
private static unsafe bool TryParseIPv6<TChar>(ReadOnlySpan<TChar> ipSpan, Span<ushort> numbers, int numbersLength, out uint scope)
80+
private static bool TryParseIPv6<TChar>(ReadOnlySpan<TChar> ipSpan, Span<ushort> numbers, int numbersLength, out uint scope)
8181
where TChar : unmanaged, IBinaryInteger<TChar>
8282
{
8383
Debug.Assert(typeof(TChar) == typeof(char) || typeof(TChar) == typeof(byte));

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,10 @@ private static bool IsLoopback(ReadOnlySpan<ushort> numbers)
144144

145145
// Remarks: MUST NOT be used unless all input indexes are verified and trusted.
146146
// start must be next to '[' position, or error is reported
147-
private static unsafe bool InternalIsValid(char* name, int start, ref int end, bool validateStrictAddress)
147+
private static bool InternalIsValid(ReadOnlySpan<char> name, out int end, bool validateStrictAddress)
148148
{
149+
int start = 0;
150+
end = name.Length;
149151
int sequenceCount = 0;
150152
int sequenceLength = 0;
151153
bool haveCompressor = false;
@@ -155,13 +157,13 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
155157
int lastSequence = 1;
156158

157159
// Starting with a colon character is only valid if another colon follows.
158-
if (name[start] == ':' && (start + 1 >= end || name[start + 1] != ':'))
160+
if (name[start] == ':' && (start + 1 >= name.Length || name[start + 1] != ':'))
159161
{
160162
return false;
161163
}
162164

163165
int i;
164-
for (i = start; i < end; ++i)
166+
for (i = start; i < name.Length; ++i)
165167
{
166168
if (havePrefix ? char.IsAsciiDigit(name[i]) : char.IsAsciiHexDigit(name[i]))
167169
{
@@ -185,7 +187,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
185187
while (true)
186188
{
187189
//accept anything in scopeID
188-
if (++i == end)
190+
if (++i == name.Length)
189191
{
190192
// no closing ']', fail
191193
return false;
@@ -201,7 +203,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
201203
}
202204
case ']':
203205
start = i;
204-
i = end;
206+
i = name.Length;
205207
//this will make i = end+1
206208
continue;
207209
case ':':
@@ -243,11 +245,12 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
243245
return false;
244246
}
245247

246-
i = end;
247-
if (!IPv4AddressHelper.IsValid(name, lastSequence, ref i, true, false, false))
248+
i = name.Length;
249+
if (!IPv4AddressHelper.IsValid(name.Slice(lastSequence, i - lastSequence), out int seqEnd, true, false, false))
248250
{
249251
return false;
250252
}
253+
i = lastSequence + seqEnd;
251254
// ipv4 address takes 2 slots in ipv6 address, one was just counted meeting the '.'
252255
++sequenceCount;
253256
haveIPv4Address = true;
@@ -278,7 +281,7 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
278281

279282
if (!expectingNumber && (sequenceLength <= 4) && (haveCompressor ? (sequenceCount < expectedSequenceCount) : (sequenceCount == expectedSequenceCount)))
280283
{
281-
if (i == end + 1)
284+
if (i == name.Length + 1)
282285
{
283286
// ']' was found
284287
end = start + 1;
@@ -321,9 +324,9 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
321324
// Remarks: MUST NOT be used unless all input indexes are verified and trusted.
322325
// start must be next to '[' position, or error is reported
323326

324-
internal static unsafe bool IsValid(char* name, int start, ref int end)
327+
internal static bool IsValid(ReadOnlySpan<char> name, out int end)
325328
{
326-
return InternalIsValid(name, start, ref end, false);
329+
return InternalIsValid(name, out end, false);
327330
}
328331
}
329332
}

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

Lines changed: 27 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,48 +1318,35 @@ public static UriHostNameType CheckHostName(string? name)
13181318
return UriHostNameType.Unknown;
13191319
}
13201320

1321-
int end = name.Length;
1322-
unsafe
1321+
if (name.StartsWith('[') && name.EndsWith(']'))
13231322
{
1324-
fixed (char* fixedName = name)
1323+
// we require that _entire_ name is recognized as ipv6 address
1324+
if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int endSeq) && endSeq == name.Length)
13251325
{
1326-
if (name.StartsWith('[') && name.EndsWith(']'))
1327-
{
1328-
// we require that _entire_ name is recognized as ipv6 address
1329-
if (IPv6AddressHelper.IsValid(fixedName, 1, ref end) && end == name.Length)
1330-
{
1331-
return UriHostNameType.IPv6;
1332-
}
1333-
}
1334-
1335-
end = name.Length;
1336-
if (IPv4AddressHelper.IsValid(fixedName, 0, ref end, false, false, false) && end == name.Length)
1337-
{
1338-
return UriHostNameType.IPv4;
1339-
}
1326+
return UriHostNameType.IPv6;
13401327
}
1328+
}
13411329

1342-
if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out int length) && length == name.Length)
1343-
{
1344-
return UriHostNameType.Dns;
1345-
}
1330+
if (IPv4AddressHelper.IsValid(name.AsSpan(), out int ipv4End, false, false, false) && ipv4End == name.Length)
1331+
{
1332+
return UriHostNameType.IPv4;
1333+
}
13461334

1347-
if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out length) && length == name.Length)
1348-
{
1349-
return UriHostNameType.Dns;
1350-
}
1335+
if (DomainNameHelper.IsValid(name, iri: false, notImplicitFile: false, out int length) && length == name.Length)
1336+
{
1337+
return UriHostNameType.Dns;
1338+
}
13511339

1352-
//This checks the form without []
1353-
end = name.Length + 2;
1354-
// we require that _entire_ name is recognized as ipv6 address
1355-
name = "[" + name + "]";
1356-
fixed (char* newFixedName = name)
1357-
{
1358-
if (IPv6AddressHelper.IsValid(newFixedName, 1, ref end) && end == name.Length)
1359-
{
1360-
return UriHostNameType.IPv6;
1361-
}
1362-
}
1340+
if (DomainNameHelper.IsValid(name, iri: true, notImplicitFile: false, out length) && length == name.Length)
1341+
{
1342+
return UriHostNameType.Dns;
1343+
}
1344+
1345+
//This checks the form without []
1346+
// we require that _entire_ name is recognized as ipv6 address
1347+
if (IPv6AddressHelper.IsValid(name.AsSpan(1), out int ipv6End) && ipv6End == name.Length)
1348+
{
1349+
return UriHostNameType.IPv6;
13631350
}
13641351
return UriHostNameType.Unknown;
13651352
}
@@ -3886,8 +3873,9 @@ private unsafe int CheckAuthorityHelper(char* pString, int idx, int length,
38863873
}
38873874

38883875
if (ch == '[' && syntax.InFact(UriSyntaxFlags.AllowIPv6Host) &&
3889-
IPv6AddressHelper.IsValid(pString, start + 1, ref end))
3876+
IPv6AddressHelper.IsValid(new ReadOnlySpan<char>(pString + (start + 1), end - (start + 1)), out int seqEnd))
38903877
{
3878+
end = start + 1 + seqEnd;
38913879
if (end < length && pString[end] is not (':' or '/' or '?' or '#'))
38923880
{
38933881
// 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,
39043892
}
39053893
}
39063894
else if (char.IsAsciiDigit(ch) && syntax.InFact(UriSyntaxFlags.AllowIPv4Host) &&
3907-
IPv4AddressHelper.IsValid(pString, start, ref end, false, StaticNotAny(flags, Flags.ImplicitFile), syntax.InFact(UriSyntaxFlags.V1_UnknownUri)))
3895+
IPv4AddressHelper.IsValid(new ReadOnlySpan<char>(pString + start, end - start), out int endSeq, false, StaticNotAny(flags, Flags.ImplicitFile), syntax.InFact(UriSyntaxFlags.V1_UnknownUri)))
39083896
{
3897+
end = start + endSeq;
39093898
flags |= Flags.IPv4HostType;
39103899

39113900
if (hasUnicode)

0 commit comments

Comments
 (0)