SftpClient: handle the SFTP session being closed by the server (#1362)

* 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

* Always re-create session

---------

Co-authored-by: Rob Hague <rob.hague00@gmail.com>
Co-authored-by: Igor Milavec <igor.milavec@gmail.com>

src/Renci.SshNet/BaseClient.cs

@@ -72,7 +72,7 @@ namespace Renci.SshNet
         /// <see langword="true"/> if this client is connected; otherwise, <see langword="false"/>.
         /// </value>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
-        public bool IsConnected
+        public virtual bool IsConnected
         {
             get
             {
@@ -228,14 +228,19 @@ namespace Renci.SshNet
             // 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.IsConnected)
+            {
+                Session = CreateAndConnectSession();
+            }
 
             try
             {
@@ -287,14 +292,19 @@ namespace Renci.SshNet
             // 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.IsConnected)
+            {
+                Session = await CreateAndConnectSessionAsync(cancellationToken).ConfigureAwait(false);
+            }
 
             try
             {

src/Renci.SshNet/SftpClient.cs

@@ -108,6 +108,22 @@ namespace Renci.SshNet
             }
         }
 
+        /// <summary>
+        /// Gets a value indicating whether this client is connected to the server and
+        /// the SFTP session is open.
+        /// </summary>
+        /// <value>
+        /// <see langword="true"/> if this client is connected and the SFTP session is open; otherwise, <see langword="false"/>.
+        /// </value>
+        /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
+        public override bool IsConnected
+        {
+            get
+            {
+                return base.IsConnected && _sftpSession.IsOpen;
+            }
+        }
+
         /// <summary>
         /// Gets remote working directory.
         /// </summary>
@@ -2473,7 +2489,15 @@ namespace Renci.SshNet
         {
             base.OnConnected();
 
-            _sftpSession = CreateAndConnectToSftpSession();
+            var sftpSession = _sftpSession;
+            if (sftpSession is null)
+            {
+                _sftpSession = CreateAndConnectToSftpSession();
+            }
+            else if (!sftpSession.IsOpen)
+            {
+                sftpSession.Connect();
+            }
         }
 
         /// <summary>

test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs

@@ -287,6 +287,44 @@ namespace Renci.SshNet.IntegrationTests
             }
         }
 
+        [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()
         {