-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Socket.SendFile() should fail with a consistent exception for connectionless sockets #47472
Comments
Tagging subscribers to this area: @dotnet/ncl Issue Details
ReproThe [Theory]
[InlineData(false)]
[InlineData(true)]
public void UDP_SendFile(bool twice)
{
string tempFile = Path.GetTempFileName();
File.WriteAllBytes(tempFile, new byte[128]);
using var client = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);
using var listener = new Socket(AddressFamily.InterNetwork, SocketType.Dgram, ProtocolType.Udp);
listener.BindToAnonymousPort(IPAddress.Loopback);
client.Connect(listener.LocalEndPoint);
Assert.Throws<SocketException>(() => client.SendFile(tempFile));
if (twice)
{
Assert.Throws<SocketException>(() => client.SendFile(tempFile)); // Throws NotSupportedException
}
}
|
What do we do on Linux? |
related: #47287 |
It does not throw. Maybe |
From the manpage:
This doesn't exclude UDP, so on linux it should work? |
of course with UDP, there is no way to now if what was sent is what was received. It would just blast content of the file without any feedback, |
I also think so (would be nice to double-check with a small app or test). On Windows, it would be nice to throw consistent exceptions. I wonder if we should explicitly invest efforts to document these platform differences better, or are these corner cases we don't care about much, since we don't expect users to meet them? |
Using SendFile with UDP doesn't really make much sense. The point of SendFile is to send large-ish files efficiently. Max size of a UDP packet is ~1500 bytes. It would be really weird to use SendFile with UDP. Additionally, on Linux, the platform sendfile() API itself doesn't support pre and post buffers -- it just takes a fd. So we mimic the Windows behavior by sending the pre and post buffers manually using send(). But if you are using UDP, this probably isn't what you want -- the pre and post buffers will end up getting sent in different packets. As such, I think we should just fail if you try to do SendFile on UDP for both Linux and Windows. The fact that this doesn't fail on Linux today is really an oversight. Also, it seems fine to just always throw NotSupportedException here. |
Seems like we agree on this, I repurposed the issue based on the discussion outcome. |
Triage: to keep is consistent we should throw on Linux, we should fix this for 6.0 until someone depends on not throwing. |
Triage: Not a regression, UDP -- low usage. Fine to move to Future. |
…less sockets (#87108) * Unified to throw NotSupportedException when SendFile() for connectionless sockets The issue was that the Socket.SendFile() threw inconsistent exceptions when the platform was Windows and the protocol was UDP. The first call would throw a SocketException, while the second call would throw a NotSupportedException. With this commit, SendFile() will consistently throw NotSupportException on all platforms when the protocol is UDP. Fix #47472 * Change to throws `NotSupportedException` if `!IsConnectionOriented` or `!Connected`. Before:. Throws `NotSupportedException` on UDP. After: Throws `NotSupportedException` if `!IsConnectionOriented` or `!Connected`. * Changed test case `UdpConnection_ThrowsException` to run regardless of platform. * Update src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs Co-authored-by: Karel Zikmund <[email protected]> --------- Co-authored-by: Karel Zikmund <[email protected]>
Socket.SendFile()
is not supported on Windows, but the exceptions being thrown are not consistent. The first call throwsSocketException
, while the second attempt leads toNotSupportedException
.Repro
The
twice=true
case fails:Edit (after discussion below)
On Linux the call does not fail. We should fail
NotSupportedException
for connectionless sockets on all OS-es in all cases.The text was updated successfully, but these errors were encountered: