From 0a548adad7457495cc86733edca990573d7289cd Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 21 Sep 2020 12:30:32 -0400 Subject: [PATCH 1/2] Fix a few Socket.SendFile issues - The string argument in the single-argument overload should be nullable. - All overloads on Windows should allow a null file path, but they've been throwing an exception - On Linux, data was silently truncated when sending a file larger than int.MaxValue with BeginSendFile. --- .../Windows/WinSock/Interop.TransmitFile.cs | 2 +- .../ref/System.Net.Sockets.cs | 2 +- .../src/System/Net/Sockets/Socket.cs | 2 +- .../src/System/Net/Sockets/SocketPal.Unix.cs | 6 +- .../System/Net/Sockets/SocketPal.Windows.cs | 23 ++++- .../tests/FunctionalTests/SendFile.cs | 89 +++++++++++++++++++ 6 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs b/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs index d097205cde4cb6..d6e290713361e2 100644 --- a/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs +++ b/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs @@ -14,7 +14,7 @@ internal static partial class Mswsock [DllImport(Interop.Libraries.Mswsock, SetLastError = true)] internal static extern unsafe bool TransmitFile( SafeHandle socket, - SafeHandle? fileHandle, + IntPtr fileHandle, int numberOfBytesToWrite, int numberOfBytesPerSend, NativeOverlapped* overlapped, 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 d8ccc335743d48..4025a301cee7fb 100644 --- a/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs +++ b/src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs @@ -383,7 +383,7 @@ public static void Select(System.Collections.IList? checkRead, System.Collection public int Send(System.ReadOnlySpan buffer, System.Net.Sockets.SocketFlags socketFlags) { throw null; } public int Send(System.ReadOnlySpan buffer, System.Net.Sockets.SocketFlags socketFlags, out System.Net.Sockets.SocketError errorCode) { throw null; } public bool SendAsync(System.Net.Sockets.SocketAsyncEventArgs e) { throw null; } - public void SendFile(string fileName) { } + public void SendFile(string? fileName) { } public void SendFile(string? fileName, byte[]? preBuffer, byte[]? postBuffer, System.Net.Sockets.TransmitFileOptions flags) { } public bool SendPacketsAsync(System.Net.Sockets.SocketAsyncEventArgs e) { throw null; } public int SendTo(byte[] buffer, int offset, int size, System.Net.Sockets.SocketFlags socketFlags, System.Net.EndPoint remoteEP) { throw null; } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs index ce976d0fd8b2a7..82465b65581629 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs @@ -1299,7 +1299,7 @@ public int Send(ReadOnlySpan buffer, SocketFlags socketFlags, out SocketEr return bytesTransferred; } - public void SendFile(string fileName) + public void SendFile(string? fileName) { SendFile(fileName, null, null, TransmitFileOptions.UseDefaultWorkerThread); } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs index fca424b656a21f..9bbe0f409050ae 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs @@ -1809,9 +1809,9 @@ public static SocketError SendAsync(SafeSocketHandle handle, IList callback) => - SendFileAsync(handle, fileStream, 0, (int)fileStream.Length, callback); + SendFileAsync(handle, fileStream, 0, fileStream.Length, callback); - private static SocketError SendFileAsync(SafeSocketHandle handle, FileStream fileStream, long offset, int count, Action callback) + private static SocketError SendFileAsync(SafeSocketHandle handle, FileStream fileStream, long offset, long count, Action callback) { long bytesSent; SocketError socketError = handle.AsyncContext.SendFileAsync(fileStream.SafeFileHandle, offset, count, out bytesSent, callback); @@ -1849,7 +1849,7 @@ public static async void SendPacketsAsync( var tcs = new TaskCompletionSource(); error = SendFileAsync(socket.InternalSafeHandle, fs, e.OffsetLong, - e.Count > 0 ? e.Count : checked((int)(fs.Length - e.OffsetLong)), + e.Count > 0 ? e.Count : fs.Length - e.OffsetLong, (transferred, se) => { bytesTransferred += transferred; diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs index 1d7947523b12ef..1a23f93a1be526 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs @@ -1137,10 +1137,27 @@ private static unsafe bool TransmitFileHelper( transmitFileBuffers.TailLength = postBuffer.Length; } - bool success = Interop.Mswsock.TransmitFile(socket, fileHandle, 0, 0, overlapped, - needTransmitFileBuffers ? &transmitFileBuffers : null, flags); + bool releaseRef = false; + IntPtr fileHandlePtr = IntPtr.Zero; + try + { + if (fileHandle != null) + { + fileHandle.DangerousAddRef(ref releaseRef); + fileHandlePtr = fileHandle.DangerousGetHandle(); + } - return success; + return Interop.Mswsock.TransmitFile( + socket, fileHandlePtr, 0, 0, overlapped, + needTransmitFileBuffers ? &transmitFileBuffers : null, flags); + } + finally + { + if (releaseRef) + { + fileHandle!.DangerousRelease(); + } + } } public static unsafe SocketError SendFileAsync(SafeSocketHandle handle, FileStream? fileStream, byte[]? preBuffer, byte[]? postBuffer, TransmitFileOptions flags, TransmitFileAsyncResult asyncResult) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs index e17435e74c0f5c..6d5bbbe7ae539f 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Runtime.Versioning; using System.Threading; using System.Threading.Tasks; @@ -104,6 +105,94 @@ public void NotConnected_ThrowsException() } } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SendFile_Empty_SucceedsSendingNothing(bool useAsync) + { + using var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + using var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + listener.BindToAnonymousPort(IPAddress.Loopback); + listener.Listen(1); + + client.Connect(listener.LocalEndPoint); + using Socket server = listener.Accept(); + + if (useAsync) + { + await Task.Factory.FromAsync(server.BeginSendFile, server.EndSendFile, null, null); + } + else + { + server.SendFile(null); + } + Assert.Equal(0, client.Available); + + if (useAsync) + { + await Task.Factory.FromAsync((c, s) => server.BeginSendFile(null, null, null, TransmitFileOptions.UseDefaultWorkerThread, c, s), server.EndSendFile, null); + } + else + { + server.SendFile(null, null, null, TransmitFileOptions.UseDefaultWorkerThread); + } + Assert.Equal(0, client.Available); + + server.Send(new byte[1]); + Assert.Equal(1, client.Receive(new byte[2])); + } + + [ActiveIssue("https://github.com/dotnet/runtime/issues/42534", TestPlatforms.Windows)] + [OuterLoop("Creates and sends a file several gigabytes long")] + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task SendFile_GreaterThan2GBFile_SendsAllBytes(bool useAsync) + { + const long FileLength = 100L + int.MaxValue; + + string tmpFile = GetTestFilePath(); + using (FileStream fs = File.Create(tmpFile)) + { + fs.SetLength(FileLength); + } + + using var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + using var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + listener.BindToAnonymousPort(IPAddress.Loopback); + listener.Listen(1); + + client.Connect(listener.LocalEndPoint); + using Socket server = listener.Accept(); + + await new Task[] + { + Task.Run(async () => + { + if (useAsync) + { + await Task.Factory.FromAsync(server.BeginSendFile, server.EndSendFile, tmpFile, null); + } + else + { + server.SendFile(tmpFile); + } + }), + Task.Run(() => + { + byte[] buffer = new byte[100_000]; + long count = 0; + while (count < FileLength) + { + int received = client.Receive(buffer); + Assert.NotEqual(0, received); + count += received; + } + Assert.Equal(0, client.Available); + }) + }.WhenAllOrAnyFailed(); + } + [OuterLoop] [Theory] [MemberData(nameof(SendFileSync_MemberData))] From ce02c8fdd39ffaeeb9ab9f435435f5d29a6b2924 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 22 Sep 2020 15:25:53 -0400 Subject: [PATCH 2/2] Address PR feedback --- .../tests/FunctionalTests/SendFile.cs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs index 6d5bbbe7ae539f..3a8db0f161ee6e 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendFile.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using System.Runtime.Versioning; using System.Threading; using System.Threading.Tasks; @@ -106,9 +105,15 @@ public void NotConnected_ThrowsException() } [Theory] - [InlineData(false)] - [InlineData(true)] - public async Task SendFile_Empty_SucceedsSendingNothing(bool useAsync) + [InlineData(false, false, false)] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, true, true)] + [InlineData(true, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, true, true)] + public async Task SendFile_NoFile_Succeeds(bool useAsync, bool usePreBuffer, bool usePostBuffer) { using var client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); using var listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); @@ -128,18 +133,26 @@ public async Task SendFile_Empty_SucceedsSendingNothing(bool useAsync) } Assert.Equal(0, client.Available); + byte[] preBuffer = usePreBuffer ? new byte[1] : null; + byte[] postBuffer = usePostBuffer ? new byte[1] : null; + int bytesExpected = (usePreBuffer ? 1 : 0) + (usePostBuffer ? 1 : 0); + if (useAsync) { - await Task.Factory.FromAsync((c, s) => server.BeginSendFile(null, null, null, TransmitFileOptions.UseDefaultWorkerThread, c, s), server.EndSendFile, null); + await Task.Factory.FromAsync((c, s) => server.BeginSendFile(null, preBuffer, postBuffer, TransmitFileOptions.UseDefaultWorkerThread, c, s), server.EndSendFile, null); } else { - server.SendFile(null, null, null, TransmitFileOptions.UseDefaultWorkerThread); + server.SendFile(null, preBuffer, postBuffer, TransmitFileOptions.UseDefaultWorkerThread); } - Assert.Equal(0, client.Available); - server.Send(new byte[1]); - Assert.Equal(1, client.Receive(new byte[2])); + byte[] receiveBuffer = new byte[1]; + for (int i = 0; i < bytesExpected; i++) + { + Assert.Equal(1, client.Receive(receiveBuffer)); + } + + Assert.Equal(0, client.Available); } [ActiveIssue("https://github.com/dotnet/runtime/issues/42534", TestPlatforms.Windows)]