From 0383d9ee68a7ee3591d12144bd179828b4cfb869 Mon Sep 17 00:00:00 2001 From: Marius Thesing Date: Sat, 23 Mar 2024 10:32:39 +0100 Subject: [PATCH 1/2] SftpClient: handle the SFTP session being closed by the server If the server closes the SFTP session but keeps the TCP connection open, this currently causes IsConnected to return true, but any operation fails with "the session is not open". SftpClient.IsConnected now also check sftpSession.IsOpen. Connect() and ConnectAsync() were reworked to take into account that the Session may already/still be open, but the SFTP session may not. This is needed so a reconnect works. fixes #843 and #1153 --- src/Renci.SshNet/BaseClient.cs | 28 +++++++++++--- src/Renci.SshNet/SftpClient.cs | 26 ++++++++++++- .../ConnectivityTests.cs | 38 +++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/Renci.SshNet/BaseClient.cs b/src/Renci.SshNet/BaseClient.cs index db11a6178..d1aa05856 100644 --- a/src/Renci.SshNet/BaseClient.cs +++ b/src/Renci.SshNet/BaseClient.cs @@ -72,7 +72,7 @@ private set /// if this client is connected; otherwise, . /// /// The method was called after the client was disposed. - public bool IsConnected + public virtual bool IsConnected { get { @@ -228,14 +228,23 @@ public void Connect() // forwarded port with a client instead of with a session // // To be discussed with Oleg (or whoever is interested) - if (IsSessionConnected()) + if (IsConnected) { throw new InvalidOperationException("The client is already connected."); } OnConnecting(); - Session = CreateAndConnectSession(); + // The session may already/still be connected here because e.g. in SftpClient, IsConnected also checks the internal SFTP session + var session = Session; + if (session is null) + { + Session = CreateAndConnectSession(); + } + else if (!session.IsConnected) + { + session.Connect(); + } try { @@ -287,14 +296,23 @@ public async Task ConnectAsync(CancellationToken cancellationToken) // forwarded port with a client instead of with a session // // To be discussed with Oleg (or whoever is interested) - if (IsSessionConnected()) + if (IsConnected) { throw new InvalidOperationException("The client is already connected."); } OnConnecting(); - Session = await CreateAndConnectSessionAsync(cancellationToken).ConfigureAwait(false); + // The session may already/still be connected here because e.g. in SftpClient, IsConnected also checks the internal SFTP session + var session = Session; + if (session is null) + { + Session = await CreateAndConnectSessionAsync(cancellationToken).ConfigureAwait(false); + } + else if (!session.IsConnected) + { + await session.ConnectAsync(cancellationToken).ConfigureAwait(false); + } try { diff --git a/src/Renci.SshNet/SftpClient.cs b/src/Renci.SshNet/SftpClient.cs index e9a73c484..16f15ada6 100644 --- a/src/Renci.SshNet/SftpClient.cs +++ b/src/Renci.SshNet/SftpClient.cs @@ -108,6 +108,22 @@ public uint BufferSize } } + /// + /// Gets a value indicating whether this client is connected to the server and + /// the SFTP session is open. + /// + /// + /// if this client is connected and the SFTP session is open; otherwise, . + /// + /// The method was called after the client was disposed. + public override bool IsConnected + { + get + { + return base.IsConnected && _sftpSession.IsOpen; + } + } + /// /// Gets remote working directory. /// @@ -2473,7 +2489,15 @@ protected override void OnConnected() { base.OnConnected(); - _sftpSession = CreateAndConnectToSftpSession(); + var sftpSession = _sftpSession; + if (sftpSession is null) + { + _sftpSession = CreateAndConnectToSftpSession(); + } + else if (!sftpSession.IsOpen) + { + sftpSession.Connect(); + } } /// diff --git a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs index 61ba9e424..a6ff3465c 100644 --- a/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs +++ b/test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs @@ -287,6 +287,44 @@ public void Common_DetectLossOfNetworkConnectivityThroughSftpInvocation() } } + [TestMethod] + public void SftpClient_HandleSftpSessionClose() + { + using (var client = new SftpClient(_connectionInfoFactory.Create())) + { + client.Connect(); + Assert.IsTrue(client.IsConnected); + + client.SftpSession.Disconnect(); + Assert.IsFalse(client.IsConnected); + + client.Connect(); + Assert.IsTrue(client.IsConnected); + + client.Disconnect(); + Assert.IsFalse(client.IsConnected); + } + } + + [TestMethod] + public async Task SftpClient_HandleSftpSessionCloseAsync() + { + using (var client = new SftpClient(_connectionInfoFactory.Create())) + { + await client.ConnectAsync(CancellationToken.None); + Assert.IsTrue(client.IsConnected); + + client.SftpSession.Disconnect(); + Assert.IsFalse(client.IsConnected); + + await client.ConnectAsync(CancellationToken.None); + Assert.IsTrue(client.IsConnected); + + client.Disconnect(); + Assert.IsFalse(client.IsConnected); + } + } + [TestMethod] public void Common_DetectSessionKilledOnServer() { From 1b1636b0864255b4c33704b9773259efca3ab513 Mon Sep 17 00:00:00 2001 From: Marius Thesing Date: Sun, 31 Mar 2024 22:18:14 +0200 Subject: [PATCH 2/2] Always re-create session --- src/Renci.SshNet/BaseClient.cs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Renci.SshNet/BaseClient.cs b/src/Renci.SshNet/BaseClient.cs index d1aa05856..6c721bebc 100644 --- a/src/Renci.SshNet/BaseClient.cs +++ b/src/Renci.SshNet/BaseClient.cs @@ -237,14 +237,10 @@ public void Connect() // The session may already/still be connected here because e.g. in SftpClient, IsConnected also checks the internal SFTP session var session = Session; - if (session is null) + if (session is null || !session.IsConnected) { Session = CreateAndConnectSession(); } - else if (!session.IsConnected) - { - session.Connect(); - } try { @@ -305,14 +301,10 @@ public async Task ConnectAsync(CancellationToken cancellationToken) // The session may already/still be connected here because e.g. in SftpClient, IsConnected also checks the internal SFTP session var session = Session; - if (session is null) + if (session is null || !session.IsConnected) { Session = await CreateAndConnectSessionAsync(cancellationToken).ConfigureAwait(false); } - else if (!session.IsConnected) - { - await session.ConnectAsync(cancellationToken).ConfigureAwait(false); - } try {