Skip to content

Commit

Permalink
SftpClient: handle the SFTP session being closed by the server (#1362)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people committed Apr 4, 2024
1 parent 9be67c0 commit 8bd08ed
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 6 deletions.
20 changes: 15 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,19 @@ 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.IsConnected)
{
Session = CreateAndConnectSession();
}

try
{
Expand Down Expand Up @@ -287,14 +292,19 @@ 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.IsConnected)
{
Session = await CreateAndConnectSessionAsync(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 8bd08ed

Please sign in to comment.