Enhance STUNUri and IPSocket with modern C# practices (#1409)

Updated SIPSorcery.csproj to include new package references for `System.Memory` and `Microsoft.Bcl.HashCode` for .NET Framework 4.6.2 and .NET Standard 2.0.

Refactored `STUNUri` class to implement `IEquatable<STUNUri>`, changed public fields to read-only properties, and overloaded the constructor for better initialization options. Improved `ParseSTUNUri` method with a robust `TryParse` implementation and optimized parsing logic.

Simplified `Equals` and `GetHashCode` methods for better performance. Enhanced `TryParseIPEndPoint` in `IPSocket.cs` to accept `ReadOnlySpan<char>` for improved efficiency and memory usage.

Overall, these changes promote immutability, performance, and maintainability in line with modern C# standards.

Author: paulomorgado

src/SIPSorcery.csproj

@@ -29,6 +29,11 @@
     <!-- The packages below are transitive references included to overcome vulnerabilities in a top level package. -->
     <PackageReference Include="System.Net.Security" Version="4.3.2" /> <!-- Vuln version referenced by ystem.Net.WebSockets.Client. -->
   </ItemGroup>
+  
+  <ItemGroup Condition="'$(TargetFramework)' == 'net462' OR '$(TargetFramework)' == 'netstandard2.0'">
+    <PackageReference Include="System.Memory" Version="4.6.3" />
+    <PackageReference Include="Microsoft.Bcl.HashCode" Version="6.0.0" />
+  </ItemGroup>
 
   <PropertyGroup>
     <TargetFrameworks>netstandard2.0;netstandard2.1;netcoreapp3.1;net462;net5.0;net6.0;net8.0</TargetFrameworks>

src/net/STUN/STUNUri.cs

@@ -17,6 +17,7 @@
 //-----------------------------------------------------------------------------
 
 using System;
+using System.ComponentModel;
 using System.Net.Sockets;
 using SIPSorcery.Sys;
 
@@ -79,29 +80,29 @@ public enum STUNProtocolsEnum
         dtls = 4,
     }
 
