Skip to content

Commit

Permalink
Relaxing IsConnected Check for Datagram Sockets (#87916)
Browse files Browse the repository at this point in the history
* Relaxing IsConnected Check for Datagram Sockets

* Adding Listener inside Test

* Add relaxing to every Connect Path and invert condition to SocketType.Stream

* Make test theory and pass ipv4 ipv6 data

* Add LoopbacksAndAny

* Fix forgotten usage of parameter

* Handle OSX case

* Review feedback
  • Loading branch information
liveans authored Jul 17, 2023
1 parent 9ffdb53 commit fecdb74
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 16 deletions.
28 changes: 12 additions & 16 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs
Original file line number Diff line number Diff line change
Expand Up @@ -797,10 +797,7 @@ public void Connect(EndPoint remoteEP)
throw new InvalidOperationException(SR.net_sockets_mustnotlisten);
}

if (_isConnected)
{
throw new SocketException((int)SocketError.IsConnected);
}
ThrowIfConnectedStreamSocket();

ValidateBlockingMode();

Expand Down Expand Up @@ -839,10 +836,7 @@ public void Connect(IPAddress address, int port)
throw new ArgumentOutOfRangeException(nameof(port));
}

if (_isConnected)
{
throw new SocketException((int)SocketError.IsConnected);
}
ThrowIfConnectedStreamSocket();

ValidateForMultiConnect(isMultiEndpoint: false); // needs to come before CanTryAddressFamily call

Expand Down Expand Up @@ -902,10 +896,7 @@ public void Connect(IPAddress[] addresses, int port)
throw new NotSupportedException(SR.net_invalidversion);
}

if (_isConnected)
{
throw new SocketException((int)SocketError.IsConnected);
}
ThrowIfConnectedStreamSocket();

ValidateForMultiConnect(isMultiEndpoint: true); // needs to come before CanTryAddressFamily call

Expand Down Expand Up @@ -2648,10 +2639,7 @@ internal bool ConnectAsync(SocketAsyncEventArgs e, bool userSocket, bool saeaCan
throw new InvalidOperationException(SR.net_sockets_mustnotlisten);
}

if (_isConnected)
{
throw new SocketException((int)SocketError.IsConnected);
}
ThrowIfConnectedStreamSocket();

// Prepare SocketAddress.
EndPoint? endPointSnapshot = e.RemoteEndPoint;
Expand Down Expand Up @@ -3736,6 +3724,14 @@ private void ThrowIfDisposed()
ObjectDisposedException.ThrowIf(Disposed, this);
}

private void ThrowIfConnectedStreamSocket()
{
if (_isConnected && _socketType == SocketType.Stream)
{
throw new SocketException((int)SocketError.IsConnected);
}
}

private bool IsConnectionOriented => _socketType == SocketType.Stream;

internal static void SocketListDangerousReleaseRefs(IList? socketList, ref int refsAdded)
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Net.Sockets/tests/FunctionalTests/Connect.cs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,21 @@ public async Task Connect_WithoutListener_ThrowSocketExceptionWithAppropriateInf
Assert.Contains(a.LocalEndPoint.ToString(), ex.Message);
}
}

[Theory]
[MemberData(nameof(LoopbacksAndAny))]
public async Task Connect_DatagramSockets_DontThrowConnectedException_OnSecondAttempt(IPAddress listenAt, IPAddress secondConnection)
{
using Socket listener = new Socket(listenAt.AddressFamily, SocketType.Dgram, ProtocolType.Udp);
using Socket s = new Socket(listenAt.AddressFamily, SocketType.Dgram, ProtocolType.Udp);
listener.Bind(new IPEndPoint(listenAt, 0));

await ConnectAsync(s, new IPEndPoint(listenAt, ((IPEndPoint)listener.LocalEndPoint).Port));
Assert.True(s.Connected);
// According to the OSX man page, it's enough connecting to an invalid address to dissolve the connection. (0 port connection returns error on OSX)
await ConnectAsync(s, new IPEndPoint(secondConnection, PlatformDetection.IsOSX ? 1 : 0));
Assert.True(s.Connected);
}
}

public sealed class ConnectSync : Connect<SocketHelperArraySync>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,12 @@ public abstract class MemberDatas
new object[] { IPAddress.Loopback, true },
new object[] { IPAddress.Loopback, false },
};

public static readonly object[][] LoopbacksAndAny = new object[][]
{
new object[] { IPAddress.IPv6Loopback, IPAddress.IPv6Any },
new object[] { IPAddress.Loopback, IPAddress.Any },
};
}

//
Expand Down

0 comments on commit fecdb74

Please sign in to comment.