robinrodricks / FluentStorage

A polycloud .NET cloud storage abstraction layer. Provides Blob storage (AWS S3, GCP, FTP, SFTP, Azure Blob/File/Event Hub/Data Lake) and Messaging (AWS SQS, Azure Queue/ServiceBus). Supports .NET 5+ and .NET Standard 2.0+. Pure C#.
MIT License
263 stars 33 forks source link

[Request] Remove dependency of Renci.SshNet.Async #59

Closed beeradmoore closed 7 months ago

beeradmoore commented 7 months ago

The nuget package Renci.SshNet.Async appears rather out of date. On its own it has a dependency of SSH.NET v2016.1.0 which indicates that it has a vulnerability with moderate severity. There is an issue created 12 months ago which asked to bump the dependency but nothing came of it. Consdering FluentStorage.SFTP uses later versions (see below) It appears that the package just needs csporj text edit and no other major changes.

This isn't as much of an issue for FluentStorage.SFTP as it currently has a dependency of SSH.NET (>= 2020.0.2) across all target platforms. So by default people using FluentStorage.SFTP do not have this problem to worry about.

However if you attempt to manually update SSH.NET to 2023.0.0 or even 2023.0.1 you will get an error while attempting to call ListDirectories

Method not found: 'System.Collections.Generic.IEnumerable`1<Renci.SshNet.Sftp.SftpFile> Renci.SshNet.SftpClient.EndListDirectory(System.IAsyncResult)'.
   at Renci.SshNet.Async.SshNetExtensions.ListDirectoryAsync(SftpClient client, String path, Action`1 listCallback, TaskFactory`1 factory, TaskCreationOptions creationOptions, TaskScheduler scheduler)

I don't know if this is due to SSH.NET having targets of .NET 6 and higher (and also .NET framework, and other things) while Renci.SshNet.Async package only has netstandard1.3;net40;uap10.0.

Playing around locally if I remove using Renci.SshNet.Async from SshNetSftpBlobStorage I get 2 errors. Both are calls to client.ListDirectoryAsync( which are here and here.

It looks like the people working on SSH.NET took on a dependency of Microsoft.Bcl.AsyncInterfaces to allow IAsyncEnumerable on frameworks that don't support it (here). I don't know how that would work here but it looks similar as this project also targets netstandard2.0 and up.

As for code changes, I have never used IAsyncEnumerable but I hacked this together which hopefully is a starting off point for someone who knows how to use this better.

I first changed

IEnumerable<SftpFile> directoryContents = await client.ListDirectoryAsync(fullPathGrouping.Key);

List<Blob> blobCollection = directoryContents
    .Where(f => (f.IsDirectory || f.IsRegularFile) && f.FullName == fullPath)
    .Select(ConvertSftpFileToBlob).ToList();

to

List<Blob> blobCollection = new List<Blob>();
await foreach (SftpFile file in client.ListDirectoryAsync(fullPathGrouping.Key, cancellationToken))
{
    if ((file.IsDirectory || file.IsRegularFile) && file.FullName == fullPath)
    {
        blobCollection.Add(ConvertSftpFileToBlob(file));
    }
}

and then later on

IEnumerable<SftpFile> directoryContents = await client.ListDirectoryAsync(folderToList, cancellationToken);
var tempBlobCollection = directoryContents
    .Where(dc => (options.FilePrefix == null || dc.Name.StartsWith(options.FilePrefix))
                    && (dc.IsDirectory || dc.IsRegularFile || dc.OwnerCanRead)
                    && !cancellationToken.IsCancellationRequested
                    && dc.Name != "."
                    && dc.Name != "..")
    .Take(options.MaxResults.Value)
    .Select(ConvertSftpFileToBlob)
    .Where(options.BrowseFilter).ToLis

to

var tempBlobCollection = new List<Blob>();
await foreach (SftpFile dc in client.ListDirectoryAsync(folderToList, cancellationToken))
{
    if ((options.FilePrefix == null || dc.Name.StartsWith(options.FilePrefix))
                    && (dc.IsDirectory || dc.IsRegularFile || dc.OwnerCanRead)
                    && !cancellationToken.IsCancellationRequested
                    && dc.Name != "."
                    && dc.Name != "..")
    {
        tempBlobCollection.Add(ConvertSftpFileToBlob(dc));
    }
}

// TODO: options.BrowseFilter()

(note: this doesn't use options.BrowseFilter, nor does it really make any use of options.MaxResults.Value).

With this I am able to push SSH.NET to 2023.0.1 in my test project which is a .NET MAUI app targeting .NET 8). My test project only ever calls ListAsync and never GetBlobsAsync so I don't know if that first half of the code even works correctly. I just know that it compiles and works for my use case here.

robinrodricks commented 7 months ago

Can you make a PR for this, upgrading SSH.NET to the latest, as well as including the fixes you mentioned? It would be highly appreciated. Thanks.

beeradmoore commented 7 months ago

I'll get onto that tomorrow for you. I won't be certain the way I used IAsyncEnumerable is the correct way though. I'll see if I can run the in project tests to ensure I test all target frameworks as well.

beeradmoore commented 7 months ago

So with the stuff I said above regarding Microsoft.Bcl.AsyncInterfaces, it appears that this was all automatically handled by updating SSH.NET to 2023.0.1.

I did this change as a part of #56

robinrodricks commented 7 months ago

Thanks, if you are done with this, please close this ticket.

beeradmoore commented 7 months ago

Yep this one is all sorted. Thanks for accepting all those PRs so quickly