-    public class STUNUri
+    public sealed class STUNUri : IEquatable<STUNUri>
     {
         public const string SCHEME_TRANSPORT_TCP = "transport=tcp";
         public const string SCHEME_TRANSPORT_TLS = "transport=tls";
 
-        public static readonly string[] SCHEME_TRANSPORT_SEPARATOR = { "?transport=" };
+        public static readonly string SCHEME_TRANSPORT_SEPARATOR = "?transport=";
         public const char SCHEME_ADDR_SEPARATOR = ':';
         public const int SCHEME_MAX_LENGTH = 5;
 
         public const STUNSchemesEnum DefaultSTUNScheme = STUNSchemesEnum.stun;
 
-        public STUNProtocolsEnum Transport = STUNProtocolsEnum.udp;
-        public STUNSchemesEnum Scheme = DefaultSTUNScheme;
+        public STUNProtocolsEnum Transport { get; } = STUNProtocolsEnum.udp;
+        public STUNSchemesEnum Scheme { get; } = DefaultSTUNScheme;
 
-        public string Host;
-        public int Port;
+        public string Host { get; }
+        public int Port { get; }
 
         /// <summary>
         /// If the port is specified in a URI it affects the way a DNS lookup occurs.
         /// An explicit port means to lookup the A or AAAA record directly without
         /// checking for SRV records.
         /// </summary>
-        public bool ExplicitPort;
+        public bool ExplicitPort { get; }
 
         /// <summary>
         /// The network protocol for this URI type.
@@ -124,107 +125,126 @@ public ProtocolType Protocol
         private STUNUri()
         { }
 
-        public STUNUri(STUNSchemesEnum scheme, string host, int port = STUNConstants.DEFAULT_STUN_PORT)
+        [EditorBrowsable(EditorBrowsableState.Advanced)]
+        public STUNUri(STUNSchemesEnum scheme, string host, int port)
         {
             Scheme = scheme;
             Host = host;
             Port = port;
         }
 
-        public static STUNUri ParseSTUNUri(string uri)
+        public STUNUri(STUNSchemesEnum scheme, string host, int port = STUNConstants.DEFAULT_STUN_PORT, STUNProtocolsEnum transport = STUNProtocolsEnum.udp, bool explicitPort = false)
         {
-            STUNUri stunUri = new STUNUri();
+            Scheme = scheme;
+            Host = host;
+            Port = port;
+            Transport = transport;
+            ExplicitPort = explicitPort;
+        }
 
-            if (String.IsNullOrEmpty(uri))
+        public static STUNUri ParseSTUNUri(string uriStr)
+        {
+            if (!TryParse(uriStr, out var uri))
             {
                 throw new ApplicationException("A STUN URI cannot be parsed from an empty string.");
             }
-            else
+
+            return uri;
+        }
+
+        public static bool TryParse(string uriStr, out STUNUri uri)
+        {
+            uri = null;
+
+            if (string.IsNullOrEmpty(uriStr))
             {
-                //Split uri to include support to transport detection
-                var splitUri = uri.Split(SCHEME_TRANSPORT_SEPARATOR, StringSplitOptions.RemoveEmptyEntries);
-                if(splitUri.Length > 1)
+                return false;
+            }
+
+            ReadOnlySpan<char> uriSpan = uriStr.AsSpan();
+            STUNProtocolsEnum transport = STUNProtocolsEnum.udp;
+
+            // Handle transport protocol
+            int transportIndex = uriSpan.IndexOf('?');
+            if (transportIndex >= 0 && uriSpan.Slice(transportIndex, SCHEME_TRANSPORT_SEPARATOR.Length).SequenceEqual(SCHEME_TRANSPORT_SEPARATOR.AsSpan()))
+            {
+                var protocolSpan = uriSpan.Slice(transportIndex + SCHEME_TRANSPORT_SEPARATOR.Length).Trim();
+#if NET6_0_OR_GREATER
+                if (!protocolSpan.IsEmpty && !Enum.TryParse(protocolSpan, true, out transport))
+#else
+                if (!protocolSpan.IsEmpty && !Enum.TryParse(protocolSpan.ToString(), true, out transport))
+#endif
                 {
-                    uri = splitUri[0];
-                    var protocolStr = splitUri[1].Trim();
-                    if (string.IsNullOrEmpty(protocolStr) || !Enum.TryParse<STUNProtocolsEnum>(protocolStr, true, out stunUri.Transport))
-                    {
-                        stunUri.Transport = STUNProtocolsEnum.udp;
-                    }
+                    transport = STUNProtocolsEnum.udp;
                 }
+                uriSpan = uriSpan.Slice(0, transportIndex);
+            }
 
-                uri = uri.Trim();
+            uriSpan = uriSpan.Trim();
+            var scheme = DefaultSTUNScheme;
 
-                // If the scheme is included it needs to be within the first 5 characters.
-                if (uri.Length > SCHEME_MAX_LENGTH + 2)
-                {
-                    string schemeStr = uri.Substring(0, SCHEME_MAX_LENGTH + 1);
-                    int colonPosn = schemeStr.IndexOf(SCHEME_ADDR_SEPARATOR);
+            // Handle scheme parsing
+            if (uriSpan.Length > SCHEME_MAX_LENGTH + 2)
+            {
+                ReadOnlySpan<char> schemeSpan = uriSpan.Slice(0, SCHEME_MAX_LENGTH + 1);
+                int colonPosn = schemeSpan.IndexOf(SCHEME_ADDR_SEPARATOR);
 
-                    if (colonPosn == -1)
-                    {
-                        // No scheme has been specified, use default.
-                        stunUri.Scheme = DefaultSTUNScheme;
-                    }
-                    else
+                if (colonPosn >= 0)
+                {
+#if NET6_0_OR_GREATER
+                    if (!Enum.TryParse(schemeSpan.Slice(0, colonPosn), true, out scheme))
+#else
+                    if (!Enum.TryParse(schemeSpan.Slice(0, colonPosn).ToString(), true, out scheme))
+#endif
                     {
-                        if (!Enum.TryParse<STUNSchemesEnum>(schemeStr.Substring(0, colonPosn), true, out stunUri.Scheme))
-                        {
-                            stunUri.Scheme = DefaultSTUNScheme;
-                        }
-
-                        uri = uri.Substring(colonPosn + 1);
+                        scheme = DefaultSTUNScheme;
                     }
+                    uriSpan = uriSpan.Slice(colonPosn + 1);
                 }
+            }
 
-                if (uri.IndexOf(':') != -1)
-                {
-                    stunUri.ExplicitPort = true;
+            var explicitPort = false;
+            int port;
+            string host;
+
+            int lastColonPos = uriSpan.LastIndexOf(':');
+            if (lastColonPos != -1)
+            {
+                explicitPort = true;
 
-                    if (IPSocket.TryParseIPEndPoint(uri, out var ipEndPoint))
+                if (IPSocket.TryParseIPEndPoint(uriSpan, out var ipEndPoint))
+                {
+                    if (ipEndPoint.AddressFamily == AddressFamily.InterNetworkV6)
                     {
-                        if (ipEndPoint.AddressFamily == AddressFamily.InterNetworkV6)
-                        {
-                            stunUri.Host = $"[{ipEndPoint.Address}]";
-                        }
-                        else
-                        {
-                            stunUri.Host = ipEndPoint.Address.ToString();
-                        }
-
-                        stunUri.Port = ipEndPoint.Port;
+                        host = $"[{ipEndPoint.Address}]";
                     }
                     else
                     {
-                        stunUri.Host = uri.Substring(0, uri.LastIndexOf(':'));
-                        if (!Int32.TryParse(uri.Substring(uri.LastIndexOf(':') + 1), out stunUri.Port))
-                        {
-                            stunUri.Port = STUNConstants.GetPortForScheme(stunUri.Scheme);
-                        }
+                        host = ipEndPoint.Address.ToString();
                     }
+                    port = ipEndPoint.Port;
                 }
                 else
                 {
-                    stunUri.Host = uri?.Trim();
-                    stunUri.Port = STUNConstants.GetPortForScheme(stunUri.Scheme);
+                    host = uriSpan.Slice(0, lastColonPos).ToString();
+#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
+                    if (!int.TryParse(uriSpan.Slice(lastColonPos + 1), out port))
+#else
+                    if (!int.TryParse(uriSpan.Slice(lastColonPos + 1).ToString(), out port))
+#endif
+                    {
+                        port = STUNConstants.GetPortForScheme(scheme);
+                    }
                 }
             }
-
-            return stunUri;
-        }
-
-        public static bool TryParse(string uriStr, out STUNUri uri)
-        {
-            try
-            {
-                uri = ParseSTUNUri(uriStr);
-                return (uri != null);
-            }
-            catch
+            else
             {
-                uri = null;
-                return false;
+                host = uriSpan.ToString();
+                port = STUNConstants.GetPortForScheme(scheme);
             }
+
+            uri = new STUNUri(scheme, host, port: port, transport: transport, explicitPort: explicitPort);
+            return true;
         }
 
         public override string ToString()
@@ -261,9 +281,14 @@ public static bool AreEqual(STUNUri uri1, STUNUri uri2)
             return uri1 == uri2;
         }
 
+        public bool Equals(STUNUri other)
+        {
+            return (this == other);
+        }
+
         public override bool Equals(object obj)
         {
-            return AreEqual(this, (STUNUri)obj);
+            return Equals(this, (STUNUri)obj);
         }
 
         public static bool operator ==(STUNUri uri1, STUNUri uri2)
@@ -307,11 +332,7 @@ public override bool Equals(object obj)
 
         public override int GetHashCode()
         {
-            return Scheme.GetHashCode()
-                + Transport.GetHashCode()
-                + ((Host != null) ? Host.GetHashCode() : 0)
-                + Port
-                + ((ExplicitPort) ? 1 : 0);
+            return HashCode.Combine(Scheme, Transport, Host, Port, ExplicitPort);
         }
     }
 }

