Skip to content

Commit

Permalink
Mitigate regressions in IPAddress
Browse files Browse the repository at this point in the history 
Commit migrated from dotnet/corefx@a13c1dd
  • Loading branch information
@stephentoub
stephentoub committed Aug 21, 2017
1 parent 3396e90 commit 0f62ec5
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 116 deletions.
123 changes: 45 additions & 78 deletions src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System.Diagnostics;
using System.Net.Sockets;
using System.Runtime.CompilerServices;

namespace System.Net
{
Expand Down Expand Up @@ -122,7 +123,7 @@ public IPAddress(long newAddress)
/// </para>
/// </devdoc>
public IPAddress(byte[] address, long scopeid) :
this(new ReadOnlySpan<byte>(address ?? throw new ArgumentNullException(nameof(address))), scopeid)
this(new ReadOnlySpan<byte>(address ?? ThrowAddressNullException()), scopeid)
{
}

Expand Down Expand Up @@ -150,27 +151,6 @@ public IPAddress(ReadOnlySpan<byte> address, long scopeid)
PrivateScopeId = (uint)scopeid;
}

internal unsafe IPAddress(byte* address, int addressLength, long scopeid)
{
Debug.Assert(address != null);
Debug.Assert(addressLength == IPAddressParserStatics.IPv6AddressBytes);

// Consider: Since scope is only valid for link-local and site-local
// addresses we could implement some more robust checking here
if (scopeid < 0 || scopeid > 0x00000000FFFFFFFF)
{
throw new ArgumentOutOfRangeException(nameof(scopeid));
}

_numbers = new ushort[NumberOfLabels];
for (int i = 0; i < NumberOfLabels; i++)
{
_numbers[i] = (ushort)(address[i * 2] * 256 + address[i * 2 + 1]);
}

PrivateScopeId = (uint)scopeid;
}

internal unsafe IPAddress(ushort* numbers, int numbersLength, uint scopeid)
{
Debug.Assert(numbers != null);
Expand Down Expand Up @@ -201,22 +181,17 @@ private IPAddress(ushort[] numbers, uint scopeid)
/// </para>
/// </devdoc>
public IPAddress(byte[] address) :
this(new ReadOnlySpan<byte>(address ?? throw new ArgumentNullException(nameof(address))))
this(new ReadOnlySpan<byte>(address ?? ThrowAddressNullException()))
{
}

public IPAddress(ReadOnlySpan<byte> address)
{
if (address.Length != IPAddressParserStatics.IPv4AddressBytes && address.Length != IPAddressParserStatics.IPv6AddressBytes)
{
throw new ArgumentException(SR.dns_bad_ip_address, nameof(address));
}

if (address.Length == IPAddressParserStatics.IPv4AddressBytes)
{
PrivateAddress = (uint)((address[3] << 24 | address[2] << 16 | address[1] << 8 | address[0]) & 0x0FFFFFFFF);
}
else
else if (address.Length == IPAddressParserStatics.IPv6AddressBytes)
{
_numbers = new ushort[NumberOfLabels];

Expand All @@ -225,27 +200,9 @@ public IPAddress(ReadOnlySpan<byte> address)
_numbers[i] = (ushort)(address[i * 2] * 256 + address[i * 2 + 1]);
}
}
}

internal unsafe IPAddress(byte* address, int addressLength)
{
Debug.Assert(address != null);
Debug.Assert(addressLength > 0);
Debug.Assert(
addressLength == IPAddressParserStatics.IPv4AddressBytes ||
addressLength == IPAddressParserStatics.IPv6AddressBytes);

if (addressLength == IPAddressParserStatics.IPv4AddressBytes)
{
PrivateAddress = (uint)((address[3] << 24 | address[2] << 16 | address[1] << 8 | address[0]) & 0x0FFFFFFFF);
}
else
{
_numbers = new ushort[NumberOfLabels];
for (int i = 0; i < NumberOfLabels; i++)
{
_numbers[i] = (ushort)(address[i * 2] * 256 + address[i * 2 + 1]);
}
throw new ArgumentException(SR.dns_bad_ip_address, nameof(address));
}
}

