Skip to content
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

discrepancy between TcpClient constructor and Connect method #67879

Closed
wfurt opened this issue Apr 11, 2022 · 2 comments · Fixed by #67951
Closed

discrepancy between TcpClient constructor and Connect method #67879

wfurt opened this issue Apr 11, 2022 · 2 comments · Fixed by #67951
Labels
area-System.Net.Sockets enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@wfurt
Copy link
Member

wfurt commented Apr 11, 2022

TcpClient has public constructor that takes host name and port and when used, it connects underlying Socket. However the behavior is different when creating TcpClient and calling Connect method to the same host.

using System.Net.Sockets;

string host = "localhost";

var client1 = new TcpClient();
client1.Connect(host, 8080);
Console.WriteLine(client1.Client.LocalEndPoint);

var client2 = new TcpClient();
client2.ConnectAsync(host, 8080).GetAwaiter().GetResult();
Console.WriteLine(client2.Client.LocalEndPoint);

var client3 = new TcpClient(host, 8080);
Console.WriteLine(client3.Client.LocalEndPoint);

will produce something like

[::ffff:127.0.0.1]:51920
[::ffff:127.0.0.1]:51922
127.0.0.1:51924

So in some cases use dual mode socket and sometimes we use socket for specific address family. Connect and ConnectAsync produce same result in example above (localhost resolves to single IPv4 address) but ConnectAsync use Socket.ConnectAsync while the synchronous implementation use implementation from TcpClient and that can produce different results.

Also documentation (https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.-ctor?view=net-6.0#system-net-sockets-tcpclient-ctor(system-string-system-int32)) claims that IPv6 will be preferred when available but that is not what the code really does:

IPAddress[] addresses = Dns.GetHostAddresses(hostname);
ExceptionDispatchInfo? lastex = null;
try
{
foreach (IPAddress address in addresses)
{

While somebody may depend on this particular behavior, it seems like it would make sense to decease number of Connect implementations and overall behavior variations.

related to #63162

@ghost
Copy link

ghost commented Apr 11, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

TcpClient has public constructor that takes host name and port and when used, it connects underlying Socket. However the behavior is different when creating TcpClient and calling Connect method to the same host.

using System.Net.Sockets;

string host = "localhost";

var client1 = new TcpClient();
client1.Connect(host, 8080);
Console.WriteLine(client1.Client.LocalEndPoint);

var client2 = new TcpClient();
client2.ConnectAsync(host, 8080).GetAwaiter().GetResult();
Console.WriteLine(client2.Client.LocalEndPoint);

var client3 = new TcpClient(host, 8080);
Console.WriteLine(client3.Client.LocalEndPoint);

will produce something like

[::ffff:127.0.0.1]:51920
[::ffff:127.0.0.1]:51922
127.0.0.1:51924

So in some cases use dual mode socket and sometimes we use socket for specific address family. Connect and ConnectAsync produce same result in example above (localhost resolves to single IPv4 address) but ConnectAsync use Socket.ConnectAsync while the synchronous implementation use implementation from TcpClient and that can produce different results.

Also documentation (https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.-ctor?view=net-6.0#system-net-sockets-tcpclient-ctor(system-string-system-int32)) claims that IPv6 will be preferred when available but that is not what the code really does:

IPAddress[] addresses = Dns.GetHostAddresses(hostname);
ExceptionDispatchInfo? lastex = null;
try
{
foreach (IPAddress address in addresses)
{

While somebody may depend on this particular behavior, it seems like it would make sense to decease number of Connect implementations and overall behavior variations.

related to #63162

Author: wfurt
Assignees: -
Labels:

area-System.Net.Sockets

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 11, 2022
@karelz
Copy link
Member

karelz commented Apr 12, 2022

Triage: Makes sense to make the behavior consistent (and predictable) - we should remove the cruft in TcpClient and call socket.Connect.
Should be simple to do.

@karelz karelz added this to the Future milestone Apr 12, 2022
@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels Apr 12, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 13, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 17, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 16, 2022
@karelz karelz modified the milestones: Future, 7.0.0 Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Sockets enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants