From b223a0200511f08b2648a8c86b8aa69860971c95 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 03:38:36 +0200 Subject: [PATCH 1/7] failing test --- .../tests/FunctionalTests/Connect.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index d4bdcfed80a0f6..512380685e349f 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -82,6 +82,18 @@ public async Task Connect_MultipleIPAddresses_Success(IPAddress listenAt) } } + [Fact] + public async Task Connect_WhenUnavailable_ThrowsSocketExceptionWithInfo() + { + IPAddress badIp = IPAddress.Parse("4.3.2.1"); + int badPort = 4321; + IPEndPoint badEndpoint = new IPEndPoint(badIp, badPort); + using Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + SocketException ex = await Assert.ThrowsAnyAsync(() => ConnectAsync(client, badEndpoint)); + Assert.Contains("4.3.2.1", ex.Message); + Assert.Contains("4321", ex.Message); + } + [Fact] public async Task Connect_OnConnectedSocket_Fails() { From cfa3dee9dfeddf201e33e3add168a805674b2af0 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 03:39:26 +0200 Subject: [PATCH 2/7] include IP/port information in ConnectAsync exception messages --- .../src/System/Net/Sockets/Socket.Tasks.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs index cde90ae0931cc5..9fc5fbf961f4b8 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Tasks.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Net.Internals; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; @@ -1140,7 +1141,10 @@ private void ThrowException(SocketError error, CancellationToken cancellationTok private Exception CreateException(SocketError error, bool forAsyncThrow = true) { - Exception e = new SocketException((int)error); + // When available, include the IP endpoint information in connection exceptions: + Exception e = LastOperation == SocketAsyncOperation.Connect && _socketAddress != null ? + SocketExceptionFactory.CreateSocketException((int)error, _socketAddress.GetIPEndPoint()) : + new SocketException((int)error); if (forAsyncThrow) { From aef67a5bbe2438bc8818fa2d0d0b8bf0b7d19c99 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 03:39:45 +0200 Subject: [PATCH 3/7] improve test --- .../tests/FunctionalTests/Connect.cs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 512380685e349f..371c85dd3288e7 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -82,16 +82,17 @@ public async Task Connect_MultipleIPAddresses_Success(IPAddress listenAt) } } + [OuterLoop("Slow on Windows")] [Fact] - public async Task Connect_WhenUnavailable_ThrowsSocketExceptionWithInfo() + public virtual async Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo() { - IPAddress badIp = IPAddress.Parse("4.3.2.1"); - int badPort = 4321; - IPEndPoint badEndpoint = new IPEndPoint(badIp, badPort); + // Port 288 is unassigned: + // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt + IPEndPoint badEndpoint = new IPEndPoint(IPAddress.Loopback, 288); using Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); SocketException ex = await Assert.ThrowsAnyAsync(() => ConnectAsync(client, badEndpoint)); - Assert.Contains("4.3.2.1", ex.Message); - Assert.Contains("4321", ex.Message); + Assert.Contains("127.0.0.1", ex.Message); + Assert.Contains("288", ex.Message); } [Fact] @@ -216,5 +217,8 @@ public ConnectTask(ITestOutputHelper output) : base(output) {} public sealed class ConnectEap : Connect { public ConnectEap(ITestOutputHelper output) : base(output) {} + + // We should skip this since the exception creation logic is defined in SocketHelperEap. + public override Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo() => Task.CompletedTask; } } From 4d14f60a268f9fe7673bf43b10fd3f70f91fee75 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 03:46:10 +0200 Subject: [PATCH 4/7] relax assertion in Connect_AfterDisconnect_Fails --- .../System.Net.Sockets/tests/FunctionalTests/Connect.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 371c85dd3288e7..3e998b13399df7 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -128,7 +128,7 @@ public async Task Connect_AfterDisconnect_Fails() } else { - SocketException se = await Assert.ThrowsAsync(() => ConnectAsync(client, new IPEndPoint(IPAddress.Loopback, port))); + SocketException se = await Assert.ThrowsAnyAsync(() => ConnectAsync(client, new IPEndPoint(IPAddress.Loopback, port))); Assert.Equal(SocketError.IsConnected, se.SocketErrorCode); } } From b41023fe24246659ace06dc9355997808962a382 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 04:06:03 +0200 Subject: [PATCH 5/7] add test for DnsEndpoint --- .../tests/FunctionalTests/Connect.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 3e998b13399df7..2a2fc76d81dc49 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -83,12 +83,14 @@ public async Task Connect_MultipleIPAddresses_Success(IPAddress listenAt) } [OuterLoop("Slow on Windows")] - [Fact] - public virtual async Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo() + [Theory] + [InlineData(false)] + [InlineData(true)] + public virtual async Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo(bool useDns) { // Port 288 is unassigned: // https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt - IPEndPoint badEndpoint = new IPEndPoint(IPAddress.Loopback, 288); + EndPoint badEndpoint = useDns ? (EndPoint)new DnsEndPoint("localhost", 288) : new IPEndPoint(IPAddress.Loopback, 288); using Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); SocketException ex = await Assert.ThrowsAnyAsync(() => ConnectAsync(client, badEndpoint)); Assert.Contains("127.0.0.1", ex.Message); @@ -219,6 +221,6 @@ public sealed class ConnectEap : Connect public ConnectEap(ITestOutputHelper output) : base(output) {} // We should skip this since the exception creation logic is defined in SocketHelperEap. - public override Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo() => Task.CompletedTask; + public override Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointInfo(bool useDns) => Task.CompletedTask; } } From 886222bbbdd238736cb176dbe3c6c13d08bf657b Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 04:50:40 +0200 Subject: [PATCH 6/7] handle DnsEndPoint case --- .../src/System/Net/Sockets/MultipleConnectAsync.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MultipleConnectAsync.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MultipleConnectAsync.cs index c201d247f9edf3..816a6cab8c70f8 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MultipleConnectAsync.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/MultipleConnectAsync.cs @@ -239,7 +239,10 @@ private void InternalConnectCallback(object? sender, SocketAsyncEventArgs args) SocketAsyncEventArgs args = _internalArgs!; args.RemoteEndPoint = new IPEndPoint(attemptAddress, _endPoint!.Port); - if (!attemptSocket.ConnectAsync(args)) + bool pending = attemptSocket.ConnectAsync(args); + // Copy the socket address so we can use it in exception message. + _userArgs!._socketAddress = _internalArgs!._socketAddress; + if (!pending) { InternalConnectCallback(null, args); } From 9e9cedf325f7813aec4eec4fdf3231ee3099b856 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 10 Jun 2020 04:57:05 +0200 Subject: [PATCH 7/7] simpler assertion --- .../System.Net.Sockets/tests/FunctionalTests/Connect.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs index 2a2fc76d81dc49..e8b8cd665b0348 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs @@ -93,8 +93,7 @@ public virtual async Task Connect_WhenFails_ThrowsSocketExceptionWithIPEndpointI EndPoint badEndpoint = useDns ? (EndPoint)new DnsEndPoint("localhost", 288) : new IPEndPoint(IPAddress.Loopback, 288); using Socket client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); SocketException ex = await Assert.ThrowsAnyAsync(() => ConnectAsync(client, badEndpoint)); - Assert.Contains("127.0.0.1", ex.Message); - Assert.Contains("288", ex.Message); + Assert.Contains("127.0.0.1:288", ex.Message); } [Fact]