Expand All @@ -269,7 +226,7 @@ public static bool TryParse(string ipString, out IPAddress address)
return false;
}

address = IPAddressParser.Parse(ipString, tryParse: true);
address = IPAddressParser.Parse(ipString.AsReadOnlySpan(), tryParse: true);
return (address != null);
}

Expand All @@ -286,7 +243,7 @@ public static IPAddress Parse(string ipString)
throw new ArgumentNullException(nameof(ipString));
}

return IPAddressParser.Parse(ipString, tryParse: false);
return IPAddressParser.Parse(ipString.AsReadOnlySpan(), tryParse: false);
}

public static IPAddress Parse(ReadOnlySpan<char> ipSpan)
Expand All @@ -298,21 +255,13 @@ public bool TryWriteBytes(Span<byte> destination, out int bytesWritten)
{
if (IsIPv6)
{
Debug.Assert(_numbers != null && _numbers.Length == NumberOfLabels);

if (destination.Length < IPAddressParserStatics.IPv6AddressBytes)
{
bytesWritten = 0;
return false;
}

int j = 0;
for (int i = 0; i < NumberOfLabels; i++)
{
destination[j++] = (byte)((_numbers[i] >> 8) & 0xFF);
destination[j++] = (byte)((_numbers[i]) & 0xFF);
}

WriteIPv6Bytes(destination);
bytesWritten = IPAddressParserStatics.IPv6AddressBytes;
}
else
Expand All @@ -323,20 +272,35 @@ public bool TryWriteBytes(Span<byte> destination, out int bytesWritten)
return false;
}

uint address = PrivateAddress;
unchecked
{
destination[0] = (byte)(address);
destination[1] = (byte)(address >> 8);
destination[2] = (byte)(address >> 16);
destination[3] = (byte)(address >> 24);
}

WriteIPv4Bytes(destination);
bytesWritten = IPAddressParserStatics.IPv4AddressBytes;
}

