Skip to content

Add IAsyncEnumerable<SftpFile> to enumerate remote files #907

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Add IAsyncEnumerable<SftpFile> to enumerate remote files #907

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
67944ef
Compile with C# 8.0 and add netstandard2.1 target
IgorMilavec Dec 19, 2021
5f02a11
Add EnumerateDirectoryAsync
IgorMilavec Dec 19, 2021
b51cd1e
Add EnumerateDirectoryAsync to ISftpClient
IgorMilavec Dec 22, 2021
97c11ff
Fix review comments
IgorMilavec Feb 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion src/Renci.SshNet/ISftpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,6 @@ public interface ISftpClient : IDisposable
IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallback = null);

#if FEATURE_TAP

/// <summary>
/// Asynchronously retrieves list of files in remote directory.
/// </summary>
Expand All @@ -720,7 +719,29 @@ public interface ISftpClient : IDisposable
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
#if NETSTANDARD2_1_OR_GREATER
[Obsolete("Use EnumerateDirectoryAsync()")]
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
#endif
Task<IEnumerable<SftpFile>> ListDirectoryAsync(string path, CancellationToken cancellationToken);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to only expose the IAsyncEnumerable version (with ListDirectoryAsync as name), even if that means it's .NET Standard 2.1 (or higher) only.
Is the NETSTANDARD2_1_OR_GREATER symbol also defined for .NET 5.0 or 6.0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we reuse the same name with a different signature, this means a breaking change when the caller changes its target framework. If we do a "soft" transition with Obsoletes and a new name then we give callers time to transition to the new method at their own pace. But we do end up with a new name...
The symbol NETSTANDARD* is unfortunately not defined when targeting net5.0 or net6.0, so this check would need to be changed to #if NETSTANDARD2_1_OR_GREATER || NET5_0_OR_GREATER.


#if NETSTANDARD2_1_OR_GREATER
/// <summary>
/// Asynchronously enumerates the files in remote directory.
/// </summary>
/// <param name="path">The path.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
/// <returns>
/// An <see cref="IAsyncEnumerable{SftpFile}"/> that represents the asynchronous enumeration operation.
/// The enumeration contains an async stream of <see cref="SftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
/// <exception cref="SshConnectionException">Client is not connected.</exception>
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
IAsyncEnumerable<SftpFile> EnumerateDirectoryAsync(string path, CancellationToken cancellationToken);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return ISftpFile instead of the concrete SftpFile.

#endif
#endif

/// <summary>
Expand Down
12 changes: 5 additions & 7 deletions src/Renci.SshNet/Renci.SshNet.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
<AssemblyName>Renci.SshNet</AssemblyName>
<AssemblyOriginatorKeyFile>../Renci.SshNet.snk</AssemblyOriginatorKeyFile>
<LangVersion>6</LangVersion>
<LangVersion>8.0</LangVersion>
<SignAssembly>true</SignAssembly>
<TargetFrameworks>net35;net40;net472;netstandard1.3;netstandard2.0</TargetFrameworks>
<TargetFrameworks>net35;net40;net472;netstandard1.3;netstandard2.1</TargetFrameworks>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep these (and other) changes to have a .NET Standard 2.1 flavor in this PR, or separate them out?
This PR would of course rely on that one being merged first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it would be more clean if we first remove old targets and move to .NET Standard 2.0. We should probably also add .NET Standard 2.1 target in that PR, even without using 2.1 features. Later we can implement new 2.1 features in separate PRs (like this one) one feature at a time.

</PropertyGroup>

<!--
<PropertyGroup Condition=" '$(VisualStudioVersion)' == '16.0' ">
<TargetFrameworks>net35;net40;netstandard1.3;netstandard2.0;netstandard2.1</TargetFrameworks>
<PropertyGroup Condition=" '$(VisualStudioVersion)' == '15.0' ">
<TargetFrameworks>net35;net40;net472;netstandard1.3;netstandard2.0</TargetFrameworks>
</PropertyGroup>
-->


