Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 4006e52

Browse files
author
David Shulman
committed
Merged PR 171500: Fix IPv6Address parsing
Fix IPv6Address parsing
2 parents 876e51e + 9e043b3 commit 4006e52

File tree

6 files changed

+75
-18
lines changed

6 files changed

+75
-18
lines changed

‎src/System.Net.Primitives/src/System/Net/IPAddress.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,10 @@ public IPAddress(ReadOnlySpan<byte> address, long scopeid)
157157
PrivateScopeId = (uint)scopeid;
158158
}
159159

160-
internal unsafe IPAddress(ushort* numbers, int numbersLength, uint scopeid)
160+
internal IPAddress(ReadOnlySpan<ushort> numbers, uint scopeid)
161161
{
162162
Debug.Assert(numbers != null);
163-
Debug.Assert(numbersLength == NumberOfLabels);
163+
Debug.Assert(numbers.Length == NumberOfLabels);
164164

165165
var arr = new ushort[NumberOfLabels];
166166
for (int i = 0; i < arr.Length; i++)

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,17 @@ internal class IPAddressParser
1414
{
1515
private const int MaxIPv4StringLength = 15; // 4 numbers separated by 3 periods, with up to 3 digits per number
1616

17-
internal static unsafe IPAddress Parse(ReadOnlySpan<char> ipSpan, bool tryParse)
17+
internal static IPAddress Parse(ReadOnlySpan<char> ipSpan, bool tryParse)
1818
{
1919
if (ipSpan.IndexOf(':') >= 0)
2020
{
2121
// The address is parsed as IPv6 if and only if it contains a colon. This is valid because
2222
// we don't support/parse a port specification at the end of an IPv4 address.
23-
ushort* numbers = stackalloc ushort[IPAddressParserStatics.IPv6AddressShorts];
24-
new Span<ushort>(numbers, IPAddressParserStatics.IPv6AddressShorts).Clear();
23+
Span<ushort> numbers = stackalloc ushort[IPAddressParserStatics.IPv6AddressShorts];
24+
numbers.Clear();
2525
if (Ipv6StringToAddress(ipSpan, numbers, IPAddressParserStatics.IPv6AddressShorts, out uint scope))
2626
{
27-
return new IPAddress(numbers, IPAddressParserStatics.IPv6AddressShorts, scope);
27+
return new IPAddress(numbers, scope);
2828
}
2929
}
3030
else if (Ipv4StringToAddress(ipSpan, out long address))
@@ -186,7 +186,7 @@ public static unsafe bool Ipv4StringToAddress(ReadOnlySpan<char> ipSpan, out lon
186186
}
187187
}
188188

189-
public static unsafe bool Ipv6StringToAddress(ReadOnlySpan<char> ipSpan, ushort* numbers, int numbersLength, out uint scope)
189+
public static unsafe bool Ipv6StringToAddress(ReadOnlySpan<char> ipSpan, Span<ushort> numbers, int numbersLength, out uint scope)
190190
{
191191
Debug.Assert(numbers != null);
192192
Debug.Assert(numbersLength >= IPAddressParserStatics.IPv6AddressShorts);

‎src/System.Net.Primitives/src/System/Net/IPv6AddressHelper.cs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,17 @@ internal unsafe static bool IsValidStrict(char* name, int start, ref int end)
113113
{
114114
start++;
115115
needsClosingBracket = true;
116+
117+
// IsValidStrict() is only called if there is a ':' in the name string, i.e.
118+
// it is a possible IPv6 address. So, if the string starts with a '[' and
119+
// the pointer is advanced here there are still more characters to parse.
120+
Debug.Assert(start < end);
121+
}
122+
123+
// Starting with a colon character is only valid if another colon follows.
124+
if (name[start] == ':' && (start + 1 >= end || name[start + 1] != ':'))
125+
{
126+
return false;
116127
}
117128

118129
int i;
@@ -284,7 +295,7 @@ internal unsafe static bool IsValidStrict(char* name, int start, ref int end)
284295
// Nothing
285296
//
286297

287-
internal static unsafe void Parse(ReadOnlySpan<char> address, ushort* numbers, int start, ref string scopeId)
298+
internal static void Parse(ReadOnlySpan<char> address, Span<ushort> numbers, int start, ref string scopeId)
288299
{
289300
int number = 0;
290301
int index = 0;

‎src/System.Net.Primitives/tests/FunctionalTests/IPAddressParsing.cs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,10 @@ public void TryParseIPv6_ValidAddress_RoundtripMatchesExpected(string address, s
346346

347347
public static IEnumerable<object[]> InvalidIpv6Addresses()
348348
{
349+
yield return new object[] { "" }; // malformed
350+
yield return new object[] { "[" }; // malformed
351+
yield return new object[] { "[]" }; // malformed
352+
yield return new object[] { "[:]" }; // malformed
349353
yield return new object[] { ":::4df" };
350354
yield return new object[] { "4df:::" };
351355
yield return new object[] { "0:::4df" };
@@ -368,6 +372,24 @@ public static IEnumerable<object[]> InvalidIpv6Addresses()
368372
yield return new object[] { "Fe08::1]]" }; // two trailing brackets
369373
yield return new object[] { "[Fe08::1]]" }; // one leading and two trailing brackets
370374
yield return new object[] { ":1" }; // leading single colon
375+
yield return new object[] { ":1:2" }; // leading single colon
376+
yield return new object[] { ":1:2:3" }; // leading single colon
377+
yield return new object[] { ":1:2:3:4" }; // leading single colon
378+
yield return new object[] { ":1:2:3:4:5" }; // leading single colon
379+
yield return new object[] { ":1:2:3:4:5:6" }; // leading single colon
380+
yield return new object[] { ":1:2:3:4:5:6:7" }; // leading single colon
381+
yield return new object[] { ":1:2:3:4:5:6:7:8" }; // leading single colon
382+
yield return new object[] { ":1:2:3:4:5:6:7:8:9" }; // leading single colon
383+
yield return new object[] { "::1:2:3:4:5:6:7:8" }; // compressor with too many number groups
384+
yield return new object[] { "1::2:3:4:5:6:7:8" }; // compressor with too many number groups
385+
yield return new object[] { "1:2::3:4:5:6:7:8" }; // compressor with too many number groups
386+
yield return new object[] { "1:2:3::4:5:6:7:8" }; // compressor with too many number groups
387+
yield return new object[] { "1:2:3:4::5:6:7:8" }; // compressor with too many number groups
388+
yield return new object[] { "1:2:3:4:5::6:7:8" }; // compressor with too many number groups
389+
yield return new object[] { "1:2:3:4:5:6::7:8" }; // compressor with too many number groups
390+
yield return new object[] { "1:2:3:4:5:6:7::8" }; // compressor with too many number groups
391+
yield return new object[] { "1:2:3:4:5:6:7:8::" }; // compressor with too many number groups
392+
yield return new object[] { "::1:2:3:4:5:6:7:8:9" }; // compressor with too many number groups
371393
yield return new object[] { "1:" }; // trailing single colon
372394
yield return new object[] { " ::1" }; // leading whitespace
373395
yield return new object[] { "::1 " }; // trailing whitespace

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections.Generic;
6-
using System.Text;
6+
using System.Diagnostics;
77
using System.Globalization;
8+
using System.Text;
89

910
namespace System
1011
{
@@ -26,16 +27,14 @@ internal static string ParseCanonicalName(string str, int start, ref bool isLoop
2627
{
2728
unsafe
2829
{
29-
ushort* numbers = stackalloc ushort[NumberOfLabels];
30-
// optimized zeroing of 8 shorts = 2 longs
31-
((long*)numbers)[0] = 0L;
32-
((long*)numbers)[1] = 0L;
30+
Span<ushort> numbers = stackalloc ushort[NumberOfLabels];
31+
numbers.Clear();
3332
isLoopback = Parse(str, numbers, start, ref scopeId);
3433
return '[' + CreateCanonicalName(numbers) + ']';
3534
}
3635
}
3736

38-
internal static unsafe string CreateCanonicalName(ushort* numbers)
37+
private static string CreateCanonicalName(ReadOnlySpan<ushort> numbers)
3938
{
4039
// RFC 5952 Sections 4 & 5 - Compressed, lower case, with possible embedded IPv4 addresses.
4140

@@ -83,7 +82,7 @@ internal static unsafe string CreateCanonicalName(ushort* numbers)
8382
// Longest consecutive sequence of zero segments, minimum 2.
8483
// On equal, first sequence wins.
8584
// <-1, -1> for no compression.
86-
private static unsafe KeyValuePair<int, int> FindCompressionRange(ushort* numbers)
85+
private static KeyValuePair<int, int> FindCompressionRange(ReadOnlySpan<ushort> numbers)
8786
{
8887
int longestSequenceLength = 0;
8988
int longestSequenceStart = -1;
@@ -117,7 +116,7 @@ private static unsafe KeyValuePair<int, int> FindCompressionRange(ushort* number
117116

118117
// Returns true if the IPv6 address should be formated with an embedded IPv4 address:
119118
// ::192.168.1.1
120-
private static unsafe bool ShouldHaveIpv4Embedded(ushort* numbers)
119+
private static bool ShouldHaveIpv4Embedded(ReadOnlySpan<ushort> numbers)
121120
{
122121
// 0:0 : 0:0 : x:x : x.x.x.x
123122
if (numbers[0] == 0 && numbers[1] == 0 && numbers[2] == 0 && numbers[3] == 0 && numbers[6] != 0)
@@ -185,6 +184,12 @@ private static unsafe bool InternalIsValid(char* name, int start, ref int end, b
185184
bool expectingNumber = true;
186185
int lastSequence = 1;
187186

187+
// Starting with a colon character is only valid if another colon follows.
188+
if (name[start] == ':' && (start + 1 >= end || name[start + 1] != ':'))
189+
{
190+
return false;
191+
}
192+
188193
int i;
189194
for (i = start; i < end; ++i)
190195
{
@@ -379,7 +384,7 @@ internal static unsafe bool IsValid(char* name, int start, ref int end)
379384
// Nothing
380385
//
381386

382-
internal static unsafe bool Parse(string address, ushort* numbers, int start, ref string scopeId)
387+
private static bool Parse(string address, Span<ushort> numbers, int start, ref string scopeId)
383388
{
384389
int number = 0;
385390
int index = 0;

‎src/System.Private.Uri/tests/FunctionalTests/UriIpHostTest.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,26 @@ public void UriIPv6Host_EmbeddedIPv4_Success(string address, string expected)
313313
[InlineData("")]
314314
[InlineData(" ")]
315315
[InlineData("1")]
316-
[InlineData(":1")]
316+
[InlineData(":")] // leading single colon
317+
[InlineData(":1")] // leading single colon
318+
[InlineData(":1:2")] // leading single colon
319+
[InlineData(":1:2:3")] // leading single colon
320+
[InlineData(":1:2:3:4")] // leading single colon
321+
[InlineData(":1:2:3:4:5")] // leading single colon
322+
[InlineData(":1:2:3:4:5:6")] // leading single colon
323+
[InlineData(":1:2:3:4:5:6:7")] // leading single colon
324+
[InlineData(":1:2:3:4:5:6:7:8")] // leading single colon
325+
[InlineData(":1:2:3:4:5:6:7:8:9")] // leading single colon
326+
[InlineData("::1:2:3:4:5:6:7:8")] // compressor with too many number groups
327+
[InlineData("1::2:3:4:5:6:7:8")] // compressor with too many number groups
328+
[InlineData("1:2::3:4:5:6:7:8")] // compressor with too many number groups
329+
[InlineData("1:2:3::4:5:6:7:8")] // compressor with too many number groups
330+
[InlineData("1:2:3:4::5:6:7:8")] // compressor with too many number groups
331+
[InlineData("1:2:3:4:5::6:7:8")] // compressor with too many number groups
332+
[InlineData("1:2:3:4:5:6::7:8")] // compressor with too many number groups
333+
[InlineData("1:2:3:4:5:6:7::8")] // compressor with too many number groups
334+
[InlineData("1:2:3:4:5:6:7:8::")] // compressor with too many number groups
335+
[InlineData("::1:2:3:4:5:6:7:8:9")] // compressor with too many number groups
317336
[InlineData("1:")]
318337
[InlineData("::1 ")]
319338
[InlineData(" ::1")]

0 commit comments

Comments
 (0)