Files
DodoSSH/tests/DodoSSH.Client.App.Tests/TransferQueueingTests.cs
T
jaap-jan 03c8e8589a Keep downloads inside the folder they were sent to, never run a file opened for editing, and let a bucket upload be retried
- A remote name went straight into Path.Combine. A POSIX name may hold backslashes, a colon or a drive
  letter, and an S3 key may hold "../", so a name chosen by whoever could write to the host decided
  where on this machine a download landed. LocalDirectory.ChildPath cleans the name and requires the
  resolved parent to be the target folder; every download path and EDIT use it, and a name that cannot
  be a file there is left on the host and reported.
- EDIT handed the copy to the shell's default action, which for .bat, .hta, .js and friends is to run
  it. LocalEditor opens such a file with its own "edit" verb or Notepad on Windows and `open -t` on
  macOS, and refuses a .desktop launcher on Linux.
- A failed bucket upload leaves its part object, and a retry asked for an offset no bucket accepts, so
  the row could never finish. IRemoteFileStore.CanWriteFromAnOffset is false for buckets and the queue
  starts those over.
- The SFTP host picker re-found its selection from the empty moment of every vault reload and landed
  on the first host once a minute; the chosen host is now remembered, and a typed password is only
  sent to the host it was typed for.
2026-10-02 13:16:30 +02:00

371 lines
14 KiB
C#

using System.Collections.Concurrent;
using DodoSSH.Client.Shell.ViewModels;
using DodoSSH.Client.Ssh;
using DodoSSH.Client.Transfer;
using NSubstitute;
namespace DodoSSH.Client.App.Tests;
/// <summary>
/// What may be queued for transfer, and what is said about the rest.
/// </summary>
/// <remarks>
/// <para>
/// This is the whole of what drag and drop decides. The handlers on the screen extract paths or rows from a
/// drop and hand them here; every rule about which of them can be moved, which are skipped and what the
/// status line says lives in the view model, where it needs no window.
/// </para>
/// <para>
/// <b>What these cannot cover is the drag itself.</b> Headless Avalonia has no native window and cannot
/// synthesise a platform drag, so a test that pretended to drop a file from the file manager would pass
/// while confirming nothing. The wiring is verified by hand — see <c>docs/manual-checks.md</c> — and what
/// is automated is the half that a person checking by eye would most easily get wrong: the counting.
/// </para>
/// </remarks>
public sealed class TransferQueueingTests : IDisposable
{
private readonly string directory =
Path.Combine(Path.GetTempPath(), $"dodossh-drop-{Guid.CreateVersion7():N}");
/// <summary>What the view model has posted and <see cref="Queued"/> has not run yet.</summary>
/// <remarks>
/// A queue of this test's own, standing in for the dispatcher — which is what it replaces. Draining
/// <c>Dispatcher.UIThread</c> meant depending on which thread a runner happened to touch that
/// process-wide object from first, and once another class got there first every test here died on
/// "the calling thread cannot access this object" having asserted nothing about transfers at all.
/// <para>
/// A queue rather than a poster that runs the action inline, and the difference is not stylistic: the
/// transfer queue raises <c>Changed</c> from its pump thread as well as from the call that enqueued,
/// so inline execution would have a background thread adding rows to an <c>ObservableCollection</c>
/// while the test reads it. Draining keeps every mutation on the thread doing the asserting, which is
/// the one thing the dispatcher was providing that is worth keeping.
/// </para>
/// </remarks>
private readonly ConcurrentQueue<Action> posted = new();
private readonly TransfersViewModel transfers;
public TransferQueueingTests()
{
Directory.CreateDirectory(directory);
transfers = new TransfersViewModel(
Substitute.For<ISftpSessionFactory>(),
TimeProvider.System,
posted.Enqueue);
}
/// <inheritdoc />
public void Dispose()
{
if (Directory.Exists(directory))
{
Directory.Delete(directory, recursive: true);
}
}
/// <remarks>
/// The refusal that has to happen before anything else: there is nowhere to put a file until a host is
/// connected, and a queue that filled up first would start failing the moment one was.
/// </remarks>
[Fact]
public void DroppingFilesWhileDisconnected_QueuesNothingAndSaysWhy()
{
transfers.IsConnected = false;
transfers.QueueUploads([File("one.txt")]);
Queued().ShouldBeEmpty();
transfers.Status.ShouldContain("Connect to a host first");
}
[Fact]
public void DroppingSeveralFiles_QueuesEachOfThem()
{
Connected();
transfers.QueueUploads([File("one.txt"), File("two.txt"), File("three.txt")]);
Queued().Count.ShouldBe(3);
transfers.Status.ShouldContain("3 files");
transfers.Status.ShouldContain("/srv/app");
}
/// <remarks>
/// The paths in an operating-system drop come from another process and are not obliged to still be
/// right by the time the drop lands.
/// </remarks>
[Fact]
public void DroppingAFileThatHasGone_SkipsItAndSaysSo()
{
Connected();
transfers.QueueUploads([File("one.txt"), Path.Combine(directory, "never-existed.txt")]);
Queued().ShouldHaveSingleItem();
transfers.Status.ShouldContain("1 item was no longer there");
}
[Fact]
public void DroppingRemoteRowsOnTheLocalPane_QueuesDownloads()
{
Connected();
transfers.QueueDownloads([RemoteFile("one.log"), RemoteFile("two.log")]);
Queued().Count.ShouldBe(2);
transfers.Status.ShouldContain("2 files");
transfers.Status.ShouldContain("download into");
}
/// <remarks>
/// <para>
/// The buttons were the only way to queue anything before drag and drop, and they now go through the
/// same two methods — so there is one set of rules rather than two that have to agree. This is what
/// says they still do.
/// </para>
/// <para>
/// Files only, since folders stopped being skipped: walking one needs a session to list and to make
/// directories on, and this fixture deliberately has none — everything here is about what may be
/// queued, which needs no server. The recursive half is exercised in <c>ShellFlowTests</c>, against the
/// SFTP fake that a real connect produces.
/// </para>
/// </remarks>
/// <summary>
/// A name the host chose cannot choose where on this machine the download lands.
/// </summary>
/// <remarks>
/// Backslashes and colons are ordinary in a POSIX file name and are separators and drive letters on
/// Windows, and a slash arrives in a name from an S3 key — so every one of these would have escaped the
/// folder through <c>Path.Combine</c> somewhere. They are queued inside it under a cleaned name, and the
/// bare <c>..</c>, which no cleaning can make a file name, is left on the host and said so.
/// </remarks>
[Fact]
public void ADownloadNamedToEscapeTheFolder_LandsInsideItOrNotAtAll()
{
Connected();
transfers.QueueDownloads(
[
RemoteFile(@"..\..\AppData\Roaming\Microsoft\Windows\Start Menu\Programs\Startup\x.bat"),
RemoteFile(@"C:\Windows\evil.dll"),
RemoteFile("../../escaped.txt"),
RemoteFile(".."),
]);
var landed = Queued().Select(row => row.Transfer.LocalPath).ToList();
landed.Count.ShouldBe(3);
landed.ShouldAllBe(path => Path.GetDirectoryName(path) == directory);
transfers.Status.ShouldContain("1 name points outside the folder");
}
[Fact]
public void TheDownloadButton_GoesThroughTheSamePathAsADrop()
{
Connected();
transfers.SelectedRemoteEntry = RemoteFile("one.log");
transfers.DownloadCommand.Execute(null);
Queued().ShouldHaveSingleItem();
transfers.Status.ShouldContain("download into");
}
/// <remarks>
/// Queueing is reachable from any thread — a drop is handled on the UI thread, a retry is not — and
/// nothing about it may depend on which one. Worth stating because the version of this class that
/// drained <c>Dispatcher.UIThread</c> did depend on exactly that, and said so only by failing in CI on
/// a machine whose scheduling differed. The draining still happens on the test's own thread, as
/// <see cref="Queued"/> explains; what is asserted here is the half that has no business caring.
/// </remarks>
[Fact]
public void QueueingFromAnotherThread_StillEnqueues()
{
Exception? failure = null;
var thread = new Thread(() =>
{
try
{
Connected();
transfers.QueueUploads([File("one.txt")]);
}
catch (Exception exception)
{
failure = exception;
}
});
thread.Start();
thread.Join();
failure.ShouldBeNull();
Queued().ShouldHaveSingleItem();
}
/// <remarks>
/// The phone's way in, and it has to obey the same rules as every other: a document chosen in the system
/// picker is copied into the cache and the copy is queued, which is an upload with one extra property —
/// that this application made the file and will delete it again. Everything about *what may be queued*
/// is the same, and this says so rather than leaving a second path free to drift.
/// </remarks>
[Fact]
public void StagedUploads_QueueUnderTheSameRulesAsAnyOther()
{
Connected();
transfers.QueueStagedUploads([File("picked.txt"), Path.Combine(directory, "never-existed.txt")]);
Queued().ShouldHaveSingleItem();
transfers.Status.ShouldContain("1 file");
transfers.Status.ShouldContain("1 item was no longer there");
}
/// <remarks>
/// <para>
/// The phone's way out. The delivery itself — copying the finished file into the document the save
/// picker made — needs a transfer that actually runs and a picker to have made something, so it is
/// checked by hand in <c>docs/manual-checks.md</c> phase 14. What is worth pinning here is the pair of
/// refusals in front of it, because both would otherwise be discovered as an empty file sitting in
/// somebody's Downloads: the picker creates the destination the moment it is dismissed, so anything
/// this method turns away after that point has already cost a visible artefact.
/// </para>
/// </remarks>
[Fact]
public void ADeliveredDownload_QueuesTheFileAndRefusesADirectory()
{
var delivered = 0;
Connected();
transfers.QueueDeliveredDownload(
RemoteFile("one.log"),
Path.Combine(directory, "staged", "one.log"),
_ =>
{
delivered++;
return Task.CompletedTask;
});
Queued().ShouldHaveSingleItem();
transfers.Status.ShouldContain("one.log");
// A directory has nothing to fetch, and the message is the same one every other path on this screen
// gives for the same mistake.
transfers.QueueDeliveredDownload(
RemoteDirectory("logs"),
Path.Combine(directory, "staged", "logs"),
_ => Task.CompletedTask);
Queued().Count.ShouldBe(1);
transfers.Status.ShouldContain("Only files");
// Nothing is delivered by queueing. The callback runs when the bytes are there and not before.
delivered.ShouldBe(0);
}
/// <remarks>
/// <para>
/// The local half of the automatic refresh. A download that finished left this pane showing the folder
/// as it was before the file arrived, so the file this application had just fetched was not in the
/// listing it was fetched into — and the way to see it was to press REFRESH.
/// </para>
/// <para>
/// Synchronous, unlike its remote counterpart, and that is the whole reason it is nothing more than a
/// re-read where it stands: a local listing is a filesystem call, so there is no round trip to defer and
/// no busy state to wait out. See <c>TransfersViewModel.SettleRemoteListingAsync</c> for the other side.
/// </para>
/// </remarks>
[Fact]
public void ADownloadThatLandsInTheFolderShowing_ReReadsItWithoutBeingAsked()
{
Connected();
transfers.RefreshLocalCommand.Execute(null);
transfers.LocalEntries.ShouldBeEmpty();
var landed = File("fetched.bin");
transfers.NoteTransferLanded(new TransferSnapshot(
Guid.CreateVersion7(),
TransferDirection.Download,
"fetched.bin",
landed,
"/srv/app/fetched.bin",
128,
128,
TransferState.Completed,
BytesPerSecond: 0,
Failure: null));
transfers.LocalEntries.Select(row => row.Name).ShouldBe(["fetched.bin"]);
}
/// <remarks>
/// Only the folder on screen, for the reason its remote counterpart gives: a queue writing into
/// somewhere nobody is looking at would otherwise re-read a directory per completed transfer.
/// </remarks>
[Fact]
public void ADownloadThatLandsSomewhereElse_ChangesNothingOnScreen()
{
Connected();
transfers.RefreshLocalCommand.Execute(null);
File("elsewhere.bin");
transfers.NoteTransferLanded(new TransferSnapshot(
Guid.CreateVersion7(),
TransferDirection.Download,
"elsewhere.bin",
Path.Combine(Path.GetTempPath(), "somewhere-else", "elsewhere.bin"),
"/srv/app/elsewhere.bin",
128,
128,
TransferState.Completed,
BytesPerSecond: 0,
Failure: null));
transfers.LocalEntries.ShouldBeEmpty();
}
/// <summary>The queue's rows, once the posts that create them have been let run.</summary>
/// <remarks>
/// <c>TransfersViewModel</c> adds a row from the transfer queue's own <c>Changed</c> event, which it
/// marshals because the queue raises it from a pump thread. Nothing drains that here, so the posts are
/// run by hand — the alternative is asserting on the status line alone, which is a string this code
/// wrote about itself and proves nothing about anything having been enqueued.
/// </remarks>
private IReadOnlyList<TransferRowViewModel> Queued()
{
while (posted.TryDequeue(out var action))
{
action();
}
return transfers.Transfers;
}
private void Connected()
{
transfers.IsConnected = true;
transfers.RemotePath = "/srv/app";
transfers.LocalPath = directory;
}
private string File(string name)
{
var path = Path.Combine(directory, name);
System.IO.File.WriteAllText(path, "contents");
return path;
}
private static RemoteEntryRowViewModel RemoteFile(string name) => new(
new SftpEntry(name, $"/srv/app/{name}", SftpEntryKind.File, 128, DateTimeOffset.UnixEpoch, "-rw-r--r--"));
private static RemoteEntryRowViewModel RemoteDirectory(string name) => new(
new SftpEntry(name, $"/srv/app/{name}", SftpEntryKind.Directory, 0, DateTimeOffset.UnixEpoch, "drwxr-xr-x"));
}