<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.3' ">
<PackageReference Include="SshNet.Security.Cryptography" Version="[1.3.0]" />
<PackageReference Include="System.Diagnostics.TraceSource" Version="4.3.0" />
Expand Down
4 changes: 2 additions & 2 deletions src/Renci.SshNet/Security/Cryptography/EcdsaDigitalSignature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public override bool Verify(byte[] input, byte[] signature)
// for 521 sig_size is 132
var sig_size = _key.KeyLength == 521 ? 132 : _key.KeyLength / 4;
var ssh_data = new SshDataSignature(signature, sig_size);
#if NETSTANDARD2_0
#if !NETFRAMEWORK
return _key.Ecdsa.VerifyData(input, ssh_data.Signature, _key.HashAlgorithm);
#else
var ecdsa = (ECDsaCng)_key.Ecdsa;
Expand All @@ -57,7 +57,7 @@ public override bool Verify(byte[] input, byte[] signature)
/// </returns>
public override byte[] Sign(byte[] input)
{
#if NETSTANDARD2_0
#if !NETFRAMEWORK
var signed = _key.Ecdsa.SignData(input, _key.HashAlgorithm);
#else
var ecdsa = (ECDsaCng)_key.Ecdsa;
Expand Down
8 changes: 4 additions & 4 deletions src/Renci.SshNet/Security/Cryptography/EcdsaKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public class EcdsaKey : Key, IDisposable
internal const string ECDSA_P384_OID_VALUE = "1.3.132.0.34"; // Also called nistP384 or secP384r1
internal const string ECDSA_P521_OID_VALUE = "1.3.132.0.35"; // Also called nistP521or secP521r1

#if !NETSTANDARD2_0
#if NETFRAMEWORK
internal enum KeyBlobMagicNumber : int
{
BCRYPT_ECDSA_PUBLIC_P256_MAGIC = 0x31534345,
Expand Down Expand Up @@ -57,7 +57,7 @@ public override string ToString()
return string.Format("ecdsa-sha2-nistp{0}", KeyLength);
}

#if NETSTANDARD2_0
#if !NETFRAMEWORK
/// <summary>
/// Gets the HashAlgorithm to use
/// </summary>
Expand Down Expand Up @@ -144,7 +144,7 @@ public override BigInteger[] Public
byte[] curve;
byte[] qx;
byte[] qy;
#if NETSTANDARD2_0
#if !NETFRAMEWORK
var parameter = Ecdsa.ExportParameters(false);
qx = parameter.Q.X;
qy = parameter.Q.Y;
Expand Down Expand Up @@ -278,7 +278,7 @@ public EcdsaKey(byte[] data)

private void Import(string curve_oid, byte[] publickey, byte[] privatekey)
{
#if NETSTANDARD2_0
#if !NETFRAMEWORK
var curve = ECCurve.CreateFromValue(curve_oid);
var parameter = new ECParameters
{
Expand Down
61 changes: 59 additions & 2 deletions src/Renci.SshNet/SftpClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Renci.SshNet.Common;
using Renci.SshNet.Sftp;
#if FEATURE_TAP
using System.Runtime.CompilerServices;
using System.Threading.Tasks;
#endif

Expand Down Expand Up @@ -537,7 +538,6 @@ public IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallbac
}

#if FEATURE_TAP

/// <summary>
/// Asynchronously retrieves list of files in remote directory.
/// </summary>
Expand All @@ -552,11 +552,15 @@ public IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallbac
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
#if NETSTANDARD2_1_OR_GREATER
[Obsolete("Use EnumerateDirectoryAsync()")]
[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
#endif
public async Task<IEnumerable<SftpFile>> ListDirectoryAsync(string path, CancellationToken cancellationToken)
{
base.CheckDisposed();
if (path == null)
throw new ArgumentNullException("path");
throw new ArgumentNullException(nameof(path));
if (_sftpSession == null)
throw new SshConnectionException("Client not connected.");
cancellationToken.ThrowIfCancellationRequested();
Expand Down Expand Up @@ -594,6 +598,59 @@ public async Task<IEnumerable<SftpFile>> ListDirectoryAsync(string path, Cancell
return result;
}

#if NETSTANDARD2_1_OR_GREATER
/// <summary>
/// Asynchronously enumerates the files in remote directory.
/// </summary>
/// <param name="path">The path.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to observe.</param>
/// <returns>
/// An <see cref="IAsyncEnumerable{SftpFile}"/> that represents the asynchronous enumeration operation.
/// The enumeration contains an async stream of <see cref="SftpFile"/> for the files in the directory specified by <paramref name="path" />.
/// </returns>
/// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
/// <exception cref="SshConnectionException">Client is not connected.</exception>
/// <exception cref="SftpPermissionDeniedException">Permission to list the contents of the directory was denied by the remote host. <para>-or-</para> A SSH command was denied by the server.</exception>
/// <exception cref="SshException">A SSH error where <see cref="Exception.Message" /> is the message from the remote host.</exception>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
public async IAsyncEnumerable<SftpFile> EnumerateDirectoryAsync(string path, [EnumeratorCancellation]CancellationToken cancellationToken)
{
base.CheckDisposed();
if (path == null)
throw new ArgumentNullException(nameof(path));
if (_sftpSession == null)
throw new SshConnectionException("Client not connected.");
cancellationToken.ThrowIfCancellationRequested();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we perform this check in all async methods that we invoke here. Do we want this check here as well?


var fullPath = await _sftpSession.GetCanonicalPathAsync(path, cancellationToken).ConfigureAwait(false);

var handle = await _sftpSession.RequestOpenDirAsync(fullPath, cancellationToken).ConfigureAwait(false);
try
{
var basePath = (fullPath[fullPath.Length - 1] == '/') ?
fullPath :
fullPath + '/';

while (true)
{
var files = await _sftpSession.RequestReadDirAsync(handle, cancellationToken).ConfigureAwait(false);
if (files == null)
{
break;
}

foreach (var file in files)
{
yield return new SftpFile(_sftpSession, basePath + file.Key, file.Value);
}
}
}
finally
{
await _sftpSession.RequestCloseAsync(handle, cancellationToken).ConfigureAwait(false);
}
}
#endif
#endif

/// <summary>
Expand Down