SFTP: stop waiting for ever on a host whose login prints text, and draw the file panes' icons whole #25

Merged
job merged 1 commits from claude/sftp-connection-icons-5ad752 into main 2026-10-19 09:15:25 +00:00
5 changed files with 100 additions and 10 deletions

No files matched your search

+4
View File
@@ -1223,10 +1223,14 @@
The two buttons in the drawer's header: the overflow menu and the arrow that puts the drawer away.
Chrome rather than controls — they are the frame around whichever panel is showing, which is why they
are flat and quiet until the pointer finds them.
No padding, because the box is fixed: Fluent's own 8,5,8,6 left a glyph ten pixels of width inside
twenty-six, and every 15px Material icon on the SFTP panes was drawn with its sides cut off.
-->
<Style Selector="Button.paneicon">
<Setter Property="Width" Value="26" />
<Setter Property="Height" Value="26" />
<Setter Property="Padding" Value="0" />
<Setter Property="CornerRadius" Value="6" />
<Setter Property="FontSize" Value="13" />
<Setter Property="Foreground" Value="{StaticResource TextFaint}" />
@@ -209,10 +209,9 @@
design's, unlike HOST/BUCKET across the arrow column, which keeps its own words for a reason
recorded where it is drawn.
UP is arrow_upward here and arrow_downward on the host/bucket pane below, matching the design's own
two glyphs exactly rather than picking one "up a directory" icon for both — both buttons do the same
thing (go to the parent directory), and the pair reading differently is the design's own choice
faithfully carried over rather than a functional difference invented to justify it.
UP is arrow_upward on both panes. The design drew arrow_downward on the host/bucket pane, and that
was carried over until it was reported as a wrong icon — which it is: both buttons go to the parent
directory, and a down arrow under the tooltip "Up one directory" says the opposite of what it does.
-->
<!--
◆ THE PATH SITS IN THE STAR COLUMN, ALONE, AND THAT PLACEMENT IS LOAD-BEARING.
@@ -398,8 +397,8 @@
directory tree and a flat namespace with inferred folders — and that reason still holds, so only
the style moves to the design's 10px tracked label, not the wording.
UP is arrow_downward here, matching arrow_upward on the local pane above per the design's own two
distinct glyphs; see that pane's own remark on why the pair differs without the actions differing.
UP is arrow_upward, the same as the local pane's; see that pane's own remark for why it is no longer
the design's arrow_downward.
-->
<!--
◆ THE PATH IS TYPED HERE, and that is what the star column is now holding.
@@ -446,7 +445,7 @@
<StackPanel Grid.Column="2" Orientation="Horizontal" Spacing="4" Margin="8,0,0,0">
<Button Classes="paneicon" Command="{Binding RemoteUpCommand}"
IsEnabled="{Binding IsConnected}" ToolTip.Tip="Up one directory">
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="16" Text="&#xE5DB;" />
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="16" Text="&#xE5D8;" />
</Button>
<Button Classes="paneicon" Command="{Binding RefreshRemoteCommand}"
IsEnabled="{Binding IsConnected}" ToolTip.Tip="Refresh">
@@ -546,7 +545,7 @@
</Button>
<Button Classes="paneicon" Command="{Binding RenameRemoteCommand}"
IsEnabled="{Binding CanRenameRemote}" ToolTip.Tip="Rename on the host">
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="15" Text="&#xE9AD;" />
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="15" Text="&#xE9A2;" />
</Button>
<Button Classes="paneicon" Command="{Binding MoveRemoteCommand}"
IsEnabled="{Binding CanMoveRemote}" ToolTip.Tip="Move to another directory">
@@ -139,20 +139,41 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
IAsyncDisposable? lifetime,
CancellationToken cancellationToken)
{
var info = BuildConnectionInfo(request);
// Keep-alive here for the reason it is on the shell client: a transfer connection left open while
// somebody browses is idle in exactly the same way, and a file listing against a machine that has
// since rebooted would otherwise wait on a socket nothing will ever answer.
var client = new SftpClient(BuildConnectionInfo(request))
//
// ◆ The operation timeout is the connect timeout for the handshake, and nothing afterwards. SSH.NET's
// default is to wait for ever, and its ConnectAsync runs the SFTP handshake — the version exchange
// and the home directory lookup — synchronously after the login, where the cancellation token is not
// read. A host whose login prints anything into the channel (an echo in .bashrc is the classic) sends
// text SSH.NET reads as the length of a packet a gigabyte long, and the file browser then said
// "Connecting…" for ever. Reset once connected, so a slow write on a slow link is not cut off.
var client = new SftpClient(info)
{
BufferSize = SftpBufferSize,
KeepAliveInterval = KeepAliveInterval,
OperationTimeout = info.Timeout,
};
// No progress for the file-transfer path. The screen that waits on one is the file browser, which
// reports itself, and a second connection opened behind an already-open shell has nothing the user
// is watching a step list for.
var gate = await ConnectThroughHostKeyGateAsync(client, request, progress: null, cancellationToken)
HostKeyGate gate;
try
{
gate = await ConnectThroughHostKeyGateAsync(client, request, progress: null, cancellationToken)
.ConfigureAwait(false);
}
catch (SshOperationTimeoutException exception) when (info.IsAuthenticated)
{
throw SftpHandshakeTimedOut(request, info, exception);
}
client.OperationTimeout = Timeout.InfiniteTimeSpan;
// Read once, here, rather than per call. SftpClient.WorkingDirectory canonicalises against the server
// on first read, so leaving it to the property would put a round trip behind something that reads
@@ -172,6 +193,24 @@ public sealed class SshNetConnectionFactory(IKnownHostStore knownHosts)
return new SshNetSftpSession(client, gate.Presented!, home, lifetime);
}
/// <summary>Says what a timeout after the login means for a file-transfer connection.</summary>
/// <remarks>
/// Only after the login: a timeout before it is the network's or the server's, and SSH.NET's own words
/// for it are left alone. One after it is the SFTP handshake's — the host let this account in and then
/// never answered as a file server — and SSH.NET's "Operation has timed out" would send somebody looking
/// at the network for what is nearly always a line of text in a login script on the host.
/// </remarks>
private static TimeoutException SftpHandshakeTimedOut(
SshConnectionRequest request,
ConnectionInfo info,
SshOperationTimeoutException exception) =>
new(
$"{request.Host} accepted the login, but its SFTP service did not answer within "
+ $"{info.Timeout.TotalSeconds:0} seconds. This is usually a login script on the host that "
+ "prints text (an echo in .bashrc, say): SFTP needs the remote shell to stay silent when "
+ "it is not interactive.",
exception);
/// <summary>
/// Runs the handshake with host key trust attached, and translates a refusal this factory caused.
/// </summary>
@@ -211,6 +211,31 @@ public sealed class SftpSessionTests(SshServerFixture fixture)
refusal.Presentation.Fingerprint.ShouldStartWith("SHA256:");
}
[Fact]
public async Task AHostWhoseLoginPrintsText_FailsWithinTheConnectTimeout_AndSaysWhy()
{
var knownHosts = new InMemoryKnownHostStore();
var factory = new SshNetConnectionFactory(knownHosts);
var request = Request() with { Port = fixture.ChattyPort, ConnectTimeout = TimeSpan.FromSeconds(3) };
var unknown = await Should.ThrowAsync<SshHostKeyUnknownException>(async () =>
await factory.OpenSftpAsync(request, Token));
await knownHosts.TrustAsync(unknown.Presentation, Token);
// The banner's first four bytes read as the length of a packet about a gigabyte long, and SSH.NET's
// own default is to wait for the rest of it for ever — which the file browser drew as "Connecting…"
// with no end. The outer bound is only there so that regression fails this test rather than hangs it,
// and it is a cancellation rather than a timeout so that it cannot be mistaken for the answer.
using var bound = CancellationTokenSource.CreateLinkedTokenSource(Token);
bound.CancelAfter(TimeSpan.FromSeconds(30));
var failure = await Should.ThrowAsync<TimeoutException>(async () =>
await factory.OpenSftpAsync(request, Token).WaitAsync(bound.Token));
failure.Message.ShouldContain("SFTP service did not answer within 3 seconds");
}
private SshConnectionRequest Request() => new(
fixture.Host,
fixture.Port,
@@ -42,6 +42,17 @@ public sealed class SshServerFixture : IAsyncLifetime
private const int SshPort = 2222;
/// <summary>
/// A second port on the same sshd, where every session starts with a line of text before SFTP.
/// </summary>
/// <remarks>
/// What a host with an <c>echo</c> in its login script looks like to a file-transfer client: the text
/// arrives where the server's first SFTP packet should be. A port rather than an account because this
/// image's sshd runs unprivileged and cannot log in as anybody but <see cref="Username"/>, and a port
/// rather than a change to the main one so the shell and SFTP suites keep a server that behaves.
/// </remarks>
private const int ChattySshPort = 2223;
/// <summary>
/// How many connections in a row the server has to answer before this fixture calls it ready.
/// </summary>
@@ -65,6 +76,9 @@ public sealed class SshServerFixture : IAsyncLifetime
/// <summary>Host port the container's sshd is published on.</summary>
public ushort Port => container!.GetMappedPublicPort(SshPort);
/// <summary>Host port of the same sshd whose sessions print a banner first; see <see cref="ChattySshPort"/>.</summary>
public ushort ChattyPort => container!.GetMappedPublicPort(ChattySshPort);
/// <summary>Host the container is reachable at.</summary>
public string Host => container!.Hostname;
@@ -83,6 +97,7 @@ public sealed class SshServerFixture : IAsyncLifetime
.WithEnvironment("SUDO_ACCESS", "false")
.WithEnvironment("PUBLIC_KEY", ExportOpenSshPublicKey(ClientKey))
.WithPortBinding(SshPort, assignRandomHostPort: true)
.WithPortBinding(ChattySshPort, assignRandomHostPort: true)
// The entrypoint generates host keys, rewrites sshd_config and installs the authorized
// key before sshd is usable, so a published port is not readiness. Both conditions are
@@ -176,6 +191,10 @@ public sealed class SshServerFixture : IAsyncLifetime
/// been touched — the same trap as the wrong file, one layer up. Measured from the log, after a version
/// of this fixture did exactly that and failed twenty-eight tests.
/// </para>
/// <para>
/// The <see cref="ChattySshPort"/> block goes last because a <c>Match</c> runs to the end of the file:
/// anything appended after it would apply only to that port.
/// </para>
/// </remarks>
private async Task ReconfigureAsync()
{
@@ -185,6 +204,10 @@ public sealed class SshServerFixture : IAsyncLifetime
"sed -i 's/^AllowTcpForwarding no/AllowTcpForwarding yes/' /config/sshd/sshd_config"
+ " && sed -i 's/^#*MaxStartups .*/MaxStartups 200/' /config/sshd/sshd_config"
+ " && printf '\\nPerSourcePenalties no\\n' >> /config/sshd/sshd_config"
+ $" && sed -i 's/^Port {SshPort}$/Port {SshPort}\\nPort {ChattySshPort}/' /config/sshd/sshd_config"
+ $" && printf '\\nMatch LocalPort {ChattySshPort}\\n"
+ " ForceCommand /bin/sh -c \"echo Welcome to the test server; exec /usr/lib/ssh/sftp-server\"\\n'"
+ " >> /config/sshd/sshd_config"
+ " && pkill -HUP sshd",
]);