sochix / TLSharp

Telegram client library implemented in C#
1k stars 380 forks source link

Applied fix for sochix/TLSharp#773 #853

Closed CloneDeath closed 5 years ago

CloneDeath commented 5 years ago

Ignored IPv6 datacenters, since it's currently not supported. Added .gitignore for Jetbrains Rider IDE files.

knocte commented 5 years ago

Ignored IPv6 datacenters, since it's currently not supported.

Not supported? Where did you get that from?

CloneDeath commented 5 years ago

Take a look at #773

"An address incompatible with the requested protocol was used [2001:b28:f23f:f005::a]:443"

This happens whenever trying to connect to a datacenter with an IPv6 address. I can confirm that this has halted the incompatible protocol exceptions (by avoiding using them and just using IPv4 datacenters instead).

It is true that it doesn't fix the problem, just bypasses it. If IPv6 support is ever added, then we would need to filter out that small IsIpV6 check. In the meantime, at least it won't crash with no workaround anymore.

knocte commented 5 years ago

It is true that it doesn't fix the problem

Exactly.

If IPv6 support is ever added

What makes you think that IPv6 is not already supported?

CloneDeath commented 5 years ago

"An address incompatible with the requested protocol was used [2001:b28:f23f:f005::a]:443"

The exception being thrown whenever trying to connect to an IPv6 address is what makes me think that it's not already supported.

knocte commented 5 years ago

The exception being thrown whenever trying to connect to an IPv6 address is what makes me think that it's not already supported.

Well, that might just be because the origin is IPv4.

CloneDeath commented 5 years ago

Thanks for the tip. I'm looking into it more now... It looks like the TcpClient default constructor only supports IPv4, which we use, and could be the source of the bug where it looks like IPv4 isn't working. I'm testing it now...

Constructor only works for IPv4: https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.-ctor?view=netframework-4.8#System_Net_Sockets_TcpClient__ctor

Where we are using IPv4-only constructor: https://github.com/sochix/TLSharp/blob/master/TLSharp.Core/Network/TcpTransport.cs#L19

P.S. I appreciate your patience and thoroughness!

CloneDeath commented 5 years ago

Yup. The TcpClient constructor was the problem. I can connect to IPv6 addresses now.

You can reproduce the issue & fix simply with this snippet of code (works with any IPv6 address... the broken case always fails, even with the IPv6 loopback address, throwing the same address exception).

using System;
using System.Net;
using System.Net.Sockets;

namespace IPv6Test {
    class Program {
        static void Main() {
            var ip_address = "::1";
            try {
                ReproduceIssue(ip_address);
                Console.WriteLine("Current Method [Passed]");
            }
            catch (SocketException ex){
                Console.WriteLine("Current Method [Failed]: " + ex.SocketErrorCode);
            }

            try {
                FixedIssue(ip_address);
                Console.WriteLine("Fix [Passed]");
            }
            catch (SocketException ex){
                Console.WriteLine("Fix [Failed] " + ex.SocketErrorCode);
            }
        }

        static void ReproduceIssue(string testAddress) {
            var ipAddress = IPAddress.Parse(testAddress);

            var client = new TcpClient();
            client.Connect(ipAddress, 443);
        }

        static void FixedIssue(string testAddress) {
            var ipAddress = IPAddress.Parse(testAddress);
            var endpoint = new IPEndPoint(ipAddress, 443);

            var client = new TcpClient(endpoint);
            client.Connect(endpoint);
        }
    }
}

Which outputs:

Current Method [Failed]: AddressFamilyNotSupported Fix [Passed]

CloneDeath commented 5 years ago

Alright, updated the PR to support IPv6 addresses.

knocte commented 5 years ago

And this fixes #773?

BTW can you remove all whitespace changes? you're modifying two other files for no reason, only TLSharp.Core/Network/TcpTransport.cs should be modified in this PR.

CloneDeath commented 5 years ago

Sure, closing this PR, and opening a new one for the new branch fix-ipv6-addresses