followup on SocketAddress validation (dotnet/corefx#40165)

* followup on SocketAddress validation
* update unknow faimily tests
* feedback from review
* fix spelling
* add comments about new values
* feedback from review

Commit migrated from https://github.com/dotnet/corefx/commit/b129f7657a1b93ce9cf577d769d9d03c862e2338

diff --git a/src/libraries/Common/src/System/Net/SocketAddressPal.Windows.cs b/src/libraries/Common/src/System/Net/SocketAddressPal.Windows.cs
index 1033886..b28bad4 100644 (file)
--- a/src/libraries/Common/src/System/Net/SocketAddressPal.Windows.cs
+++ b/src/libraries/Common/src/System/Net/SocketAddressPal.Windows.cs
@@ -18,9 +18,9 @@ namespace System.Net
 
         public static unsafe void SetAddressFamily(byte[] buffer, AddressFamily family)
         {
-            if ((int)(family) > (int)AddressFamily.Max)
+            if ((int)(family) > ushort.MaxValue)
             {
-                // For legacy values up to AddressFamily.Max, family maps directly to Winsock value.
+                // For legacy values family maps directly to Winsock value.
                 // Other values will need mapping if/when supported.
                 // Currently, that is Netlink, Packet and ControllerAreaNetwork, neither of them supported on Windows.
                 throw new PlatformNotSupportedException();

diff --git a/src/libraries/Common/src/System/Net/Sockets/ProtocolFamily.cs b/src/libraries/Common/src/System/Net/Sockets/ProtocolFamily.cs
index 513de29..6594880 100644 (file)
--- a/src/libraries/Common/src/System/Net/Sockets/ProtocolFamily.cs
+++ b/src/libraries/Common/src/System/Net/Sockets/ProtocolFamily.cs
@@ -42,7 +42,6 @@ namespace System.Net.Internals
         Irda = AddressFamily.Irda,
         NetworkDesigners = AddressFamily.NetworkDesigners,
         Max = 29, //AddressFamily.Max
-        Netlink = AddressFamily.Netlink,
         Packet = AddressFamily.Packet,
         ControllerAreaNetwork = AddressFamily.ControllerAreaNetwork,
     }

diff --git a/src/libraries/Native/Unix/System.Native/pal_networking.c b/src/libraries/Native/Unix/System.Native/pal_networking.c
index 25a0d1d..1ece3b6 100644 (file)
--- a/src/libraries/Native/Unix/System.Native/pal_networking.c
+++ b/src/libraries/Native/Unix/System.Native/pal_networking.c
@@ -492,11 +492,6 @@ static bool TryConvertAddressFamilyPlatformToPal(sa_family_t platformAddressFami
         case AF_INET6:
             *palAddressFamily = AddressFamily_AF_INET6;
             return true;
-#ifdef AF_NETLINK
-        case AF_NETLINK:
-            *palAddressFamily = AddressFamily_AF_NETLINK;
-            return true;
-#endif
 #ifdef AF_PACKET
         case AF_PACKET:
             *palAddressFamily = AddressFamily_AF_PACKET;

diff --git a/src/libraries/Native/Unix/System.Native/pal_networking.h b/src/libraries/Native/Unix/System.Native/pal_networking.h
index 2e3281b..4a51cce 100644 (file)
--- a/src/libraries/Native/Unix/System.Native/pal_networking.h
+++ b/src/libraries/Native/Unix/System.Native/pal_networking.h
@@ -53,6 +53,8 @@ typedef enum
  *
  * NOTE: these values are taken from System.Net.AddressFamily. If you add
  *       new entries, be sure that the values are chosen accordingly.
+ *       Unix specific values have distinct offset to avoid conflict with
+ *       Windows values.
  */
 typedef enum
 {
@@ -60,9 +62,8 @@ typedef enum
     AddressFamily_AF_UNIX = 1,     // System.Net.AddressFamily.Unix
     AddressFamily_AF_INET = 2,     // System.Net.AddressFamily.InterNetwork
     AddressFamily_AF_INET6 = 23,   // System.Net.AddressFamily.InterNetworkV6
-    AddressFamily_AF_NETLINK = 30, // System.Net.AddressFamily.Netlink
-    AddressFamily_AF_PACKET = 31,  // System.Net.AddressFamily.Packet
-    AddressFamily_AF_CAN = 32,     // System.Net.AddressFamily.ControllerAreaNetwork
+    AddressFamily_AF_PACKET = 65536,  // System.Net.AddressFamily.Packet
+    AddressFamily_AF_CAN = 65537,     // System.Net.AddressFamily.ControllerAreaNetwork
 } AddressFamily;
 
 /*

diff --git a/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs b/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs
index d2eda86..8578764 100644 (file)
--- a/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs
+++ b/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs
@@ -403,9 +403,8 @@ namespace System.Net.Sockets
         Irda = 26,
         NetworkDesigners = 28,
         Max = 29,
-        Netlink = 30,
-        Packet = 31,
-        ControllerAreaNetwork = 32,
+        Packet = 65536,
+        ControllerAreaNetwork = 65537,
     }
     public enum SocketError
     {

diff --git a/src/libraries/System.Net.Primitives/src/System/Net/Sockets/AddressFamily.cs b/src/libraries/System.Net.Primitives/src/System/Net/Sockets/AddressFamily.cs
index 42b84f3..4892c29 100644 (file)
--- a/src/libraries/System.Net.Primitives/src/System/Net/Sockets/AddressFamily.cs
+++ b/src/libraries/System.Net.Primitives/src/System/Net/Sockets/AddressFamily.cs
@@ -38,8 +38,10 @@ namespace System.Net.Sockets
         Irda = 26,              // IrDA
         NetworkDesigners = 28,  // Network Designers OSI & gateway enabled protocols
         Max = 29,               // Max
-        Netlink = 30,           // Netlink protocol
-        Packet = 31,            // Linux Packet
-        ControllerAreaNetwork = 32, // Controller Area Network automotive bus protocol
+        // Unix specific values are past Uint16.MaxValue to avoid conflicts with Windows values.
+        // On Windows we pass values straight to OS and if we add new protocol supported by Windows,
+        // we should use actual OS value.
+        Packet = 65536,         // Linux Packet
+        ControllerAreaNetwork = 65537, // Controller Area Network automotive bus protocol
     }
 }

diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/SocketAddressTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/SocketAddressTest.cs
index 974d16e..bb887a6 100644 (file)
--- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/SocketAddressTest.cs
+++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/SocketAddressTest.cs
@@ -76,16 +76,28 @@ namespace System.Net.Primitives.Functional.Tests
         }
 
         [Theory]
-        [InlineData((AddressFamily)12345, -1)]
-        [InlineData((AddressFamily)12345, 16)]
+        [InlineData((AddressFamily)65539, -1)]
+        [InlineData((AddressFamily)65539, 16)]
+        [InlineData((AddressFamily)1_000_000, -1)]
         public static void ToString_UnknownFamily_Throws(AddressFamily family, int size)
         {
+            // Values above last known value should throw.
             Assert.Throws<PlatformNotSupportedException>(() => size >= 0 ? new SocketAddress(family, size) : new SocketAddress(family));
         }
 
         [Theory]
+        [InlineData((AddressFamily)125)]
+        [InlineData((AddressFamily)65535)]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void ToString_LegacyUnknownFamily_Success(AddressFamily family)
+        {
+            // For legacy reasons, unknown values in ushort range don't throw on Windows.
+            var sa = new SocketAddress(family);
+            Assert.NotNull(sa.ToString());
+        }
+
+        [Theory]
         [InlineData(AddressFamily.Packet)]
-        [InlineData(AddressFamily.Netlink)]
         [InlineData(AddressFamily.ControllerAreaNetwork)]
         [PlatformSpecific(~TestPlatforms.Linux)]
         public static void ToString_UnsupportedFamily_Throws(AddressFamily family)
@@ -109,7 +121,7 @@ namespace System.Net.Primitives.Functional.Tests
         {
             foreach (AddressFamily family in Enum.GetValues(typeof(AddressFamily)))
             {
-                if ((int)family > (int)AddressFamily.Max)
+                if (family == AddressFamily.Packet || family == AddressFamily.ControllerAreaNetwork)
                 {
                     // Skip Linux specific protocols.
                     continue;

diff --git a/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs b/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs
index 102b70d..e5eb3c6 100644 (file)
--- a/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs
+++ b/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs
@@ -157,9 +157,8 @@ namespace System.Net.Sockets
         Irda = 26,
         NetworkDesigners = 28,
         Max = 29,
-        Netlink = 30,
-        Packet = 31,
-        ControllerAreaNetwork = 32,
+        Packet = 65536,
+        ControllerAreaNetwork = 65537,
     }
     public enum ProtocolType
     {

diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.netcoreapp.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.netcoreapp.cs
index 2aef051..d9f49cc 100644 (file)
--- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.netcoreapp.cs
+++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/CreateSocketTests.netcoreapp.cs
@@ -14,7 +14,6 @@ namespace System.Net.Sockets.Tests
     public partial class CreateSocket
     {
         [Theory]
-        [InlineData(AddressFamily.Netlink)]
         [InlineData(AddressFamily.Packet)]
         [InlineData(AddressFamily.ControllerAreaNetwork)]
         [PlatformSpecific(~TestPlatforms.Linux)]
@@ -25,7 +24,6 @@ namespace System.Net.Sockets.Tests
         }
 
         [Theory]
-        [InlineData(AddressFamily.Netlink)]
         [InlineData(AddressFamily.Packet)]
         [InlineData(AddressFamily.ControllerAreaNetwork)]
         [PlatformSpecific(TestPlatforms.Linux)]