From 98a0c07b39be51bf817e3b8ab7cac532e51afd66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Sim=C3=B5es?= Date: Tue, 4 Jan 2022 16:02:17 +0000 Subject: [PATCH] Various fixes and improvements - Add missing Equals and GetHashCode on several classes. - Rework GetHostEntry to properly parse data from native call. - Apply back fix from #217 - Fix several comments for documentation. - Several fixes in code style. - Improvements from analysers. - Disable running Unit Tests. --- azure-pipelines.yml | 2 +- nanoFramework.System.Net/DNS.cs | 28 +++++--- nanoFramework.System.Net/IPAddress.cs | 83 +++++++++++++++++------ nanoFramework.System.Net/IPEndPoint.cs | 27 +++++--- nanoFramework.System.Net/SocketAddress.cs | 64 +++++++++++++++-- 5 files changed, 156 insertions(+), 48 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 05ab576..c145ea8 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -41,7 +41,7 @@ jobs: - template: azure-pipelines-templates/class-lib-build.yml@templates parameters: sonarCloudProject: 'nanoframework_lib-nanoFramework.System.Net' - runUnitTests: true + runUnitTests: false unitTestRunsettings: '$(System.DefaultWorkingDirectory)\Tests\SocketTests\nano.runsettings' ############################## diff --git a/nanoFramework.System.Net/DNS.cs b/nanoFramework.System.Net/DNS.cs index f1dfc05..0941650 100644 --- a/nanoFramework.System.Net/DNS.cs +++ b/nanoFramework.System.Net/DNS.cs @@ -26,20 +26,28 @@ public static class Dns /// public static IPHostEntry GetHostEntry(string hostNameOrAddress) { + NativeSocket.getaddrinfo(hostNameOrAddress, out string canonicalName, out byte[][] addresses); - //Do we need to try to pase this as an Address???? - string canonicalName; - byte[][] addresses; + int addressesCount = addresses.Length; + IPAddress[] ipAddresses = new IPAddress[addressesCount]; + IPHostEntry ipHostEntry = new(); - NativeSocket.getaddrinfo(hostNameOrAddress, out canonicalName, out addresses); + for (int i = 0; i < addressesCount; i++) + { + byte[] address = addresses[i]; - int cAddresses = addresses.Length; - IPAddress[] ipAddresses = new IPAddress[cAddresses]; - IPHostEntry ipHostEntry = new IPHostEntry(); + AddressFamily family = (AddressFamily)((address[1] << 8) | address[0]); - for (int i = 0; i < cAddresses; i++) - { - ipAddresses[i] = new IPAddress(addresses[i]); + if (family == AddressFamily.InterNetwork) + { + uint ipAddr = (uint)((address[7] << 24) | (address[6] << 16) | (address[5] << 8) | (address[4])); + + ipAddresses[i] = new IPAddress(ipAddr); + } + else + { + throw new NotImplementedException(); + } } ipHostEntry.hostName = canonicalName; diff --git a/nanoFramework.System.Net/IPAddress.cs b/nanoFramework.System.Net/IPAddress.cs index c65df3d..ff3b20f 100644 --- a/nanoFramework.System.Net/IPAddress.cs +++ b/nanoFramework.System.Net/IPAddress.cs @@ -11,29 +11,28 @@ namespace System.Net { /// - /// Provides an internet protocol (IP) address. + /// Initializes a new instance of the class. /// [Serializable] public class IPAddress { /// - /// Provides an IP address that indicates that the server must listen for client activity on all network interfaces. - /// This field is read-only. + /// Provides an IP address that indicates that the server must listen for client activity on all network interfaces. This field is read-only. /// - public static readonly IPAddress Any = new IPAddress(0x0000000000000000); - + public static readonly IPAddress Any = new(0x0000000000000000); + /// /// Provides the IP loopback address. This field is read-only. /// - public static readonly IPAddress Loopback = new IPAddress(0x000000000100007F); + public static readonly IPAddress Loopback = new(0x000000000100007F); - internal long Address; + internal readonly long Address; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] - private AddressFamily _family = AddressFamily.InterNetwork; + private readonly AddressFamily _family = AddressFamily.InterNetwork; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] - private ushort[] _numbers = new ushort[NumberOfLabels]; + private readonly ushort[] _numbers = new ushort[NumberOfLabels]; internal const int IPv4AddressBytes = 4; internal const int IPv6AddressBytes = 16; @@ -55,12 +54,22 @@ public AddressFamily AddressFamily /// /// Initializes a new instance of the class with the address specified as an Int64. /// - /// + /// The long value of the IP address. For example, the value 0x2414188f in big-endian format would be the IP address "143.24.20.36". + /// + /// < 0 or + /// > 0x00000000FFFFFFFF + /// + /// + /// The IPAddress instance is created with the property set to . + /// The value is assumed to be in network byte order. + /// public IPAddress(long newAddress) { if (newAddress < 0 || newAddress > 0x00000000FFFFFFFF) { +#pragma warning disable S3928 // Parameter names used into ArgumentException constructors should match an existing one throw new ArgumentOutOfRangeException(); +#pragma warning restore S3928 // OK to throw this here } Address = newAddress; @@ -73,15 +82,29 @@ public IPAddress(long newAddress) /// Initializes a new instance of the class with the address specified as a Byte array. /// /// + /// is . + /// contains a bad IP address. + /// + /// The IPAddress is created with the property set to . + /// If the length of is 4, (Byte[]) constructs an IPv4 address; otherwise, an IPv6 address with a scope of 0 is constructed. + /// The array is assumed to be in network byte order with the most significant byte first in index position 0. + /// public IPAddress(byte[] address) { - if (address[0] == (byte)AddressFamily.InterNetwork) + if (address == null) + { +#pragma warning disable S3928 // Parameter names used into ArgumentException constructors should match an existing one + throw new ArgumentNullException(); +#pragma warning restore S3928 // OK to throw this here + } + + if (address.Length == IPv4AddressBytes) { _family = AddressFamily.InterNetwork; - // need to offset address by 4 (1st are family, 2nd are port - Address = ((address[3 + 4] << 24 | address[2 + 4] << 16 | address[1 + 4] << 8 | address[0 + 4]) & 0x0FFFFFFFF); + + Address = (address[3] << 24 | address[2] << 16 | address[1] << 8 | address[0]) & 0x0FFFFFFFF; } - else if (address[0] == (byte)AddressFamily.InterNetworkV6) + else if (address.Length == IPv6AddressBytes) { _family = AddressFamily.InterNetworkV6; @@ -93,7 +116,9 @@ public IPAddress(byte[] address) else { // unsupported address family - throw new NotSupportedException(); +#pragma warning disable S3928 // Parameter names used into ArgumentException constructors should match an existing one + throw new ArgumentException(); +#pragma warning restore S3928 // OK to throw this here } } @@ -140,12 +165,10 @@ public static IPAddress Parse(string ipString) /// /// Converts an Internet address to its standard notation. /// - /// A string that contains the IP address in either IPv4 dotted-quad or - /// in IPv6 colon-hexadecimal notation. + /// A string that contains the IP address in either IPv4 dotted-quad or in IPv6 colon-hexadecimal notation. /// /// - /// The ToString method converts the IP address that is stored in the Address property to either IPv4 dotted-quad or - /// IPv6 colon-hexadecimal notation. + /// The method converts the IP address that is stored in the property to either IPv4 dotted-quad or IPv6 colon-hexadecimal notation. /// public override string ToString() { @@ -161,6 +184,7 @@ public static IPAddress GetDefaultLocalAddress() NetworkInterface[] interfaces = NetworkInterface.GetAllNetworkInterfaces(); int cnt = interfaces.Length; + for (int i = 0; i < cnt; i++) { NetworkInterface ni = interfaces[i]; @@ -180,6 +204,23 @@ public static IPAddress GetDefaultLocalAddress() return Any; } + /// + public override int GetHashCode() + { + // For IPv6 addresses, we cannot simply return the integer + // representation as the hashcode. Instead, we calculate + // the hashcode from the string representation of the address. + if (_family == AddressFamily.InterNetworkV6) + { + return ToString().GetHashCode(); + } + else + { + // For IPv4 addresses, we can simply use the integer representation. + return unchecked((int)Address); + } + } + // For security, we need to be able to take an IPAddress and make a copy that's immutable and not derived. internal IPAddress Snapshot() { @@ -188,8 +229,8 @@ internal IPAddress Snapshot() case AddressFamily.InterNetwork: return new IPAddress(Address); - //case AddressFamily.InterNetworkV6: - // return new IPAddress(m_Numbers, (uint)m_ScopeId); + //case AddressFamily.InterNetworkV6: + // return new IPAddress(m_Numbers, (uint)m_ScopeId); } throw new NotSupportedException(); diff --git a/nanoFramework.System.Net/IPEndPoint.cs b/nanoFramework.System.Net/IPEndPoint.cs index 1b57252..f327646 100644 --- a/nanoFramework.System.Net/IPEndPoint.cs +++ b/nanoFramework.System.Net/IPEndPoint.cs @@ -1,4 +1,3 @@ -// namespace System.Net // // Copyright (c) .NET Foundation and Contributors // Portions Copyright (c) Microsoft Corporation. All rights reserved. @@ -19,17 +18,17 @@ public class IPEndPoint : EndPoint /// Specifies the minimum value that can be assigned to the Port property. This field is read-only. /// public const int MinPort = 0x00000000; - + /// /// Specifies the maximum value that can be assigned to the Port property. The MaxPort value is set to 0x0000FFFF. This field is read-only. /// public const int MaxPort = 0x0000FFFF; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] - private IPAddress _address; + private readonly IPAddress _address; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] - private int _port; + private readonly int _port; /// /// Initializes a new instance of the class with the specified address and port number. @@ -96,9 +95,12 @@ public int Port public override SocketAddress Serialize() { // create a new SocketAddress - // - SocketAddress socketAddress = new SocketAddress(AddressFamily.InterNetwork, SocketAddress.IPv4AddressSize); + SocketAddress socketAddress = new( + AddressFamily.InterNetwork, + SocketAddress.IPv4AddressSize); + byte[] buffer = socketAddress.m_Buffer; + // // populate it // @@ -127,10 +129,10 @@ public override EndPoint Create(SocketAddress socketAddress) // Debug.Assert(socketAddress.Family == AddressFamily.InterNetwork); - int port = (int)( + int port = (buf[2] << 8 & 0xFF00) | (buf[3]) - ); + ; long address = (long)( (buf[4] & 0x000000FF) | @@ -158,8 +160,7 @@ public override string ToString() /// true if the objects are equal. public override bool Equals(object obj) { - IPEndPoint ep = obj as IPEndPoint; - if (ep == null) + if (obj is not IPEndPoint ep) { return false; } @@ -167,6 +168,12 @@ public override bool Equals(object obj) return ep._address.Equals(_address) && ep._port == _port; } + /// + public override int GetHashCode() + { + return _address.GetHashCode() ^ _port; + } + // For security, we need to be able to take an IPEndPoint and make a copy that's immutable and not derived. internal IPEndPoint Snapshot() { diff --git a/nanoFramework.System.Net/SocketAddress.cs b/nanoFramework.System.Net/SocketAddress.cs index ac79d94..aceb6a3 100644 --- a/nanoFramework.System.Net/SocketAddress.cs +++ b/nanoFramework.System.Net/SocketAddress.cs @@ -17,7 +17,7 @@ public class SocketAddress internal const int IPv4AddressSize = 16; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] - internal byte[] m_Buffer; + internal readonly byte[] m_Buffer; [Diagnostics.DebuggerBrowsable(Diagnostics.DebuggerBrowsableState.Never)] internal long _address; @@ -40,7 +40,7 @@ internal SocketAddress(byte[] address) } internal SocketAddress(IPAddress ipAddress) - :this(ipAddress.AddressFamily, + : this(ipAddress.AddressFamily, (ipAddress.AddressFamily == AddressFamily.InterNetwork) ? IPv4AddressSize : IPv6AddressSize) { @@ -90,11 +90,11 @@ internal SocketAddress(IPAddress ipAddress) /// public SocketAddress(AddressFamily family, int size) { - // Debug.Assert(size > 2); + // Debug.Assert(size > 2); m_Buffer = new byte[size]; //(size / IntPtr.Size + 2) * IntPtr.Size];//sizeof DWORD - m_Buffer[0] = unchecked((byte)((int)family )); + m_Buffer[0] = unchecked((byte)((int)family)); m_Buffer[1] = unchecked((byte)((int)family >> 8)); } @@ -129,10 +129,62 @@ public byte this[int offset] internal IPEndPoint GetIPEndPoint() { IPAddress address = GetIPAddress(); - int port = (int)((m_Buffer[2] << 8 & 0xFF00) | (m_Buffer[3])); + int port = ((m_Buffer[2] << 8) & 0xFF00) | (m_Buffer[3]); return new IPEndPoint(address, port); } + /// + public override bool Equals(object obj) + { + if (obj is not SocketAddress castedComparand || Size != castedComparand.Size) + { + return false; + } + + for (int i = 0; i < Size; i++) + { + if (this[i] != castedComparand[i]) + { + return false; + } + } + + return true; + } + + /// + public override int GetHashCode() + { + int i; + int hash = 0; + int size = Size & ~3; + + for (i = 0; i < size; i += 4) + { + hash ^= m_Buffer[i] + | (m_Buffer[i + 1] << 8) + | (m_Buffer[i + 2] << 16) + | (m_Buffer[i + 3] << 24); + } + + if ((Size & 3) != 0) + { + + int remnant = 0; + int shift = 0; + + for (; i < Size; ++i) + { + remnant |= m_Buffer[i] << shift; + shift += 8; + } + + hash ^= remnant; + } + + return hash; + } + internal IPAddress GetIPAddress() { //if (Family == AddressFamily.InterNetworkV6) @@ -153,7 +205,7 @@ internal IPAddress GetIPAddress() //} //else if (Family == AddressFamily.InterNetwork) { - long address = (long)( + long address = ( (m_Buffer[4] & 0x000000FF) | (m_Buffer[5] << 8 & 0x0000FF00) | (m_Buffer[6] << 16 & 0x00FF0000) |