Added constructor option to toggle usage of IPv6 datacenter addresses.

[CloneDeath]

This should provide a workaround for #857 without disabling IPv6.

I wanted to clean up the code, and put the data-center selection method into a virtual function, so it could be overridden... But that didn't look as good, so I opted to just take the switch in as a constructor boolean.

[knocte]

@CloneDeath this new API would be useful for people that have trouble connecting to IPv6, but it's not the case explained in issue #857. There, the developer just says that he only gets IPv6 addresses from the list. And he doesn't explicitly say what's the problem connecting to those, is it an exception? He doesn't copy-paste it. You're just doing guess work here.

In this cases it's better to leave the bug in the codebase until a proper bug reporter comes along that makes us understand the problem better and produce a better fix. Ask him for more information in the issue he opened please.

[CloneDeath]

Yeah, I am kinda stumped as it is... I'm guessing (from what he said) that 1) His country/ISP doesn't support IPv4 and 2) Despite that, Telegram is giving him only IPv6 data-centers to choose from.

But, then I guess he's just ignoring the migration, and using whatever IP he wants then... I don't know. Like you said, let's wait for another person to have the issue, and maybe we can narrow down a better solution then.

[knocte]

@CloneDeath take a look at https://github.com/sochix/TLSharp/issues/859

[CloneDeath]

@knocte I still think this PR is relevant and should be merged in.

Specifically, countries like Brazil (and I've heard people from Rusia or the area around) do not currently support IPv6, and until there is more universal adaptation of it, I think the best course of action is for it to default be enabled (because IPv6 is better), but offer the ability for others to disable it with the optional constructor parameter.

Let me know if you have any problems with this idea.

[CloneDeath]

@knocte

Okay, I've been using this feature for a couple months now, and I think it should be merged in. It's a good compromise for those who don't have access to IPv6.

[knocte]

@CloneDeath sorry I lost track of this issue, I thought the latest complaints raised by people were supposed to be fixed in 00330c9bd010945285e56027345d3f37bcf4792a? I get that some countries may not have access to IPv6 but TlSharp should detect this and gracefully downgrade to IPv4, it shouldn't be the responsibility of the developer to specify what to use.

Maybe I'm missing something.

[CloneDeath]

You can't really detect if you have IPv6 or not...

For example, in the Brazil case, if you ask the OS, and you're running Windows 10, since the OS does support it.. but the country's infrastructure does not, you'd ideally like to get a "No" instead. You also couldn't trust the NIC to see if you have an assigned IPv6 address, because it could even be that their private network supports it, but not externally because Brazil/ISP.

Because it's not necessarily the fault of the machine, it could be any machine in-between, I think the best way to detect it would be to (and I don't like this) try to connect with any IP address, and if it fails, see if it's because IPv6 isn't supported, then if so, go back to the list of IPs, and select a non-IPv6 address, then try again.

That's quite a bit of work, and until someone does that, I think adding a simple bool to disable IPv6 should meet everyone's needs. Then, once the above check is in place, or IPv6 is universally adopted, we can drop the optional bool param.

[knocte]

That's quite a bit of work

I disagree, this should be very easy to do, for one that is reproducing the problem.

[CloneDeath]

That's quite a bit of work

I disagree, this should be very easy to do, for one that is reproducing the problem.

You're right, let me rephrase that: It's a little bit too much work for me right now (juggling a few contracts). But maybe in the future. I just don't know if/when that future is.

But pretend someone else did it right now, and it's available now, and working: Going that route... my concern (that I glossed over above) is that if people create and dispose of the telegram client repeatedly, and IPv6 isn't supported, then there will just be this overhead every-time a connection is made.

In the new project I'm working on, we need a single server to periodically connect to many different telegram accounts. It honestly isn't feasible for us to keep thousands of open connections (or more), and the server may be running in regions without IPv6 support, meaning, in those areas, the software will just run slower.

Even with the detection solution in place, I'd still want to disable and bypass the check in those regions, since it'll add some overhead.

[knocte]

Fair enough, you convinced me.

But let's make some tweaks to the PR. In general I'm not a big fan of bool parameters. How about we create an enum called IPVersionPreference, which has these fields: Default, IPv4, IPv6.

[knocte]

Not convinced on the name of the enum actually, maybe DataCenterIPVersion better?

[knocte]

@CloneDeath ping

[solarin]

ok, so you guys just want to implement the Enum-style parameter or you also want, in case you pass default, to automatically downgrade to ipv4 in case ipv6 fails?

[solarin]

to avoid always checking for ipv6 availability, we can create a settings.json file and add this setting once it's resolved.

[knocte]

I think the .Default should do the same as what the current code does today, that is, not filter any address by type

[knocte]

@solarin let's do a fix without settings first, just a new overload that takes an optional parameter (the enum), whose default would map to .Default.

[solarin]

ok

[solarin]

so the enum will have 3 choices Default IPv6 IPv4

Default is doing what the code is doing now.

IPv4 will accept only ipv4 addresses.

Should IPv6 accept only ipv6 addresses, that is, if only ipv4 addresses are passed by telegram we disregard them hence failing to connect? or Ipv6 should reasonably accept also ipv4 addresses?

[knocte]

I don't understand the confusion. Default will do what current master does, IPv4 will only choose datacenters with IPv4 addresses, IPv6 will only choose datacenters with IPv6 addresses. Do you think we need an extra option?

[knocte]

if only ipv4 addresses are passed by telegram we disregard them hence failing to connect?

Oh I understand now, sorry. In this case, if user uses .IPv4 enum member and no IPv4 addresses returned, yes, we should fail the connection and explain the problem in the error message.

If you don't like this solution above, we could do this new alternative:

• .Default
• .OnlyIPv4
• .OnlyIPv6
• .PreferIPv4
• .PreferIPv6

The latter "Prefer" ones would just order the datacenters depending on the address type, to try to connect first to one kind.

[knocte]

Superseded by https://github.com/sochix/TLSharp/pull/924

[solarin]

Superseded by https://github.com/sochix/TLSharp/pull/925/