SocketAddress with arbitrary AddressFamily throws PNSE on Unix

[stephentoub]

This succeeds on Windows:

new SocketAddress((AddressFamily)12345).ToString()

returning:

"12345:32:{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}"

but on Linux and macOS it throws:

Unhandled Exception of Type System.PlatformNotSupportedException
Message :
System.PlatformNotSupportedException : Operation is not supported on this platform.
Stack Trace :
   at System.Net.SocketAddressPal.ThrowOnFailure(Error err) in /_/src/Common/src/System/Net/SocketAddressPal.Unix.cs:line 51
   at System.Net.SocketAddressPal.SetAddressFamily(Byte[] buffer, AddressFamily family) in /_/src/Common/src/System/Net/SocketAddressPal.Unix.cs:line 76
   at System.Net.SocketAddress..ctor(AddressFamily family, Int32 size) in /_/src/Common/src/System/Net/SocketAddress.cs:line 96

[wfurt]

How come it works on Windows? Did OS created socket with unknown family? When I tested Netlink and CanBus I would get error.

[stephentoub]

How come it works on Windows? Did OS created socket with unknown family? When I tested Netlink and CanBus I would get error.

On Windows it doesn't create a Socket or go to the OS at all: it just fills bits into the buffer:
https://github.com/dotnet/corefx/blob/e3a4edabd12b5f7dc9be20136cc867bbe67f1f7c/src/Common/src/System/Net/SocketAddressPal.Windows.cs#L19-L28

In contrast, on Unix it does:
https://github.com/dotnet/corefx/blob/e3a4edabd12b5f7dc9be20136cc867bbe67f1f7c/src/Common/src/System/Net/SocketAddressPal.Unix.cs#L73
which calls:
https://github.com/dotnet/corefx/blob/6f6790fa8219359f2acbe4b0624a3fa9a0a6943b/src/Native/Unix/System.Native/pal_networking.c#L583
which will return EAFNOSUPPORT:
https://github.com/dotnet/corefx/blob/6f6790fa8219359f2acbe4b0624a3fa9a0a6943b/src/Native/Unix/System.Native/pal_networking.c#L585
and result in the PlatformNotSupportedException.

[wfurt]

What do you suggest? Defer creating OS socket?
Socket with unknown family would not be useful for any real usage, right?

[tmds]

On Windows it doesn't create a Socket or go to the OS at all: it just fills bits into the buffer:

Do the enum values of AddressFamily match the native AF_xxx values on Windows?

[tmds]

Do the enum values of AddressFamily match the native AF_xxx values on Windows?

Yes, this seems to be the case. That's why it makes sense to fill in the address family in the Windows implementation. On Linux, you can only fill in what you can map from 'Windows values', PNSE makes sense then.

The recently added values don't map directly with Windows AF_xxx values:

https://github.com/dotnet/corefx/blob/e3a4edabd12b5f7dc9be20136cc867bbe67f1f7c/src/System.Net.Primitives/src/System/Net/Sockets/AddressFamily.cs#L46-L48

Maybe we should start these at a higher offset, e.g. 65536 (so they take more than 2 bytes).
SocketAddress on Windows can throw PNSE for these.

[tmds]

@stephentoub @wfurt based on my earlier comment, we may want to change the values that added in AddressFamily. If such a change is wanted, it should probably be for 3.0?

[stephentoub]

we may want to change the values that added in AddressFamily. If such a change is wanted, it should probably be for 3.0?

Thanks, @tmds. @karelz, @wfurt, what do you want to do with this?

[davidsh]

Thanks, @tmds. @karelz, @wfurt, what do you want to do with this?

These recently added APIs were done for .NET Core 3.0. We need to fix this before we ship .NET Core 3.0.

Do we understand what the right fix is?

[wfurt]

I don't think the addition matters that much @davidsh. You can have yet another protocol number and that will behave different way on Unix and Windows. (like the example with 12345) I'm not aware of any practical impact so this will probably not meet 3.0 bar.

[tmds]

It's not major, this is the practical impact:

• AddressFamily values can no longer match with Windows AF_xxx values for values 30 and higher. So if such values are added, they will need some mapping (like we do on non-Windows).
• SocketAddress should throw on PNSE on Windows for these new values, which it doesn't.

[karelz]

I think we should try to address it and run it by shiproom for 3.0. If it gets rejected, the fix will go into master for 5.0.

[davidsh]

@wfurt @tmds @stephentoub Can one of you please submit a PR with proposed fixes for this (i.e. different enum values, etc.)? We'd like something concrete to discuss with shiproom.