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

AuthenticateAsClientAsync doesn't respect ReadTimeout in a certain scenario #1536

Open
sherlock1982 opened this issue Apr 9, 2024 · 4 comments

Comments

@sherlock1982
Copy link

sherlock1982 commented Apr 9, 2024

FTP Server OS: Windows

FTP Server Type: FileZilla

Client Computer OS: Windows

FluentFTP Version: 49.0.2

Framework: .NET 8

If I try to connect using ftp & ftps it perfectly works. Though ftpes for some reason somehow blocked in my network. Though directly from the machine I can connect.
The issue is in the following code:

				try {
#if NETSTANDARD || NET5_0_OR_GREATER
					m_sslStream.AuthenticateAsClientAsync(targetHost, clientCerts, sslProtocols, Client.Config.ValidateCertificateRevocation).Wait();
#else
					m_sslStream.AuthenticateAsClient(targetHost, clientCerts, sslProtocols, Client.Config.ValidateCertificateRevocation);
#endif
				}

Looks like AuthenticateAsClientAsync doesn't default respect ReadTimeout of 15 seconds and hangs forever. On the other hand AuthenticateAsClient perfectly works.

I can't find any hints in the documentation on why AuthenticateAsClientAsync can behave differently. But I don't see any benefits here to use this function over AuthenticateAsClient as the code is completely sync.

Here are my differences from default client config (for ftpes):

            conn.Config.EncryptionMode = FtpEncryptionMode.Explicit;
            conn.Config.DataConnectionEncryption = true;
            conn.Config.SslBuffering = FtpsBuffering.Off;
            conn.Config.SslProtocols = System.Security.Authentication.SslProtocols.Tls12;

I also found this #682 where looks like SslBuffering.Off was a worarkound. This no longer works.

If it helps here's what i see in console logs:


# Connect(False)
Status:   FluentFTP 49.0.2.0(.NET 8.0) FtpClient
Status:   Connecting to IP #1= ***:21
Status:   Waiting for a response
Response: 220-FileZilla Server 0.9.60 beta
Response: 220-written by Tim Kosse ([email protected])
Response: 220 Please visit https://filezilla-project.org/ [738984.382d]
Status:   Detected FTP server: FileZilla
Command:  AUTH TLS
Status:   Waiting for response to: AUTH TLS
Response: 234 Using authentication type TLS [52ms]

Using a CancellationToken on AuthenticateAsClientAsync also works. Ideally I think API should be completely async with CancellationToken support starting from Connect()

Also worth mentioning that in documentation this function has no timeout exception apart from OperationCanceledException. Implicitly it looks like it is not supposed to respect any timeouts. Though this is also true for AuthenticateAsClient

@FanDjango
Copy link
Collaborator

Thanks for this report. Let's take it apart into separate pieces:

I also found this #682 where looks like SslBuffering.Off was a worarkound. This no longer works.

Let's check that SSL Buffering is actually turned off. There should be a message in the log (verbose!) stating that no SSL Buffering is actually being used (because->.NET 8). So your turning this off yourself "is overkill". And since this doesn't help for your problem... I understand.

I can't find any hints in the documentation on why AuthenticateAsClientAsync can behave differently.

Same for me

On the other hand AuthenticateAsClient perfectly works.

So, in the end, we should perhaps simply use AuthenticateAsClient on the SyncClient in all cases? I think so.

And if that were so, why do we actually think we need to turn SSL Buffering off (at least, in the SYNC client)? Looks like some testing is in order.

@sherlock1982
Copy link
Author

sherlock1982 commented Apr 9, 2024

Well the decision is up to the author I think here.
I can confirm the following:

  1. Buffering is off for me.

  2. AuthenticateAsClientAsync hangs forever looks like (without CancellationToken timeout).

  3. AuthenticateAsClient after 15 seconds (ReadTimeout) throws IOException with inner SocketException with the message: Unable to read data from the transport connection: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond..

  4. AuthenticateAsClientAsync can still be cancelled with a cancellation token.

  5. Inside ActivateEncryptionAsync AuthenticateAsClientAsync is called with provided CancellationToken for .NET >5 which is how it should be. But note that again Token should be cancelled explicitly as ReadTimeout won't be respected.

@FanDjango
Copy link
Collaborator

There is no provision for CancellationToken(s) in the entirety of the SyncClient. Thus using AuthenticateAsClientAsync seems problematic, so much for that.

So two open problems, in my mind:

SyncClient: Remove usage of AuthenticateAsClientAsync overall.
AsyncClient: Need to somehow respect ReadTimeout if .NET > 5.

FanDjango added a commit to FanDjango/FluentFTP that referenced this issue Apr 16, 2024
FanDjango added a commit that referenced this issue Apr 16, 2024
Correct Auth as Client call see #1536
@FanDjango
Copy link
Collaborator

I would like you to test the current master as to the problems you reported. I have just merged a first take on the SYNC side of this topic. Can do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants