Skip to content

Commit

Permalink
SftpClient: handle the SFTP session being closed by the server
Browse files Browse the repository at this point in the history
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 sshnet#843 and sshnet#1153
  • Loading branch information
mus65 committed Mar 23, 2024
1 parent 3e6fc4f commit 0383d9e
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 6 deletions.
28 changes: 23 additions & 5 deletions src/Renci.SshNet/BaseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ private set
/// <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
{
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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
{
Expand Down
26 changes: 25 additions & 1 deletion src/Renci.SshNet/SftpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,22 @@ public uint BufferSize
}
}

/// <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>
Expand Down Expand Up @@ -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();
}
}

/// <summary>
Expand Down
38 changes: 38 additions & 0 deletions test/Renci.SshNet.IntegrationTests/ConnectivityTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down

0 comments on commit 0383d9e

Please sign in to comment.