From 7d2d6aa6fb64d2556ddae54e06a5325012526695 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 13 Aug 2020 14:25:37 +0200 Subject: [PATCH 1/4] fix: NetworkStream throwing inconsistent exceptions --- .../src/System/Net/Sockets/NetworkStream.cs | 32 +++++++++++++------ .../FunctionalTests/NetworkStreamTest.cs | 30 +++++++++-------- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs index e2a3ac6fff45d..61e65da58ac47 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs @@ -264,13 +264,20 @@ public override int Read(Span buffer) ThrowIfDisposed(); if (!CanRead) throw new InvalidOperationException(SR.net_writeonlystream); - int bytesRead = _streamSocket.Receive(buffer, SocketFlags.None, out SocketError errorCode); - if (errorCode != SocketError.Success) + try { - var exception = new SocketException((int)errorCode); - throw NetworkErrorHelper.MapSocketException(exception); + int bytesRead = _streamSocket.Receive(buffer, SocketFlags.None, out SocketError errorCode); + if (errorCode != SocketError.Success) + { + var exception = new SocketException((int)errorCode); + throw NetworkErrorHelper.MapSocketException(exception); + } + return bytesRead; + } + catch (Exception exception) when (!(exception is OutOfMemoryException)) + { + throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); } - return bytesRead; } public override unsafe int ReadByte() @@ -348,11 +355,18 @@ public override void Write(ReadOnlySpan buffer) ThrowIfDisposed(); if (!CanWrite) throw new InvalidOperationException(SR.net_readonlystream); - _streamSocket.Send(buffer, SocketFlags.None, out SocketError errorCode); - if (errorCode != SocketError.Success) + try { - var exception = new SocketException((int)errorCode); - throw NetworkErrorHelper.MapSocketException(exception); + _streamSocket.Send(buffer, SocketFlags.None, out SocketError errorCode); + if (errorCode != SocketError.Success) + { + var exception = new SocketException((int)errorCode); + throw NetworkErrorHelper.MapSocketException(exception); + } + } + catch (Exception exception) when (!(exception is OutOfMemoryException)) + { + throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); } } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs index fb29385985418..0b1a22aa59030 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs @@ -301,8 +301,10 @@ await RunWithConnectedNetworkStreamsAsync((server, _) => }); } - [Fact] - public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException(bool derivedNetworkStream) { using (Socket listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) using (Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) @@ -312,20 +314,22 @@ public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException() Task acceptTask = listener.AcceptAsync(); await Task.WhenAll(acceptTask, client.ConnectAsync(new IPEndPoint(IPAddress.Loopback, ((IPEndPoint)listener.LocalEndPoint).Port))); - using (Socket serverSocket = await acceptTask) - using (DerivedNetworkStream server = new DerivedNetworkStream(serverSocket)) - { - serverSocket.Dispose(); + using Socket serverSocket = await acceptTask; + using NetworkStream server = derivedNetworkStream ? (NetworkStream)new DerivedNetworkStream(serverSocket) : new NetworkStream(serverSocket); + + serverSocket.Dispose(); - Assert.Throws(() => server.Read(new byte[1], 0, 1)); - Assert.Throws(() => server.Write(new byte[1], 0, 1)); + Assert.Throws(() => server.Read(new byte[1], 0, 1)); + Assert.Throws(() => server.Write(new byte[1], 0, 1)); - Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); - Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.Read(new byte[1])); + Assert.Throws(() => server.Write(new byte[1])); - Assert.Throws(() => { server.ReadAsync(new byte[1], 0, 1); }); - Assert.Throws(() => { server.WriteAsync(new byte[1], 0, 1); }); - } + Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); + + Assert.Throws(() => { server.ReadAsync(new byte[1], 0, 1); }); + Assert.Throws(() => { server.WriteAsync(new byte[1], 0, 1); }); } } From 3c847a50fc308a92a4ba6db1d0617200cb4418df Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Thu, 13 Aug 2020 19:16:53 +0200 Subject: [PATCH 2/4] no more nested NetworkExceptions --- .../src/System/Net/Sockets/NetworkStream.cs | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs index 61e65da58ac47..b85c76124cfec 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs @@ -264,20 +264,23 @@ public override int Read(Span buffer) ThrowIfDisposed(); if (!CanRead) throw new InvalidOperationException(SR.net_writeonlystream); + int bytesRead; + SocketError errorCode; try { - int bytesRead = _streamSocket.Receive(buffer, SocketFlags.None, out SocketError errorCode); - if (errorCode != SocketError.Success) - { - var exception = new SocketException((int)errorCode); - throw NetworkErrorHelper.MapSocketException(exception); - } - return bytesRead; + bytesRead = _streamSocket.Receive(buffer, SocketFlags.None, out errorCode); } catch (Exception exception) when (!(exception is OutOfMemoryException)) { throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); } + + if (errorCode != SocketError.Success) + { + var exception = new SocketException((int)errorCode); + throw NetworkErrorHelper.MapSocketException(exception); + } + return bytesRead; } public override unsafe int ReadByte() @@ -355,19 +358,21 @@ public override void Write(ReadOnlySpan buffer) ThrowIfDisposed(); if (!CanWrite) throw new InvalidOperationException(SR.net_readonlystream); + SocketError errorCode; try { - _streamSocket.Send(buffer, SocketFlags.None, out SocketError errorCode); - if (errorCode != SocketError.Success) - { - var exception = new SocketException((int)errorCode); - throw NetworkErrorHelper.MapSocketException(exception); - } + _streamSocket.Send(buffer, SocketFlags.None, out errorCode); } catch (Exception exception) when (!(exception is OutOfMemoryException)) { throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); } + + if (errorCode != SocketError.Success) + { + var exception = new SocketException((int)errorCode); + throw NetworkErrorHelper.MapSocketException(exception); + } } public override unsafe void WriteByte(byte value) => From b90a9a83d4f4749bc73e3873ab0245796fb88949 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 3 Nov 2020 16:25:25 +0100 Subject: [PATCH 3/4] fix exception message and update code --- .../src/System/Net/Sockets/NetworkStream.cs | 4 ++-- .../tests/FunctionalTests/NetworkStreamTest.cs | 17 ++++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs index 64b97621d0aa8..3f13b716cd4c7 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs @@ -258,7 +258,7 @@ public override int Read(Span buffer) } catch (Exception exception) when (!(exception is OutOfMemoryException)) { - throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); + throw GetCustomException(SR.Format(SR.net_io_readfailure, exception.Message), exception); } if (errorCode != SocketError.Success) @@ -337,7 +337,7 @@ public override void Write(ReadOnlySpan buffer) } catch (Exception exception) when (!(exception is OutOfMemoryException)) { - throw GetCustomNetworkException(SR.Format(SR.net_io_writefailure, exception.Message), exception); + throw GetCustomException(SR.Format(SR.net_io_writefailure, exception.Message), exception); } if (errorCode != SocketError.Success) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs index 4b5fcd231b46d..5dc098d766e93 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs @@ -325,18 +325,17 @@ public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException(bool deri serverSocket.Dispose(); - Assert.Throws(() => server.Read(new byte[1], 0, 1)); - Assert.Throws(() => server.Write(new byte[1], 0, 1)); + Assert.Throws(() => server.Read(new byte[1], 0, 1)); + Assert.Throws(() => server.Write(new byte[1], 0, 1)); - Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); - Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); - Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); - Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); - Assert.Throws(() => { server.ReadAsync(new byte[1], 0, 1); }); - Assert.Throws(() => { server.WriteAsync(new byte[1], 0, 1); }); - } + Assert.Throws(() => { server.ReadAsync(new byte[1], 0, 1); }); + Assert.Throws(() => { server.WriteAsync(new byte[1], 0, 1); }); } } From 94e243c0165b665180f98218e099c7ea10d01af9 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 3 Nov 2020 22:35:45 +0100 Subject: [PATCH 4/4] fix test --- .../tests/FunctionalTests/NetworkStreamTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs index 5dc098d766e93..4f03d4ab4c3ea 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/NetworkStreamTest.cs @@ -328,8 +328,8 @@ public async Task DisposeSocketDirectly_ReadWriteThrowNetworkException(bool deri Assert.Throws(() => server.Read(new byte[1], 0, 1)); Assert.Throws(() => server.Write(new byte[1], 0, 1)); - Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); - Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null)); + Assert.Throws(() => server.Read((Span)new byte[1])); + Assert.Throws(() => server.Write((ReadOnlySpan)new byte[1])); Assert.Throws(() => server.BeginRead(new byte[1], 0, 1, null, null)); Assert.Throws(() => server.BeginWrite(new byte[1], 0, 1, null, null));