src/sys/Net/IPSocket.cs

@@ -1,4 +1,4 @@
-//-----------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
 // Filename: IPSocket.cs
 //
 // Description: Helper functions for socket strings and IP end points.
@@ -57,36 +57,53 @@ public static string GetSocketString(IPEndPoint endPoint)
         /// <param name="result">If the parse is successful this output parameter will contain the IPv4 or IPv6 end point.</param>
         /// <returns>Returns true if the string could be successfully parsed as an IPv4 or IPv6 end point. False if not.</returns>
         public static bool TryParseIPEndPoint(string s, out IPEndPoint result)
+            => TryParseIPEndPoint(s.AsSpan(), out result);
+
+        /// <summary>
+        /// This code is based on the IPEndPoint.TryParse method in the dotnet source code at
+        /// https://github.com/dotnet/corefx/blob/master/src/System.Net.Primitives/src/System/Net/IPEndPoint.cs.
+        /// If/when that feature makes it into .NET Standard this method can be replaced.
+        /// </summary>
+        /// <param name="s">The end point string to parse.</param>
+        /// <param name="result">If the parse is successful this output parameter will contain the IPv4 or IPv6 end point.</param>
+        /// <returns>Returns true if the string could be successfully parsed as an IPv4 or IPv6 end point. False if not.</returns>
+        public static bool TryParseIPEndPoint(ReadOnlySpan<char> s, out IPEndPoint result)
         {
-            int addressLength = s.Length;  // If there's no port then send the entire string to the address parser
+            result = null;
+            int addressLength = s.Length;
             int lastColonPos = s.LastIndexOf(':');
 
-            // Look to see if this is an IPv6 address with a port.
             if (lastColonPos > 0)
             {
                 if (s[lastColonPos - 1] == ']')
                 {
                     addressLength = lastColonPos;
                 }
-                // Look to see if this is IPv4 with a port (IPv6 will have another colon)
-                else if (s.Substring(0, lastColonPos).LastIndexOf(':') == -1)
+                else if (s.Slice(0, lastColonPos).LastIndexOf(':') == -1)
                 {
                     addressLength = lastColonPos;
                 }
             }
 
-            if (IPAddress.TryParse(s.Substring(0, addressLength), out IPAddress address))
+#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
+            if (IPAddress.TryParse(s.Slice(0, addressLength), out IPAddress address))
+#else
+            if (IPAddress.TryParse(s.Slice(0, addressLength).ToString(), out IPAddress address))
+#endif
             {
                 uint port = 0;
                 if (addressLength == s.Length ||
-                    (uint.TryParse(s.Substring(addressLength + 1), NumberStyles.None, CultureInfo.InvariantCulture, out port) && port <= MaxPort))
+#if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER
+                    (uint.TryParse(s.Slice(addressLength + 1), NumberStyles.None, CultureInfo.InvariantCulture, out port) && port <= MaxPort))
+#else
+                    (uint.TryParse(s.Slice(addressLength + 1).ToString(), NumberStyles.None, CultureInfo.InvariantCulture, out port) && port <= MaxPort))
+#endif
                 {
                     result = new IPEndPoint(address, (int)port);
                     return true;
                 }
             }
 
-            result = null;
             return false;
         }