Skip to content
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.

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

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

Closed
beeradmoore opened this issue Jan 31, 2024 · 5 comments
Closed

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

beeradmoore opened this issue Jan 31, 2024 · 5 comments

Comments

@beeradmoore
Copy link
Contributor

beeradmoore commented Jan 31, 2024

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
Copy link
Owner

robinrodricks commented Jan 31, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

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

@beeradmoore
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants