Public Access
SFTP: stop waiting for ever on a host whose login prints text, and draw the file panes' icons whole #25
No files matched your search
@@ -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="" />
|
||||
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="16" Text="" />
|
||||
</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="" />
|
||||
<TextBlock FontFamily="{StaticResource IconFont}" FontSize="15" Text="" />
|
||||
</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)
|
||||
.ConfigureAwait(false);
|
||||
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",
|
||||
]);
|
||||
|
||||
|
||||
Reference in new issue
Block a user