Public Access
Compare commits
11
Commits
9f73893e14
...
nightly
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
10f80bded1 | ||
|
|
281f849086 | ||
|
|
25407756c3 | ||
|
|
a763f4b113 | ||
|
|
881562e81b | ||
|
|
93e35a0095 | ||
|
|
b80bf23341 | ||
|
|
8c58e5a558 | ||
|
|
cec73010d3 | ||
|
|
53ff15ba86 | ||
|
|
766fe6aebe |
@@ -0,0 +1,68 @@
|
||||
<Project>
|
||||
|
||||
<!--
|
||||
◆ THE TRIMMER'S VERSION IS PINNED HERE BECAUSE OTHERWISE THE LOCK FILES ARE NOT LOCKED.
|
||||
|
||||
Microsoft.NET.ILLink.Tasks is not referenced by anything in this repository. The SDK adds it
|
||||
on its own to any project that sets IsTrimmable or IsAotCompatible — DodoSSH.Contracts and
|
||||
DodoSSH.Crypto do, and the Android head gets it from trimming being on by default there — and
|
||||
the version it asks for is whatever the running SDK happens to bundle. That version lives in
|
||||
the SDK's own Microsoft.NETCoreSdk.BundledVersions.props, as a KnownILLinkPack item.
|
||||
|
||||
Which makes it a dependency whose version is a property of the toolchain rather than of this
|
||||
repository, and that is the whole problem: packages.lock.json records it as a Direct reference
|
||||
with a requested range, so the lock file silently means "whichever SDK last ran a restore".
|
||||
global.json says rollForward: latestMinor, so CI's setup-dotnet installs the newest 10.x SDK
|
||||
that exists on the day it runs. The moment .NET ships a servicing release, CI's SDK asks for a
|
||||
version the committed lock files do not have, and the locked-mode restore in ci.yml fails with
|
||||
NU1004 before a single file is compiled.
|
||||
|
||||
That is not hypothetical. It closed the whole pipeline: main's run 125 and every open pull
|
||||
request went red together, on
|
||||
|
||||
error NU1004: The package reference Microsoft.NET.ILLink.Tasks version has changed
|
||||
from [10.0.10, ) to [10.0.11, ).
|
||||
|
||||
with nothing in any of those commits touching a package. .NET had shipped SDK 10.0.400, which
|
||||
bundles ILLink 10.0.11 where 10.0.302 bundled 10.0.10, and setup-dotnet installed it the next
|
||||
time anything ran.
|
||||
|
||||
Worse than the outage is the shape of the repair without this pin. Regenerating the lock files
|
||||
holds only until the next servicing release, and it cannot be done from a machine whose newest
|
||||
SDK is older than the runner's: a restore on 10.0.302 writes 10.0.10 straight back and re-breaks
|
||||
CI, so the recorded version becomes a fact about whoever ran restore last rather than about this
|
||||
repository. That is exactly the state locking exists to prevent, and it is not a hypothetical
|
||||
either — every SDK installed on the machine this pin was written on tops out at 10.0.302.
|
||||
|
||||
Pinning it makes the recorded version a decision this repository made, reviewable in a diff
|
||||
like every other version in Directory.Packages.props, and identical on every machine whatever
|
||||
SDK it has. Moving it is then a deliberate edit here plus a regenerated lock file, which is the
|
||||
same ceremony any other dependency bump gets.
|
||||
|
||||
It is an Update on the SDK's item rather than a PackageVersion in Directory.Packages.props, and
|
||||
it has to be: the reference is implicit, so the SDK supplies the version itself and central
|
||||
package management never gets asked. ProcessFrameworkReferences reads @(KnownILLinkPack) when
|
||||
it runs, which is why this lives in Directory.Build.targets — the item does not exist yet while
|
||||
Directory.Build.props is being evaluated.
|
||||
|
||||
Keep this within a patch or two of the runtime the SDK ships. It is the trimming analyzer and
|
||||
the ILLink task, so a small skew is harmless, but a version far behind the framework being
|
||||
analysed is a real way to miss a trim warning.
|
||||
-->
|
||||
<Target Name="PinTheILLinkPackVersion" BeforeTargets="ProcessFrameworkReferences">
|
||||
<!--
|
||||
Inside a target, and not for tidiness. The SDK ships one KnownILLinkPack per target framework
|
||||
and they all share the identity "Microsoft.NET.ILLink.Tasks", so the TargetFramework metadata
|
||||
is the only thing telling net10.0's entry from net8.0's. A condition on %(...) is item
|
||||
batching, which MSBuild permits in a target and rejects during evaluation with MSB4191 — so
|
||||
an ItemGroup at the top of this file cannot express "only the net10.0 one" at all, and the
|
||||
unconditioned Update it would have to become rewrites every framework's entry.
|
||||
-->
|
||||
<ItemGroup>
|
||||
<KnownILLinkPack Update="Microsoft.NET.ILLink.Tasks"
|
||||
Condition="'%(TargetFramework)' == 'net10.0'"
|
||||
ILLinkPackVersion="10.0.11" />
|
||||
</ItemGroup>
|
||||
</Target>
|
||||
|
||||
</Project>
|
||||
@@ -300,7 +300,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference
|
||||
> | The status bar's negotiated cipher, host-key algorithm and key/credential name | ◆ **Shipped, on both surfaces, with three honest deviations.** `ISshConnection` and `ISftpSession` now both carry `Cipher` — the server-to-client algorithm off SSH.NET's own `ConnectionInfo.CurrentServerEncryption`, captured once at construction because a rekey is not an event SSH.NET raises — and `TerminalWorkspace.GetSessionFacts` hands the cipher and the host key's algorithm back to the shell the moment a session opens; `VaultViewModel.TryBuildAuthentication` now threads the authenticating key's or credential's own `Label` into `HostAuthentication.IdentityLabel`, all the way to `MainWindowViewModel`'s surface-aware `SessionCipher`, `SessionHostKeyAlgorithm` and `SessionIdentityLabel`, composed into one `SessionIdentityText` run for the status bar. Three deviations from the mock, not omissions: the algorithm prints exactly as negotiated (`ssh-ed25519`), not the design's shortened `ed25519`, because trimming it would be an edit to a string this client did not choose; the run is plain text rather than the design's clickable element, because there is no pin-details modal for a session that is already open, and drawing a click target for a screen that does not exist would itself be a fabrication; and a typed-password session — nothing filed in the keychain to name — shows the host-key algorithm alone, with no `·` after it, because there is no item behind the dot. |
|
||||
> | S3 dimmed in the design's own switcher | **Enabled.** The mock leaves S3 as future work; this application already has bucket browsing, so SSH, SFTP and S3 are a true three-way segment, wired to `IsSshShowing`, `IsTransfersShowing` and `IsBucketsShowing` exactly alike. |
|
||||
> | The S3/Buckets screen | **Did not get the session shell in v5b.** `TransfersScreen` serves both SFTP and S3 today and only the SFTP usage in `MainWindow.axaml` sat inside the new tab row/header/status bar/sidebar; the S3 usage was unchanged at the time. **v5c gives it the shell's own look without the machinery** — a 26-pixel padded, bordered, radius-12 container and nothing past that, since a bucket has no tab to close, no host to head a card with and no pin for a sidebar to show; see the v5c section, below. |
|
||||
> | No pins destination in the design at all | **Kept anyway.** The rail still carries Pins — `KnownHostsScreen` — because the mock has no screen for approved host keys and this application's has to stay reachable. |
|
||||
> | No pins destination in the design at all | **The rail agrees with the design now.** `KnownHostsScreen` is still built and still reachable — from **Host keys** on the Keys screen's own header, which was always the second way in — but the rail's Pins row is gone. It was kept through v5b on the grounds that the mock has no screen for approved host keys, which is a reason for the screen to exist and was never a reason for a rail entry once the keychain had a door to the same place. Two rail rows landing on one screen is a rail that has to be read twice. |
|
||||
> | The popover's Settings and Preferences rows, and the design's own Settings-* family of screens | **Landed in v5c.** What was two doors to one room in v5b — Settings and Preferences both opening the same bare `Preferences` screen — is now two of three doors onto their own settings pages: Settings opens General, Preferences opens Preferences, and a third row, Vaults, opens Vaults. All three are real, distinct pages inside one settings mode; see the v5c section, below. |
|
||||
> | `· Org` after the user chip's name, and a `Primary` tag on a vault row in the popover | Neither. There is no organisation concept behind a vault — only the vault itself — and no vault is distinguished as primary; the popover's vault rows are the existing shown-vaults toggles, restyled. |
|
||||
> | The design's titlebar, which has nowhere for a sync indicator | `SYNCED` stays, on the titlebar's right side, ahead of the window's own minimise/maximise/close buttons — the one thing this titlebar keeps that the design's own does not draw at all. |
|
||||
|
||||
@@ -28,8 +28,9 @@ a phase had nothing left for a person to do, which is the good outcome rather th
|
||||
|
||||
### 1.1 No screen is sliced at the WebView's left edge · **the important one**
|
||||
|
||||
Open two terminals, then visit every nav rail entry in turn — Hosts, Keys, Pins, Snips, Logs — and both of
|
||||
the switcher's other two segments, SFTP and S3, at the rail's own head.
|
||||
Open two terminals, then visit every nav rail entry in turn — Hosts, Keys, Snips, Logs — and both of
|
||||
the switcher's other two segments, SFTP and S3, at the rail's own head. The pins screen is no longer a rail
|
||||
entry; reach it from **Host keys** on the Keys screen's header and check it the same way.
|
||||
|
||||
**Pass:** each screen draws whole, its buttons all clickable, and the nav rail stays up the left edge for
|
||||
every one of them. Since v5b's chrome pass the rail is permanent furniture — it no longer collapses for
|
||||
@@ -2423,7 +2424,9 @@ script warns rather than failing when that is legitimate, which is the first rel
|
||||
### 16.7 The update arrives, and the restart lands in it · **the whole point of the work**
|
||||
|
||||
With v0.1.0 installed and running, a vault unlocked, a host change made, and **a terminal open**, publish
|
||||
v0.1.1 (`-Upload`). Then press CHECK NOW on Settings → General rather than waiting six hours.
|
||||
v0.1.1 (`-Upload`). Then press CHECK NOW on Settings → General rather than waiting six hours. Closing and
|
||||
reopening the application does the same thing without the button: the first pass of the loop runs at launch,
|
||||
so a client started after a release finds it without anybody asking.
|
||||
|
||||
**Pass:** the progress bar moves, the banner appears above the status bar, and — the part to actually watch
|
||||
— the terminal **reflows cleanly rather than being sliced**, with the remote seeing the smaller row count.
|
||||
|
||||
@@ -712,6 +712,30 @@ The lasting hazard is the first paragraph and not the fix. Any change to a share
|
||||
to a lock file this repository cannot verify from a machine without the Android workload, and it will go
|
||||
on being noticed later than every other one.
|
||||
|
||||
**A lock file can go stale with nothing in this repository changing, because `Microsoft.NET.ILLink.Tasks`
|
||||
is versioned by the SDK and `global.json` lets the SDK float.** The reference is implicit — nothing in any
|
||||
`.csproj` asks for it — and its version tracks the runtime patch band, while `global.json` pins only
|
||||
`10.0.100` with `rollForward: latestMinor`. So `setup-dotnet` installs whatever the newest 10.x SDK is on
|
||||
the day, and the moment that SDK's band moves, locked-mode restore stops:
|
||||
|
||||
```
|
||||
error NU1004: The package reference Microsoft.NET.ILLink.Tasks version has changed
|
||||
from [10.0.10, ) to [10.0.11, ).
|
||||
```
|
||||
|
||||
It named `DodoSSH.Client.Android`, `DodoSSH.Contracts` and `DodoSSH.Crypto` — the three lock files that
|
||||
carry the entry — on a commit that touched none of them and no dependency at all.
|
||||
|
||||
The fix is `--force-evaluate` on those three, **from a machine whose SDK is at least as new as the
|
||||
runner's**, which is the part that is easy to get wrong: a `--force-evaluate` from an older SDK rewrites
|
||||
the lock at the older version, changes nothing, and looks like it worked. Check `dotnet --version` against
|
||||
the version in the error before believing a regeneration.
|
||||
|
||||
This will recur on every SDK patch that moves the band. It is the accepted cost of letting the SDK float:
|
||||
the alternative is pinning an exact SDK in `global.json`, which trades a recurring lock-file bump for a
|
||||
recurring toolchain bump and makes every contributor install one specific SDK. Neither is free, and this
|
||||
repository has chosen the floating side deliberately.
|
||||
|
||||
**.NET for Android cannot be built on a musl host, and this project's runner is Alpine. Every message the
|
||||
toolchain produces on the way to saying so names a missing file that is present.** Three CI rounds went
|
||||
into this and the first two fixed symptoms, so the messages are worth reading in the order they arrive.
|
||||
|
||||
@@ -74,9 +74,9 @@
|
||||
},
|
||||
"Microsoft.NET.ILLink.Tasks": {
|
||||
"type": "Direct",
|
||||
"requested": "[10.0.10, )",
|
||||
"resolved": "10.0.10",
|
||||
"contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg=="
|
||||
"requested": "[10.0.11, )",
|
||||
"resolved": "10.0.11",
|
||||
"contentHash": "IBf7lbovvjGWVWXZX5cJ/cO0WXbId0Zq4BuSeT94mGZuOAP66oMeH9PTBZ9Jpp3Jb6jtK0qm/NyUbPRo1gC/wQ=="
|
||||
},
|
||||
"MinVer": {
|
||||
"type": "Direct",
|
||||
|
||||
@@ -913,18 +913,30 @@
|
||||
into a grid: equal columns, and a card that grew a third line of tags is taller than its neighbours
|
||||
rather than narrower.
|
||||
|
||||
◆ 224 IS DERIVED, and the arithmetic is written out because getting it wrong is invisible. The grid's
|
||||
column at the window's minimum is 1016 less the rail's 190 and the drawer's 320, which is 506. The
|
||||
scrolling stack inside it takes 16 of margin on each side, and the vertical scrollbar takes its own —
|
||||
call the usable width 474. A WrapPanel fits floor(474 / (Width + 10)) per row, so two columns needs
|
||||
Width no more than 227.
|
||||
◆ 214 IS DERIVED, and the arithmetic is written out because getting it wrong is invisible. The grid's
|
||||
column at the window's minimum is 1081 less the rail's 255 and the drawer's 320, which is 506. The
|
||||
scrolling stack inside it takes 26 of margin on each side, and the vertical scrollbar takes its own —
|
||||
call the usable width 454. A WrapPanel fits floor(454 / (Width + 10)) per row, so two columns needs
|
||||
Width no more than 217, and 214 is that with the same few pixels of slack the previous number kept.
|
||||
|
||||
The first number here was 248, from the same reasoning with the two margins left out. It laid out
|
||||
cleanly and the layout harness passed it, because the harness asks whether a control is inside the
|
||||
window and not how many of them fit on a line — so the grid quietly became one column wide at exactly
|
||||
the size this application guarantees, which is the shape the cards exist to avoid. The second was 232,
|
||||
derived the same way against the drawer's own 304; v5 widened the drawer to 320 for the ADDRESS field's
|
||||
breathing room, which narrowed the budget this number is drawn from and had to move it down in step.
|
||||
Every number here has moved at least once, and always because something beside the cards did:
|
||||
|
||||
· 248, from this reasoning with the two margins left out. It laid out cleanly and the layout harness
|
||||
passed it, because the harness asks whether a control is inside the window and not how many of them
|
||||
fit on a line — so the grid quietly became one column wide at exactly the size this application
|
||||
guarantees, which is the shape the cards exist to avoid.
|
||||
· 232, derived against the drawer's own 304, which v5 widened to 320 for the ADDRESS field's breathing
|
||||
room — narrowing the budget this number is drawn from and moving it down in step.
|
||||
· 224, which is what that gave. The stated arithmetic still said 1016 and 190 by then: v5b's rail took
|
||||
190 to 255 and the window's minimum 1016 to 1081 in the same pass, so the two changes cancelled and
|
||||
the answer stayed right while the working went stale.
|
||||
· 214, now that HostsScreen's board is inset 26 a side rather than 16 — see that file's own remark on
|
||||
why every screen frames its content the same way. Twenty pixels of board is twenty pixels the cards
|
||||
no longer have, and this is where they come from.
|
||||
|
||||
◆ THE TEST THAT CATCHES THIS IS NOT THE HARNESS. See
|
||||
ScreenLayoutTests.TheHostsGridKeepsTwoColumnsAtTheMinimumWithTheDrawerOpen, which counts columns
|
||||
because that is the thing this number exists to buy and the thing no fit assertion can see.
|
||||
-->
|
||||
<Style Selector="Border.tile">
|
||||
<Setter Property="Background" Value="{StaticResource Raised}" />
|
||||
@@ -932,7 +944,7 @@
|
||||
<Setter Property="BorderThickness" Value="1" />
|
||||
<Setter Property="CornerRadius" Value="12" />
|
||||
<Setter Property="Padding" Value="12,10" />
|
||||
<Setter Property="Width" Value="224" />
|
||||
<Setter Property="Width" Value="214" />
|
||||
<Setter Property="Margin" Value="0,0,10,10" />
|
||||
</Style>
|
||||
<Style Selector="ListBoxItem:pointerover Border.tile">
|
||||
|
||||
@@ -98,6 +98,18 @@
|
||||
|
||||
<Grid ColumnDefinitions="*,Auto">
|
||||
|
||||
<!--
|
||||
── 26 DOWN EACH SIDE, the same inset Keychain, Snips, Logs and Pins all take. ──────────────────────
|
||||
Those four say it once, as Margin="26" on their own root; this screen repeats it on each of the four
|
||||
rows below, and it has to. The board's ScrollViewer is the last row and is deliberately full-bleed, so
|
||||
that its scrollbar rides the pane's own edge rather than floating 26 pixels inside it — a root margin
|
||||
would inset the bar with everything else. It would also inset the drawer in the second column, which
|
||||
draws its own edge and wants none.
|
||||
|
||||
It was 16 and 20 until this pass, which put the Hosts header a visible step left of and above every
|
||||
other screen's. Four numbers rather than one is the cost of the two exceptions above; changing one of
|
||||
them means changing all four.
|
||||
-->
|
||||
<Grid Grid.Column="0" RowDefinitions="Auto,Auto,Auto,*">
|
||||
|
||||
<!--
|
||||
@@ -107,7 +119,7 @@
|
||||
buttons over a board of forty is a pair whose subject the user has to work out. The group's own
|
||||
Edit/Move/Delete sit on its own heading's menu for the same reason.
|
||||
-->
|
||||
<Grid Grid.Row="0" Margin="16,20,16,16" ColumnDefinitions="Auto,Auto,*,Auto,Auto,Auto">
|
||||
<Grid Grid.Row="0" Margin="26,26,26,16" ColumnDefinitions="Auto,Auto,*,Auto,Auto,Auto">
|
||||
|
||||
<TextBlock Grid.Column="0" Text="Hosts" FontSize="33" FontWeight="Bold" LetterSpacing="-0.5"
|
||||
Foreground="{StaticResource Text}" VerticalAlignment="Center" />
|
||||
@@ -219,7 +231,7 @@
|
||||
Ctrl+K is named on it because the palette is the other way to reach a host by typing, and somebody
|
||||
who has found this box should know about the one that also connects on Enter.
|
||||
-->
|
||||
<Border Grid.Row="1" Margin="16,0,16,16">
|
||||
<Border Grid.Row="1" Margin="26,0,26,16">
|
||||
<TextBox x:Name="HostFilter" Text="{Binding HostFilter}" Height="40" CornerRadius="10"
|
||||
FontFamily="{StaticResource MonoFont}"
|
||||
PlaceholderText="Find a host by name, address or note… · Ctrl+K searches and connects" />
|
||||
@@ -232,7 +244,7 @@
|
||||
one of them sits here, above the board, rather than laid over it: a card over the cards would hide
|
||||
the very ticks or the very group it is asking about.
|
||||
-->
|
||||
<StackPanel Grid.Row="2" Margin="16,0,16,12" Spacing="10">
|
||||
<StackPanel Grid.Row="2" Margin="26,0,26,12" Spacing="10">
|
||||
|
||||
<!--
|
||||
The conflict log. The merge is only allowed to pick a winner because the value it overrode is kept
|
||||
@@ -434,7 +446,7 @@
|
||||
HostsScreen.axaml.cs.
|
||||
-->
|
||||
<ScrollViewer Grid.Row="3" x:Name="Scroll" HorizontalScrollBarVisibility="Disabled">
|
||||
<StackPanel Margin="16,0,16,20" Spacing="16">
|
||||
<StackPanel Margin="26,0,26,26" Spacing="16">
|
||||
|
||||
<!--
|
||||
Named because it is where keyboard focus lands when the terminal gives it back, and because
|
||||
|
||||
@@ -54,6 +54,21 @@ internal sealed partial class MainWindow : Window
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Colours the system-drawn frame the moment there is a handle to colour it on.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <c>OnOpened</c> and not the constructor: the window has no platform handle until it is shown, and
|
||||
/// <see cref="NativeWindowFrame"/> does nothing without one. See that class for what the frame is and
|
||||
/// why <c>BorderOnly</c> still has one.
|
||||
/// </remarks>
|
||||
protected override void OnOpened(EventArgs e)
|
||||
{
|
||||
base.OnOpened(e);
|
||||
|
||||
NativeWindowFrame.MatchTo(this);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Asks the Linux backend for the one mode it can actually draw inside this window.
|
||||
/// </summary>
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
using System.Runtime.InteropServices;
|
||||
using Avalonia.Controls;
|
||||
using Avalonia.Media;
|
||||
|
||||
namespace DodoSSH.Client.App.Views;
|
||||
|
||||
/// <summary>
|
||||
/// Paints the frame Windows still draws around a <c>BorderOnly</c> window in the application's own
|
||||
/// colour, so the top edge stops reading as a leftover system titlebar.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>The symptom this exists for:</b> a pale strip across the very top of the window, a few pixels
|
||||
/// tall and plainly not part of the application — most obvious on a machine with "show accent colour
|
||||
/// on title bars and window borders" turned on, where it comes out blue against a near-black shell.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It is not <c>TitleBar.axaml</c> leaking and it is not a margin. It is DWM, and the reason it is
|
||||
/// there is visible in Avalonia's own Win32 backend: <c>WindowImpl.UpdateWindowProperties</c> gives a
|
||||
/// <see cref="WindowDecorations.BorderOnly"/> window <c>WS_BORDER | WS_THICKFRAME</c> and then calls
|
||||
/// <c>DwmExtendFrameIntoClientArea</c> with one-pixel margins on all four sides. So the compositor
|
||||
/// owns a hairline of every edge of this window, and it fills that hairline with the system's caption
|
||||
/// and border colours — which are chosen by the user's personalisation settings and have no reason to
|
||||
/// resemble <c>CanvasColor</c>. The window is the wrong place to look for the pixels; they were never
|
||||
/// painted by anything in this tree.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The fix is to tell DWM what colour to use rather than to try to cover it. <c>DWMWA_BORDER_COLOR</c>
|
||||
/// and <c>DWMWA_CAPTION_COLOR</c> arrived in Windows 11 21H2 and are exactly that; both are set to the
|
||||
/// window's own background, so the hairline still exists — the resize grip is on it, and the drop
|
||||
/// shadow hangs off it — and simply cannot be seen. Deliberately <em>not</em> <c>DWMWA_COLOR_NONE</c>,
|
||||
/// which removes the border outright: on a dark desktop that leaves a near-black window with no edge
|
||||
/// at all, which trades one visual defect for another.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Windows 10 gets the dark-mode attribute and nothing else, and that is the whole of what is
|
||||
/// available there: the two colour attributes are unsupported, <c>DwmSetWindowAttribute</c> answers
|
||||
/// <c>E_INVALIDARG</c>, and the calls do nothing. Hence the ignored return values — every attribute
|
||||
/// here is an improvement where it lands and a no-op where it does not, so there is nothing for a
|
||||
/// caller to handle and nothing worth logging on a path that runs once at startup.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static class NativeWindowFrame
|
||||
{
|
||||
/// <summary>Windows 11 21H2 and later: the colour of the frame border.</summary>
|
||||
private const int BorderColorAttribute = 34;
|
||||
|
||||
/// <summary>Windows 11 21H2 and later: the colour of the caption, including the extended frame.</summary>
|
||||
private const int CaptionColorAttribute = 35;
|
||||
|
||||
/// <summary>
|
||||
/// Windows 10 1903 and later: draw the frame in the dark palette.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Redundant on Windows 11, where the two colour attributes above name the colours outright, and it
|
||||
/// is set anyway because it is the only one of the three that Windows 10 honours. The build before
|
||||
/// 1903 used attribute 19 for this; that is not chased here, because a border on an OS release that
|
||||
/// left support in 2020 is not worth a second interop call.
|
||||
/// </remarks>
|
||||
private const int DarkModeAttribute = 20;
|
||||
|
||||
/// <summary>
|
||||
/// Matches <paramref name="window"/>'s system-drawn frame to the colour it paints itself.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Call once the window has a handle — <c>OnOpened</c> is the first such moment. Calling earlier
|
||||
/// finds no platform handle and silently does nothing, which is the defect this replaced: the strip
|
||||
/// is only visible once the window is on screen, so a call that ran too early looks like a fix that
|
||||
/// does not work rather than a fix that never ran.
|
||||
/// </remarks>
|
||||
internal static void MatchTo(Window window)
|
||||
{
|
||||
// Every attribute below is a DWM one, and DWM is Windows. Elsewhere the frame is drawn by the
|
||||
// platform's own compositor and there is nothing here to say to it.
|
||||
if (!OperatingSystem.IsWindows())
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
if (window.TryGetPlatformHandle()?.Handle is not { } handle || handle == IntPtr.Zero)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
Set(handle, DarkModeAttribute, 1);
|
||||
|
||||
// The window's own Background rather than a named resource, so the frame cannot drift from the
|
||||
// canvas when the palette moves. A brush that is not solid — a gradient, or nothing set at all —
|
||||
// has no single colour to match, and leaving the system's own is better than inventing one.
|
||||
if (window.Background is not ISolidColorBrush { Color: var canvas })
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
var reference = ColorRef(canvas);
|
||||
Set(handle, BorderColorAttribute, reference);
|
||||
Set(handle, CaptionColorAttribute, reference);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sets one integer-valued DWM attribute, and discards the answer.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The discard is the point of this method existing rather than being three call sites. Every
|
||||
/// attribute here is unsupported on some Windows this application runs on, and unsupported means
|
||||
/// <c>E_INVALIDARG</c> and no change — which is the intended outcome on that OS, not a failure, so
|
||||
/// there is nothing for the caller to do with the <c>HRESULT</c> and nothing worth logging once at
|
||||
/// startup. Written once, with the reasoning, rather than left implicit at each call.
|
||||
/// </remarks>
|
||||
private static void Set(IntPtr window, int attribute, int value) =>
|
||||
_ = DwmSetWindowAttribute(window, attribute, ref value, sizeof(int));
|
||||
|
||||
/// <summary>
|
||||
/// Packs <paramref name="color"/> into a Win32 <c>COLORREF</c>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <c>0x00BBGGRR</c> — blue in the high byte, not red, and the alpha byte must be zero. Getting the
|
||||
/// order wrong produces a plausible-looking wrong colour rather than an error, which is the kind of
|
||||
/// bug that survives a glance at the window.
|
||||
/// </remarks>
|
||||
private static int ColorRef(Color color) => color.R | (color.G << 8) | (color.B << 16);
|
||||
|
||||
/// <remarks>
|
||||
/// <c>DllImport</c> rather than <c>LibraryImport</c>, for the reason
|
||||
/// <see cref="NativeKeyboardFocus"/> gives at its own P/Invoke: the generated form needs
|
||||
/// <c>AllowUnsafeBlocks</c> across a project that handles key material, and this signature is
|
||||
/// blittable, so there is no marshalling for it to improve.
|
||||
/// </remarks>
|
||||
#pragma warning disable SYSLIB1054
|
||||
[DllImport("dwmapi.dll")]
|
||||
private static extern int DwmSetWindowAttribute(IntPtr window, int attribute, ref int value, int size);
|
||||
#pragma warning restore SYSLIB1054
|
||||
}
|
||||
@@ -40,8 +40,9 @@
|
||||
Both are still one click away; see the popover below the user chip. The chip itself carries the signed-
|
||||
in identity this application actually has — a display name and, where the server sent one, an email —
|
||||
which is also new: the titlebar drew an account name and a vault chip before this pass and does not any
|
||||
more. See TitleBar.axaml and design-notes/v5b-fidelity-notes.md for the deviations this rail keeps on
|
||||
purpose: Pins, which the mock has no screen for at all, and the S3 segment above.
|
||||
more. See TitleBar.axaml and design-notes/v5b-fidelity-notes.md for the one deviation this rail still
|
||||
keeps on purpose: the S3 segment above. Pins was the other, and it is gone — see the remark where that
|
||||
row used to sit, between Keys and Snips.
|
||||
|
||||
Buttons rather than a TabStrip or a ListBox, still, for the reason the v3 remark gave: all three hold
|
||||
the selection themselves, so a click would move the highlight before the shell decided anything, and a
|
||||
@@ -128,25 +129,16 @@
|
||||
</Button>
|
||||
|
||||
<!--
|
||||
KEPT — the mock has no screen for approved host keys at all; see the file-level remark. push_pin
|
||||
is the same codepoint HostsScreen.axaml already draws for a host's own pin badge, reused rather
|
||||
than picked afresh so the one concept reads as one glyph everywhere it appears.
|
||||
◆ NO Pins ROW. The pins screen is still here and still reached in one click — from "Host keys"
|
||||
on the Keys screen's own header, which is where a list of approved host keys belongs: they are
|
||||
keychain material, and that button was already the second way to reach them. Two rail rows away
|
||||
from each other, both landing on the same screen, is a rail that has to be read twice.
|
||||
|
||||
◆ U+F10D, not U+E946, which both sites drew until this pass and which no glyph in the embedded
|
||||
face answers to: the cmap of Assets/Fonts/MaterialIcons (Material Icons 1.017, 2019) skips E944
|
||||
and E946, so this row and the hosts screen's own pin badge were both drawing a tofu box. F10D is
|
||||
where push_pin lives in that vintage, verified against the file rather than against a codepoints
|
||||
table for a later release of the font.
|
||||
It is also the last of the rail's own deviations from the mock to go. The row was kept in v5b on
|
||||
the grounds that the design has no screen for approved host keys at all — see the file-level
|
||||
remark — which is true of the design and was never a reason for a rail entry once the keychain
|
||||
had a door to the same place.
|
||||
-->
|
||||
<Button Classes="flat nav" Classes.active="{Binding IsKnownHostsShowing}"
|
||||
Command="{Binding ShowScreenCommand}"
|
||||
CommandParameter="{x:Static vm:ShellScreen.KnownHosts}"
|
||||
ToolTip.Tip="Host keys you have approved, and how to withdraw one">
|
||||
<StackPanel Orientation="Horizontal" Spacing="10">
|
||||
<TextBlock Classes="navicon" Text="" />
|
||||
<TextBlock Classes="navlabel" Text="Pins" />
|
||||
</StackPanel>
|
||||
</Button>
|
||||
|
||||
<Button Classes="flat nav" Classes.active="{Binding IsSnippetsShowing}"
|
||||
Command="{Binding ShowScreenCommand}"
|
||||
|
||||
@@ -74,16 +74,6 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
|
||||
/// </remarks>
|
||||
private static readonly TimeSpan CheckInterval = TimeSpan.FromHours(6);
|
||||
|
||||
/// <summary>How long to wait before the first pass.</summary>
|
||||
/// <remarks>
|
||||
/// A delay, where <c>VaultViewModel</c>'s sync loop runs a pass immediately. The difference is what the
|
||||
/// user is waiting for: a vault edited on another machine should be current by the time they have
|
||||
/// finished reading the list, whereas nothing anybody does in their first two minutes depends on an
|
||||
/// update. Launch is already contending for the network and the CPU with a schema migration, a resumed
|
||||
/// sign-in and a first sync, at the one moment somebody is watching the window.
|
||||
/// </remarks>
|
||||
private static readonly TimeSpan FirstCheckDelay = TimeSpan.FromMinutes(2);
|
||||
|
||||
private readonly IUpdateChannel updates;
|
||||
private readonly ClientSettingsStore settings;
|
||||
private readonly TimeProvider clock;
|
||||
@@ -242,16 +232,40 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab
|
||||
loop = RunCheckLoopAsync(lifetime.Token);
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// <b>The first pass runs at launch, with no delay in front of it.</b> It used to wait two minutes, on
|
||||
/// the argument that nothing anybody does in their first two minutes depends on an update and launch is
|
||||
/// already contending for the network with a schema migration, a resumed sign-in and a first sync. What
|
||||
/// that argument leaves out is the run that is over before the two minutes are: a client opened to reach
|
||||
/// one host and closed again never checks at all, and a machine used that way is exactly the one ADR
|
||||
/// 0011 warns about — quietly a year behind, with the mechanism to fix it switched on and never reached.
|
||||
/// Every start now asks.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>The yield is what keeps that off the launch path.</b> <see cref="Start"/> is called from
|
||||
/// <c>MainWindowViewModel.StartAsync</c> before the migration, so running the pass inline would put
|
||||
/// whatever the channel does before its own first await — Velopack reads the install layout from disk —
|
||||
/// between the user and their window. Yielding hands the rest of the launch back and lets the check run
|
||||
/// in a later turn, which is the same moment in every sense that matters and none of the cost.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private async Task RunCheckLoopAsync(CancellationToken cancellationToken)
|
||||
{
|
||||
try
|
||||
{
|
||||
await Task.Delay(FirstCheckDelay, clock, cancellationToken).ConfigureAwait(true);
|
||||
await Task.Yield();
|
||||
|
||||
using var timer = new PeriodicTimer(CheckInterval, clock);
|
||||
|
||||
do
|
||||
{
|
||||
// Task.Yield takes no token, unlike the delay it replaced, so a shutdown that lands while
|
||||
// the loop is waiting to be handed back the thread has to be observed here rather than
|
||||
// only at the next tick. Otherwise an application closed during launch spends its last
|
||||
// moment asking a release channel about a build it is not going to run.
|
||||
cancellationToken.ThrowIfCancellationRequested();
|
||||
|
||||
await CheckOnceAsync(cancellationToken).ConfigureAwait(true);
|
||||
}
|
||||
while (await timer.WaitForNextTickAsync(cancellationToken).ConfigureAwait(true));
|
||||
|
||||
@@ -11247,10 +11247,7 @@ internal sealed partial class VaultViewModel(
|
||||
}
|
||||
catch (TimeoutException)
|
||||
{
|
||||
Abandon(
|
||||
attempt,
|
||||
"The terminal did not start, so nothing was connected. The Microsoft Edge WebView2 "
|
||||
+ "runtime is probably missing or blocked; install it and try again.");
|
||||
Abandon(attempt, RendererNeverStarted);
|
||||
}
|
||||
catch (SshHostKeyUnknownException exception)
|
||||
{
|
||||
@@ -11275,6 +11272,29 @@ internal sealed partial class VaultViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>What a renderer that never attached is reported as.</summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The wait is translated rather than reported for the reason <see cref="OpenSessionAsync"/> gives —
|
||||
/// <see cref="TimeoutException"/> says only "The operation has timed out" — and the whole value of the
|
||||
/// translation is naming where to look. Which is why it cannot be one sentence: the desktop's answer is
|
||||
/// a runtime this application does not install, and the phone has no such runtime and no such answer.
|
||||
/// Telling somebody on a handset to install Microsoft Edge WebView2 is worse than saying nothing, at the
|
||||
/// one moment they are trying to work out what went wrong.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// A runtime check rather than a constructor parameter, for the reason
|
||||
/// <c>MainWindowViewModel.GestureWait</c> records at length: which renderer is behind the terminal is a
|
||||
/// fact about the platform this assembly is running on, not about one installation of it.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private static string RendererNeverStarted =>
|
||||
OperatingSystem.IsAndroid()
|
||||
? "The terminal did not start, so nothing was connected. Android's WebView is probably "
|
||||
+ "disabled or updating; check it in Settings and try again."
|
||||
: "The terminal did not start, so nothing was connected. The Microsoft Edge WebView2 "
|
||||
+ "runtime is probably missing or blocked; install it and try again.";
|
||||
|
||||
/// <summary>Says, in one place, that an attempt ended without a session and why.</summary>
|
||||
/// <remarks>
|
||||
/// The reason goes to two places on purpose. The status line is where somebody watching this screen is
|
||||
|
||||
@@ -84,14 +84,57 @@ const RELEASE_FOCUS_MESSAGE = 'dodossh.release-focus';
|
||||
const root = document.getElementById('root');
|
||||
const statusBanner = document.getElementById('status');
|
||||
|
||||
/** @type {Map<number, {term: object, fit: object, pane: HTMLElement}>} */
|
||||
/** @type {Map<number, {term: object, fit: object, pane: HTMLElement, notice: string}>} */
|
||||
const sessions = new Map();
|
||||
|
||||
/** @type {WebSocket | null} */
|
||||
let socket = null;
|
||||
|
||||
function setStatus(text) {
|
||||
statusBanner.textContent = text ?? '';
|
||||
/** Whose pane is showing, or null before there is one — see activate(). */
|
||||
let activeSessionId = null;
|
||||
|
||||
/*
|
||||
── THE BANNER BELONGS TO ONE PANE AT A TIME ─────────────────────────────────────────────────────────
|
||||
There is one #status element for the whole page, because there is one page for every terminal: the
|
||||
panes are stacked in the same box and all but the active one are hidden. What goes in it comes from
|
||||
two sources that are not the same size, and the difference is the whole of this.
|
||||
|
||||
The socket's troubles are the page's. There is a single socket behind every pane, so "the view is
|
||||
reconnecting" is true of whatever is on screen and true of the panes behind it.
|
||||
|
||||
A session's last words are not. "The remote closed the session." is a fact about one terminal and says
|
||||
nothing whatever about the others — so it is held on the session and drawn only while that session's
|
||||
pane is the one showing. Written straight into the shared element, which is what this used to do, it
|
||||
outlived the tab it described: switching to a live terminal left the dead one's epitaph sitting under
|
||||
it, and opening or closing any other tab wiped the message whether or not it belonged to that tab.
|
||||
|
||||
The socket's half wins when both have something to say: a page whose socket is down is not showing
|
||||
live output on any pane, which makes what became of one session the less urgent of the two.
|
||||
*/
|
||||
let transportStatus = statusBanner.textContent ?? '';
|
||||
|
||||
function renderStatus() {
|
||||
const notice = activeSessionId === null ? '' : sessions.get(activeSessionId)?.notice ?? '';
|
||||
|
||||
statusBanner.textContent = transportStatus || notice;
|
||||
}
|
||||
|
||||
/** Says something about the socket, which every pane shares. */
|
||||
function setTransportStatus(text) {
|
||||
transportStatus = text ?? '';
|
||||
renderStatus();
|
||||
}
|
||||
|
||||
/** Records what became of one session, to be drawn only while that session's pane is showing. */
|
||||
function setSessionNotice(sessionId, text) {
|
||||
const session = sessions.get(sessionId);
|
||||
|
||||
if (!session) {
|
||||
return;
|
||||
}
|
||||
|
||||
session.notice = text ?? '';
|
||||
renderStatus();
|
||||
}
|
||||
|
||||
/** Builds a frame: opcode, big-endian session id, then payload. */
|
||||
@@ -255,8 +298,31 @@ function createSession(sessionId) {
|
||||
// WebGL where it is available. Falling back rather than failing matters because a software
|
||||
// renderer is slow but usable, whereas a blank pane is not — and remote desktops and VMs
|
||||
// routinely have no usable GPU context.
|
||||
//
|
||||
// ◆ THE CONTEXT-LOSS HANDLER IS THE HALF THAT WAS MISSING, AND ON A PHONE IT IS THE WHOLE THING.
|
||||
//
|
||||
// The addon does not recover from a lost GPU context by itself, and it does not fail loudly either:
|
||||
// it stays loaded over a dead context and draws nothing at all. What that looks like from outside is
|
||||
// a terminal that is connected, still accepting keystrokes, still acknowledging output — and blank.
|
||||
// xterm's own guidance is to dispose the addon and let the DOM renderer take over, which is what this
|
||||
// does; the addon is not reloaded afterwards, because a pane that lost the context once is on a
|
||||
// surface that will do it again and thrashing between renderers is worse than being slow.
|
||||
//
|
||||
// Losing it is ordinary on Android and nearly unheard of on Windows, which is why this went unnoticed
|
||||
// for so long. Collapsing the renderer sets the native view to GONE — see
|
||||
// AndroidNativeControlHostImpl.HideWithSize — and a WebView with no surface has no GL context. The
|
||||
// shell collapses it every time a tab starts connecting, every time the connect sheet opens and every
|
||||
// time the app is backgrounded, so on a phone the first loss arrives within seconds of the first
|
||||
// session. WebView2 hides a child HWND instead and keeps rendering throughout; see
|
||||
// docs/platform-flags.md.
|
||||
try {
|
||||
term.loadAddon(new WebglAddon.WebglAddon());
|
||||
const webgl = new WebglAddon.WebglAddon();
|
||||
|
||||
// Subscribed before loadAddon, because loadAddon is what activates the addon and a context that is
|
||||
// already gone can be reported from inside that call.
|
||||
webgl.onContextLoss(() => webgl.dispose());
|
||||
|
||||
term.loadAddon(webgl);
|
||||
} catch (error) {
|
||||
console.warn('WebGL renderer unavailable; falling back to canvas.', error);
|
||||
}
|
||||
@@ -269,7 +335,7 @@ function createSession(sessionId) {
|
||||
|
||||
term.onResize(() => sendResize(sessionId, term, pane));
|
||||
|
||||
const session = { term, fit, pane };
|
||||
const session = { term, fit, pane, notice: '' };
|
||||
sessions.set(sessionId, session);
|
||||
|
||||
activate(sessionId);
|
||||
@@ -283,6 +349,11 @@ function activate(sessionId) {
|
||||
session.pane.dataset.active = String(id === sessionId);
|
||||
}
|
||||
|
||||
// The banner follows the pane. Whatever this session has to say for itself replaces whatever the
|
||||
// session that was showing had to say for its own, which is the point of holding it per session.
|
||||
activeSessionId = sessionId;
|
||||
renderStatus();
|
||||
|
||||
const active = sessions.get(sessionId);
|
||||
if (active) {
|
||||
active.term.focus();
|
||||
@@ -296,10 +367,17 @@ function activate(sessionId) {
|
||||
// caller, because more than one path reaches here: a minimised window, and a splitter dragged to the edge
|
||||
// once splits land.
|
||||
//
|
||||
// It is *not* what protects the vault's lock screen, which an earlier version of this comment claimed.
|
||||
// Collapsing the host's WebView hides a native child window without resizing it, so this page's viewport
|
||||
// does not change, no observer fires and this function is never called — measured with a live shell, and
|
||||
// confirmed by removing the guard and finding the lock cycle equally clean. See docs/platform-flags.md.
|
||||
// It is *not* what protects the vault's lock screen on the desktop, which an earlier version of this
|
||||
// comment claimed. Collapsing WebView2 hides a native child window without resizing it, so this page's
|
||||
// viewport does not change, no observer fires and this function is never called — measured with a live
|
||||
// shell, and confirmed by removing the guard and finding the lock cycle equally clean. See
|
||||
// docs/platform-flags.md.
|
||||
//
|
||||
// On the phone it *is* load-bearing, and that is the one place the two heads differ here. Android hides a
|
||||
// native child by setting it GONE, and a GONE view is skipped by its parent's layout — so collapsing the
|
||||
// renderer really does take this page's viewport to nothing, the observer really does fire, and without
|
||||
// the guard every lock, every connect sheet and every trip to the background would reflow the remote pty
|
||||
// to 2x1 and mangle the scrollback it wrapped.
|
||||
const MINIMUM_FITTABLE_PIXELS = 40;
|
||||
|
||||
function resize(session, sessionId) {
|
||||
@@ -343,7 +421,10 @@ function handleFrame(buffer) {
|
||||
session.term.write(REPLAY_BANNER);
|
||||
}
|
||||
|
||||
setStatus('');
|
||||
// This session's own line, and only this one's: a session that is open has nothing to say about
|
||||
// how it ended. The page's own "Connecting…" is cleared by the socket opening, which happens
|
||||
// before any frame can arrive.
|
||||
setSessionNotice(sessionId, '');
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -396,7 +477,14 @@ function handleFrame(buffer) {
|
||||
session.pane.remove();
|
||||
sessions.delete(sessionId);
|
||||
|
||||
setStatus('');
|
||||
// The notice went with the session record it was held on, but the page can still be pointing at
|
||||
// the pane that is now gone. Cleared rather than left dangling, so the banner stops describing a
|
||||
// closed tab while the host decides which pane to show next.
|
||||
if (activeSessionId === sessionId) {
|
||||
activeSessionId = null;
|
||||
}
|
||||
|
||||
renderStatus();
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -473,14 +561,19 @@ function handleFrame(buffer) {
|
||||
const session = sessions.get(sessionId);
|
||||
const reason = new TextDecoder().decode(payload);
|
||||
|
||||
if (session) {
|
||||
if (!session) {
|
||||
// No pane, so there is nothing this page can honestly hang the reason on. It used to go into
|
||||
// the banner anyway, which printed one session's ending underneath whichever pane happened to
|
||||
// be showing at the time.
|
||||
break;
|
||||
}
|
||||
|
||||
// The pane and its scrollback stay. The user was probably reading the last thing the
|
||||
// remote said, and that is usually why the session ended.
|
||||
session.term.write(`\r\n\x1b[38;5;244m── ${reason} ──\x1b[0m\r\n`);
|
||||
session.term.options.cursorBlink = false;
|
||||
}
|
||||
|
||||
setStatus(reason);
|
||||
setSessionNotice(sessionId, reason);
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -505,7 +598,7 @@ function scheduleReconnect() {
|
||||
return;
|
||||
}
|
||||
|
||||
setStatus('Reconnecting the terminal view…');
|
||||
setTransportStatus('Reconnecting the terminal view…');
|
||||
|
||||
reconnectTimer = setTimeout(() => {
|
||||
reconnectTimer = null;
|
||||
@@ -525,7 +618,7 @@ function connect() {
|
||||
socket.binaryType = 'arraybuffer';
|
||||
|
||||
socket.addEventListener('open', () => {
|
||||
setStatus('');
|
||||
setTransportStatus('');
|
||||
|
||||
// Back to the quick attempt for whatever the next failure turns out to be. Kept slow between
|
||||
// attempts within one outage, reset once the outage is actually over.
|
||||
|
||||
@@ -22,9 +22,9 @@
|
||||
},
|
||||
"Microsoft.NET.ILLink.Tasks": {
|
||||
"type": "Direct",
|
||||
"requested": "[10.0.10, )",
|
||||
"resolved": "10.0.10",
|
||||
"contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg=="
|
||||
"requested": "[10.0.11, )",
|
||||
"resolved": "10.0.11",
|
||||
"contentHash": "IBf7lbovvjGWVWXZX5cJ/cO0WXbId0Zq4BuSeT94mGZuOAP66oMeH9PTBZ9Jpp3Jb6jtK0qm/NyUbPRo1gC/wQ=="
|
||||
},
|
||||
"MinVer": {
|
||||
"type": "Direct",
|
||||
|
||||
@@ -16,9 +16,9 @@
|
||||
},
|
||||
"Microsoft.NET.ILLink.Tasks": {
|
||||
"type": "Direct",
|
||||
"requested": "[10.0.10, )",
|
||||
"resolved": "10.0.10",
|
||||
"contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg=="
|
||||
"requested": "[10.0.11, )",
|
||||
"resolved": "10.0.11",
|
||||
"contentHash": "IBf7lbovvjGWVWXZX5cJ/cO0WXbId0Zq4BuSeT94mGZuOAP66oMeH9PTBZ9Jpp3Jb6jtK0qm/NyUbPRo1gC/wQ=="
|
||||
},
|
||||
"MinVer": {
|
||||
"type": "Direct",
|
||||
|
||||
@@ -1596,7 +1596,7 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// v5b's redraw changes what this test has to hold. Three button shapes live in the rail now rather
|
||||
/// than one: the switcher's three segments, each a third of the rail's own content width; the six item
|
||||
/// than one: the switcher's three segments, each a third of the rail's own content width; the five item
|
||||
/// rows below it and the user chip at the foot, both the rail's full content width. A single
|
||||
/// across-the-board width assertion the way the v3 version of this test made one would either be wrong
|
||||
/// for the segments or have to loosen until it caught nothing, so each shape gets its own count and its
|
||||
@@ -1604,13 +1604,18 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The rail runs vertically, so what runs out at the window's minimum is still height — a switcher plus
|
||||
/// six rows plus a user chip have to leave room for each other in the same space the v3 rail's seven
|
||||
/// five rows plus a user chip have to leave room for each other in the same space the v3 rail's seven
|
||||
/// plain rows did. Both counts are asserted in both directions for the reason the old test's was: an
|
||||
/// entry silently dropping off the bottom would still pass every other assertion here.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Five and not six since Pins left the rail: the pins screen is reached from "Host keys" on the Keys
|
||||
/// screen, which was always the other way in. Exact rather than a bound, so putting a row back is a
|
||||
/// decision somebody makes here rather than something that slips in.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task TheNavRailHoldsItsSwitcherSixDestinationsAndTheUserChipAtTheWindowsMinimum()
|
||||
public async Task TheNavRailHoldsItsSwitcherFiveDestinationsAndTheUserChipAtTheWindowsMinimum()
|
||||
{
|
||||
await LayoutHarness.OnTheUiThreadAsync(
|
||||
() =>
|
||||
@@ -1630,7 +1635,7 @@ public sealed class ScreenLayoutTests : IAsyncLifetime
|
||||
|
||||
segments.Count.ShouldBe(3, "SSH, SFTP and S3");
|
||||
rows.Count.ShouldBe(
|
||||
6, "the mode-dependent first row, then Hosts, Keys, Pins, Snips and Logs");
|
||||
5, "the mode-dependent first row, then Hosts, Keys, Snips and Logs");
|
||||
|
||||
foreach (var segment in segments)
|
||||
{
|
||||
|
||||
@@ -403,6 +403,46 @@ public sealed class UpdateFlowTests : IDisposable
|
||||
channel.Checks.ShouldBe(1);
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// The loop rather than <c>CheckOnceAsync</c>, which is the one thing the rest of this file avoids
|
||||
/// driving — and here it is the whole point, because the claim is about when the first pass happens
|
||||
/// rather than about what it does. The first pass used to wait two minutes, which meant a client opened
|
||||
/// to reach one host and closed again never asked at all.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It waits on the pass and not on a clock, so there is nothing here to be flaky about: a regression
|
||||
/// that puts a delay back in front of the loop does not fail on a margin, it spins until the suite's own
|
||||
/// cancellation ends it.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task TheFirstPassRunsAtStart_RatherThanOnADelay()
|
||||
{
|
||||
channel.Available = new AvailableUpdate("1.3.0");
|
||||
|
||||
var updates = Build();
|
||||
await using var _ = updates.ConfigureAwait(false);
|
||||
|
||||
updates.Start();
|
||||
|
||||
while (updates.State is not UpdateState.Ready)
|
||||
{
|
||||
Token.ThrowIfCancellationRequested();
|
||||
|
||||
await Task.Yield();
|
||||
}
|
||||
|
||||
channel.Checks.ShouldBe(1);
|
||||
updates.ReadyVersion.ShouldBe("1.3.0");
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Started and disposed with nothing in between, which since the first pass stopped waiting two minutes
|
||||
/// is a race rather than a formality: the loop may be anywhere between its yield and a finished check
|
||||
/// when the cancellation lands. What is asserted is what matters either way — that disposing returns,
|
||||
/// rather than waiting on a pass that will never be allowed to finish.
|
||||
/// </remarks>
|
||||
[Fact]
|
||||
public async Task DisposingStopsTheLoop()
|
||||
{
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
using System.Net.Sockets;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
using DotNet.Testcontainers.Builders;
|
||||
using DotNet.Testcontainers.Containers;
|
||||
using Xunit;
|
||||
@@ -40,6 +42,21 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
|
||||
private const int SshPort = 2222;
|
||||
|
||||
/// <summary>
|
||||
/// How many connections in a row the server has to answer before this fixture calls it ready.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Twenty-five, and the number is measured rather than picked. Probing a fresh container 200 times with
|
||||
/// penalties left at the image's default, the first <c>Not allowed at this time</c> came back at probe
|
||||
/// 18 and 183 of the 200 were refused; with <c>PerSourcePenalties no</c> applied, none of 200 were. Ten
|
||||
/// was tried first and is useless — it sits below the threshold, so the guard passed happily against a
|
||||
/// server that was still penalising. See <see cref="WaitUntilServingAsync"/>.
|
||||
/// </remarks>
|
||||
private const int RequiredStreak = 25;
|
||||
|
||||
/// <summary>How long to keep trying before giving up on the server entirely.</summary>
|
||||
private static readonly TimeSpan ReadyTimeout = TimeSpan.FromSeconds(60);
|
||||
|
||||
private readonly SemaphoreSlim sftpGate = new(1, 1);
|
||||
|
||||
private IContainer? container;
|
||||
@@ -80,101 +97,94 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
.Build();
|
||||
|
||||
await container.StartAsync();
|
||||
await AllowTcpForwardingAsync();
|
||||
await ReconfigureAsync();
|
||||
await WaitUntilServingAsync();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Lets this server open the direct-tcpip channels a forward is made of.
|
||||
/// Turns off the hardening this suite trips over, and makes the running server re-read its config.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ◆ <b>The image ships <c>AllowTcpForwarding no</c>, and nothing says so at the point it bites.</b> A
|
||||
/// dynamic forward starts perfectly happily — it is a local listener, and opening it asks the server
|
||||
/// nothing — and then every connection through it is refused when the channel is opened. SSH.NET
|
||||
/// reports that as <c>SOCKS5: General failure</c> from the proxy, which names neither the server nor
|
||||
/// the setting, and is what the first run of <c>LoopbackProxyTests</c> collected.
|
||||
/// ◆ <b><c>PerSourcePenalties no</c> is the fix for the flake this suite had for months, and the other
|
||||
/// two settings here are not.</b> OpenSSH 9.8 added per-source penalties and 10.x has them on by
|
||||
/// default; this image runs 10.3. A source address that keeps disconnecting without authenticating is
|
||||
/// penalised, and while the penalty holds every connection from it is answered with the clear-text line
|
||||
/// <c>Not allowed at this time</c> and then closed.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Patched after start rather than baked in, because the image's entrypoint writes its configuration
|
||||
/// itself on every boot — a mounted file would be overwritten before sshd read it. sshd re-reads on
|
||||
/// <c>SIGHUP</c> and applies the result to connections made after that, and the readiness wait has
|
||||
/// already run, so nothing here races the boot.
|
||||
/// <b>This suite generates exactly that traffic, by design.</b> This client's first contact with an
|
||||
/// unknown host is a connection deliberately refused at the host key — which is a disconnect with no
|
||||
/// authentication attempt — and several tests do nothing else:
|
||||
/// <c>RefusingTheHostKey_AbortsTheConnection</c>, <c>AnUntrustedHost_IsRefusedExactlyAsAShellWouldBe</c>,
|
||||
/// and every helper that learns a host key by being turned away first. Enough of them close together and
|
||||
/// sshd stops talking to the test host altogether, for a while, and then starts again.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// From the client that is <c>SshConnectionException: The connection was closed by the remote host</c>
|
||||
/// within milliseconds — no banner, nothing to say which of the many reasons it was. It hits whichever
|
||||
/// class is running when the penalty lands and spares the rest, which is why it read as random and why
|
||||
/// the class it hit lost <em>every</em> connection it made rather than a random few. The one test in that
|
||||
/// class that expects a refusal passed throughout, for the wrong reason.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b>Two earlier diagnoses were wrong, and are recorded here so they are not tried again.</b>
|
||||
/// <c>MaxStartups</c> was blamed on the reasoning that xUnit runs test classes in parallel, so ten
|
||||
/// unauthenticated connections would be in flight at once — but every class that touches this server
|
||||
/// shares <see cref="SshCollection"/>, and xUnit's unit of parallelism is the collection, so they run one
|
||||
/// after another and never have more than a connection or two open. The reload window was blamed next,
|
||||
/// and a wait for the banner to answer was written and removed as unproven; it was unproven because the
|
||||
/// banner does answer, right up until the penalty lands.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// The line is appended rather than replaced in place, unlike the two below it, because the image's
|
||||
/// config does not mention the keyword at all — there is no line to replace, and sshd takes the first
|
||||
/// value it finds for a keyword that appears more than once.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b><c>AllowTcpForwarding</c> is what a dynamic forward needs</b>, and the image ships it off as
|
||||
/// hardening. Without it a forward opens perfectly happily — a local listener asks the server nothing —
|
||||
/// and then every connection through it is refused when the channel is opened. SSH.NET reports that as
|
||||
/// <c>SOCKS5: General failure</c>, which names neither the server nor the setting, and is what the first
|
||||
/// run of <c>LoopbackProxyTests</c> collected. That suite is also the alarm if this method ever silently
|
||||
/// stops working.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <c>MaxStartups</c> is raised for the reason it should have been in the first place rather than as a
|
||||
/// fix for anything: the compiled-in default refuses connections at random past ten unauthenticated ones
|
||||
/// in flight, and a throttle is hardening a test server has no business reproducing. It is kept, not
|
||||
/// because it was ever shown to matter here, but because removing it would be a second change riding
|
||||
/// along with this one.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Both are replaced in place rather than appended, because sshd_config takes the <em>first</em> value
|
||||
/// it finds for a keyword: an appended line would be dead the day the image ships an uncommented one of
|
||||
/// its own.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b><c>/config/sshd/sshd_config</c>, and there are two.</b> The image also carries
|
||||
/// <c>/etc/ssh/sshd_config</c>, which looks like the file to patch, reads identically, and is not the
|
||||
/// one the running server was started with — patching it changes the text and nothing else, which is a
|
||||
/// fix that appears to work and leaves the failure exactly where it was. Measured with <c>find</c>
|
||||
/// rather than assumed, after the first version of this method did precisely that.
|
||||
/// <c>/etc/ssh/sshd_config</c>, which looks like the file to patch, reads identically, and is not the one
|
||||
/// the running server was started with — patching it changes the text and nothing else, which is a fix
|
||||
/// that appears to work and leaves the failure exactly where it was.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It is on for the whole assembly rather than for the one test that needs it. Forwarding is off in
|
||||
/// this image as hardening, not as a behaviour worth reproducing: nothing else here opens a channel of
|
||||
/// any kind, so allowing it changes what exactly one suite can do and what none of the others see.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b><c>MaxStartups</c> is raised here too, against a flake this suite has and that this change is
|
||||
/// a mitigation for rather than a proven cure.</b> The distinction is stated because the evidence
|
||||
/// stops short of the claim, and a later reader deserves to know which.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// What is established: sshd's compiled-in default is <c>10:30:100</c> — past ten
|
||||
/// <em>unauthenticated</em> connections in flight it refuses new ones at random, thirty percent of the
|
||||
/// time, rising to always at a hundred — and the image ships the line commented out, so that default
|
||||
/// was what ran. xUnit runs test classes in parallel and most classes here open a connection, so ten
|
||||
/// in flight is reachable in the opening seconds. A refused connection presents to the client as
|
||||
/// <c>SshConnectionException: The connection was closed by the remote host</c> within tens of
|
||||
/// milliseconds, on whichever test connects at the wrong moment — which is exactly the observed
|
||||
/// failure, seen in CI and reproduced locally.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// What is <em>not</em> established is that this limit is the only cause, because the flake rate could
|
||||
/// not be measured reliably. On the development machine the identical unmodified suite ran 85/85 clean
|
||||
/// and, an hour later, failed 13 runs out of 15 — Docker throughput on that host swings far enough to
|
||||
/// swamp the effect being measured. Any before/after comparison taken there is noise, and two were,
|
||||
/// before that was noticed.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It is committed anyway, on the narrower argument that it is right regardless: a connection throttle
|
||||
/// is hardening this suite has no interest in reproducing. It exists to test an SSH client, not to
|
||||
/// survive a rate limit, and a test server that drops connections at random is a bad test server
|
||||
/// whether or not it is the cause of this particular flake.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Not fixed by serialising the suite</b>, which would have hidden it and cost the parallelism, and
|
||||
/// not by retrying the connect, which would have made the client's own reconnect behaviour untestable
|
||||
/// by burying it in the fixture. The limit is a property of a hardened server that this suite has no
|
||||
/// interest in reproducing — it exists to test an SSH client, not to survive a throttle.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Replaced in place rather than appended, because sshd_config takes the <em>first</em> value it finds
|
||||
/// for a keyword: an appended line would be dead the day the image ships an uncommented one of its own.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// ◆ <b>The reload window is the other candidate, and it is deliberately not guarded against.</b>
|
||||
/// <c>SIGHUP</c> makes sshd close its listeners and re-execute itself, and <c>pkill</c> returns when
|
||||
/// the signal is delivered rather than when that has finished — so in principle a connection made
|
||||
/// immediately afterwards is refused, producing this same exception. A wait that opened connections
|
||||
/// until the server answered with its banner three times running was written, and then removed: it
|
||||
/// could not be shown to change anything either, and a fixture carrying two unproven fixes for one
|
||||
/// symptom is worse than one, because the next person has to disprove both.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// If this flake returns, that is the next thing to try. Two things to know before trying it: the two
|
||||
/// causes are indistinguishable from the client, so a fix can only be judged by a repeat run and never
|
||||
/// by whether the next run passes — and the repeat run has to happen somewhere with stable Docker
|
||||
/// throughput, which the development machine is not. Better still, make sshd say why: raise its
|
||||
/// <c>LogLevel</c> here, disable Ryuk so the container outlives the run, and read
|
||||
/// <c>docker logs</c>. A <c>MaxStartups</c> refusal names itself there; a reload does not.
|
||||
/// ◆ <b>Patched after boot and reloaded, rather than injected before it — which was tried and does not
|
||||
/// work.</b> This image family runs <c>/custom-cont-init.d</c> scripts, which look like the right hook
|
||||
/// and are not: the container's own log puts <c>sshd is listening on port 2222</c> <em>before</em>
|
||||
/// <c>[custom-init] Files found, executing</c>, so a script there edits a file the running server has
|
||||
/// already read. It leaves a config that greps correctly and a server behaving as though it had never
|
||||
/// 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>
|
||||
/// </remarks>
|
||||
private async Task AllowTcpForwardingAsync()
|
||||
private async Task ReconfigureAsync()
|
||||
{
|
||||
var result = await container!.ExecAsync([
|
||||
"sh",
|
||||
"-c",
|
||||
"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"
|
||||
+ " && pkill -HUP sshd",
|
||||
]);
|
||||
|
||||
@@ -183,7 +193,107 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
throw new InvalidOperationException(
|
||||
$"Could not reconfigure the test server: {result.Stderr}");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Blocks until the server answers <see cref="RequiredStreak"/> connections in a row with its banner.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ◆ <b>This is a guard rather than a wait, and what it guards against is
|
||||
/// <c>PerSourcePenalties</c> coming back.</b> Reconfiguring above turns it off; this proves it is off,
|
||||
/// immediately and by name, instead of letting the suite discover it later as an unrelated-looking
|
||||
/// failure in whichever class happened to be running.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Consecutive, and deliberately with no pause between them.</b> Each probe opens a connection, reads
|
||||
/// the identification string and disconnects without authenticating — which is exactly the shape of
|
||||
/// connection <c>PerSourcePenalties</c> punishes, and exactly what this suite does all day: a first
|
||||
/// contact with an unknown host is a connection this client deliberately refuses at the host key.
|
||||
/// <see cref="RequiredStreak"/> back to back is therefore not a soak test, it is the specific
|
||||
/// provocation, sized above the measured threshold on purpose, and it costs well under a second when the
|
||||
/// setting is off.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It is also the one check that can tell a listening socket from a running server. The container's own
|
||||
/// readiness — a log line and <c>netstat</c> showing <c>:2222</c> — passes on a container whose sshd has
|
||||
/// gone: the socket is published by a host-side proxy that accepts before it has anything to forward to,
|
||||
/// so a dead server presents as a connection accepted and closed rather than as one refused.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Probed from the host rather than with <c>docker exec</c>, deliberately: that is the path the tests
|
||||
/// take, proxy included, and penalties are counted per source address — from inside the container the
|
||||
/// source would be the loopback rather than the address every test connects from.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
private async Task WaitUntilServingAsync()
|
||||
{
|
||||
// TimeProvider.System rather than DateTimeOffset.UtcNow, which this repository bans so that time can
|
||||
// be faked — and rather than a fake, because what is being waited on is a real container starting.
|
||||
var deadline = TimeProvider.System.GetUtcNow() + ReadyTimeout;
|
||||
var streak = 0;
|
||||
var last = "no probe ran";
|
||||
|
||||
while (streak < RequiredStreak)
|
||||
{
|
||||
if (TimeProvider.System.GetUtcNow() >= deadline)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"The test server did not answer {RequiredStreak} connections in a row within "
|
||||
+ $"{ReadyTimeout}. The last probe said: {last}. If it says \"Not allowed at this "
|
||||
+ "time\", sshd is penalising this source address and PerSourcePenalties is no longer "
|
||||
+ "being turned off — see ReconfigureAsync.");
|
||||
}
|
||||
|
||||
var (answered, what) = await ProbeAsync();
|
||||
last = what;
|
||||
|
||||
if (answered)
|
||||
{
|
||||
streak++;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only pause when it is not working. Back-to-back probes are the point while they succeed;
|
||||
// hammering a server that has not finished starting is just noise.
|
||||
streak = 0;
|
||||
await Task.Delay(TimeSpan.FromMilliseconds(200));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Opens a socket and reads far enough to see OpenSSH's identification string.</summary>
|
||||
/// <remarks>
|
||||
/// The description comes back with the answer because the interesting failures are not exceptions. A
|
||||
/// penalised source is told <c>Not allowed at this time</c> in clear text before the socket closes, and
|
||||
/// a suite that only knew "no banner" would have to go and find that out again — which is what happened
|
||||
/// the first time, at some length.
|
||||
/// </remarks>
|
||||
private async Task<(bool Answered, string What)> ProbeAsync()
|
||||
{
|
||||
try
|
||||
{
|
||||
using var probe = new TcpClient();
|
||||
using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(5));
|
||||
|
||||
await probe.ConnectAsync(Host, Port, timeout.Token);
|
||||
|
||||
var buffer = new byte[64];
|
||||
var read = await probe.GetStream().ReadAtLeastAsync(
|
||||
buffer, 4, throwOnEndOfStream: false, timeout.Token);
|
||||
|
||||
var answered = read >= 4 && "SSH-"u8.SequenceEqual(buffer.AsSpan(0, 4));
|
||||
|
||||
return (
|
||||
answered,
|
||||
answered
|
||||
? "SSH-"
|
||||
: $"{read} bytes: "
|
||||
+ Encoding.ASCII.GetString(buffer, 0, Math.Max(read, 0)).ReplaceLineEndings(" "));
|
||||
}
|
||||
catch (Exception exception) when (exception is SocketException or OperationCanceledException or IOException)
|
||||
{
|
||||
return (false, $"{exception.GetType().Name}: {exception.Message}");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -225,12 +335,17 @@ public sealed class SshServerFixture : IAsyncLifetime
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Shared rather than opened per test, and that is a limit of the server rather than an optimisation.
|
||||
/// sshd's <c>MaxStartups</c> drops connections at random once enough are part-way through a handshake,
|
||||
/// and this client's first contact with an unknown host is a connection deliberately <em>refused</em> at
|
||||
/// the host key — so a suite that opened its own session per test made two handshakes per test and
|
||||
/// pushed the whole assembly over the threshold. What that looks like is unrelated tests failing with
|
||||
/// "the connection was closed by the remote host", a different few each run.
|
||||
/// Shared rather than opened per test. This was once explained as a way of staying under the server's
|
||||
/// <c>MaxStartups</c> throttle, on the belief that the suite ran its classes in parallel and made two
|
||||
/// handshakes per test — this client's first contact with an unknown host is a connection deliberately
|
||||
/// <em>refused</em> at the host key, so every session costs two. The parallelism was not real: every
|
||||
/// class here shares one collection and xUnit runs collections, not classes, in parallel. See
|
||||
/// <see cref="WaitUntilServingAsync"/>, which is where that mistake was found and what the failure it
|
||||
/// was blamed for turned out to be.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// It stays shared regardless, on the plainer argument: one session is enough, and a handshake per test
|
||||
/// would be seconds of the suite's runtime spent proving nothing this file has not already proved.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Safe to share because an SFTP session holds no per-test state: every test here works in a directory
|
||||
|
||||
Reference in New Issue
Block a user