return true;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void WriteIPv6Bytes(Span<byte> destination)
{
Debug.Assert(_numbers != null && _numbers.Length == NumberOfLabels);
int j = 0;
for (int i = 0; i < NumberOfLabels; i++)
{
destination[j++] = (byte)((_numbers[i] >> 8) & 0xFF);
destination[j++] = (byte)((_numbers[i]) & 0xFF);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void WriteIPv4Bytes(Span<byte> destination)
{
uint address = PrivateAddress;
destination[0] = (byte)(address);
destination[1] = (byte)(address >> 8);
destination[2] = (byte)(address >> 16);
destination[3] = (byte)(address >> 24);
}

/// <devdoc>
/// <para>
/// Provides a copy of the IPAddress internals as an array of bytes.
Expand All @@ -347,16 +311,16 @@ public byte[] GetAddressBytes()
if (IsIPv6)
{
Debug.Assert(_numbers != null && _numbers.Length == NumberOfLabels);
byte[] bytes = new byte[IPAddressParserStatics.IPv6AddressBytes];
WriteIPv6Bytes(bytes);
return bytes;
}
else
{
byte[] bytes = new byte[IPAddressParserStatics.IPv4AddressBytes];
WriteIPv4Bytes(bytes);
return bytes;
}

int length = IsIPv6 ? IPAddressParserStatics.IPv6AddressBytes : IPAddressParserStatics.IPv4AddressBytes;
var bytes = new byte[length];

bool result = TryWriteBytes(new Span<byte>(bytes), out int bytesWritten);

Debug.Assert(result);

return bytes;
}

public AddressFamily AddressFamily
Expand Down Expand Up @@ -476,7 +440,7 @@ public static bool IsLoopback(IPAddress address)
{
if (address == null)
{
throw new ArgumentNullException(nameof(address));
ThrowAddressNullException();
}

if (address.IsIPv6)
Expand Down Expand Up @@ -704,5 +668,8 @@ public IPAddress MapToIPv4()

return new IPAddress(address);
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static byte[] ThrowAddressNullException() => throw new ArgumentNullException("address");
}
}
54 changes: 16 additions & 38 deletions src/libraries/System.Net.Primitives/src/System/Net/IPAddressParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,43 +11,23 @@ namespace System.Net
{
internal class IPAddressParser
{
internal static IPAddress Parse(string ipString, bool tryParse)
{
if (ipString == null)
{
if (tryParse)
{
return null;
}
throw new ArgumentNullException(nameof(ipString));
}

return ParseHelper(ipString.AsReadOnlySpan(), tryParse);
}
private const int MaxIPv4StringLength = 15; // 4 numbers separated by 3 periods, with up to 3 digits per number

internal static IPAddress Parse(ReadOnlySpan<char> ipSpan, bool tryParse)
{
return ParseHelper(ipSpan, tryParse);
}

internal static unsafe IPAddress ParseHelper(ReadOnlySpan<char> ipSpan, bool tryParse)
internal static unsafe IPAddress Parse(ReadOnlySpan<char> ipSpan, bool tryParse)
{
if (ipSpan.IndexOf(':') >= 0)
{
// If the address string contains the colon character then it can only be an IPv6 address.
// This is valid because we don't support/parse a port specification at the end of an IPv4 address.
// The address is parsed as IPv6 if and only if it contains a colon. This is valid because
// we don't support/parse a port specification at the end of an IPv4 address.
ushort* numbers = stackalloc ushort[IPAddressParserStatics.IPv6AddressShorts];
if (Ipv6StringToAddress(ipSpan, numbers, IPAddressParserStatics.IPv6AddressShorts, out uint scope))
{
return new IPAddress(numbers, IPAddressParserStatics.IPv6AddressShorts, scope);
}
}
else
else if (Ipv4StringToAddress(ipSpan, out long address))
{
if (Ipv4StringToAddress(ipSpan, out long address))
{
return new IPAddress(address);
}
return new IPAddress(address);
}

if (tryParse)
Expand All @@ -60,30 +40,28 @@ internal static unsafe IPAddress ParseHelper(ReadOnlySpan<char> ipSpan, bool try

internal static unsafe string IPv4AddressToString(uint address)
{
const int MaxLength = 15;
char* addressString = stackalloc char[MaxLength];

int charsWritten = IPv4AddressToStringHelper(address, new Span<char>(addressString, MaxLength));

char* addressString = stackalloc char[MaxIPv4StringLength];
int charsWritten = IPv4AddressToStringHelper(address, addressString);
return new string(addressString, 0, charsWritten);
}

internal static unsafe bool IPv4AddressToString(uint address, Span<char> formatted, out int charsWritten)
{
const int MaxLength = 15;

if (formatted.Length < MaxLength)
if (formatted.Length < MaxIPv4StringLength)
{
charsWritten = 0;
return false;
}

charsWritten = IPv4AddressToStringHelper(address, formatted);

fixed (char* formattedPtr = &formatted.DangerousGetPinnableReference())
{
charsWritten = IPv4AddressToStringHelper(address, formattedPtr);
}

return true;
}

internal static int IPv4AddressToStringHelper(uint address, Span<char> addressString)
private static unsafe int IPv4AddressToStringHelper(uint address, char* addressString)
{
int offset = 0;

Expand Down Expand Up @@ -162,7 +140,7 @@ internal static StringBuilder IPv6AddressToStringHelper(ushort[] address, uint s
return buffer;
}

private static void FormatIPv4AddressNumber(int number, Span<char> addressString, ref int offset)
private static unsafe void FormatIPv4AddressNumber(int number, char* addressString, ref int offset)
{
// Math.DivRem has no overload for byte, assert here for safety
Debug.Assert(number < 256);
Expand Down

0 comments on commit 0f62ec5

Please sign in to comment.