diff --git a/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs b/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs
index 5bb88ef..ad0247c 100644
--- a/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs
+++ b/src/DodoSSH.Client.Terminal/TerminalDataPlane.cs
@@ -56,6 +56,16 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
private const int MaximumRequestBytes = 16 * 1024;
private const int ReceiveBufferBytes = 64 * 1024;
+ /// The largest message from the page this host will reassemble.
+ ///
+ /// A paste is one input frame however long it is, and so is a copied selection, so the receive buffer
+ /// is the size of a read and not the size of a message. Sixteen megabytes is far beyond anything a
+ /// person pastes into a shell on purpose and still small enough that a page sending garbage cannot make
+ /// this process hold an unbounded amount of it. A message beyond it is dropped whole — never its tail,
+ /// which would be parsed as a frame whose first bytes are somebody's paste.
+ ///
+ private const int MaximumMessageBytes = 16 * 1024 * 1024;
+
private readonly TcpListener listener;
private readonly ITerminalAssetProvider assets;
private readonly Dictionary pumps = [];
@@ -451,6 +461,11 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
{
var buffer = new byte[ReceiveBufferBytes];
+ // Grown only when a message spans more than one read, and kept for the next one that does.
+ var message = Array.Empty();
+ var length = 0;
+ var oversized = false;
+
while (webSocket.State == WebSocketState.Open && !cancellationToken.IsCancellationRequested)
{
var result = await webSocket
@@ -462,14 +477,73 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
return;
}
- if (!result.EndOfMessage)
+ // The common case — a keystroke, an acknowledgement — is one read, and goes straight through.
+ if (result.EndOfMessage && length == 0 && !oversized)
{
- // A frame larger than the receive buffer. Terminal input and acknowledgements are tiny,
- // so this is either a bug or something hostile; dropping is the safe response.
+ DispatchSafely(buffer.AsSpan(0, result.Count));
continue;
}
- Dispatch(buffer.AsSpan(0, result.Count));
+ if (!oversized && length + result.Count <= MaximumMessageBytes)
+ {
+ if (length + result.Count > message.Length)
+ {
+ Array.Resize(ref message, Math.Min(MaximumMessageBytes, Math.Max(message.Length * 2, length + result.Count)));
+ }
+
+ buffer.AsSpan(0, result.Count).CopyTo(message.AsSpan(length));
+ length += result.Count;
+ }
+ else
+ {
+ oversized = true;
+ }
+
+ if (!result.EndOfMessage)
+ {
+ continue;
+ }
+
+ if (!oversized)
+ {
+ DispatchSafely(message.AsSpan(0, length));
+ }
+
+ length = 0;
+ oversized = false;
+ }
+ }
+
+ /// Dispatches one frame, so that what goes wrong with one session stays with that session.
+ ///
+ /// Every session shares this socket, and an exception escaping the receive loop closes it — so a resize
+ /// that reached a shell SSH.NET had already disposed, which is an ordinary moment at the end of every
+ /// session, used to drop every other tab to "reconnecting" and lose the output sent meanwhile. A frame
+ /// that cannot be acted on is a frame not acted on; the session it was for reports its own ending.
+ ///
+ private void DispatchSafely(ReadOnlySpan frame)
+ {
+ try
+ {
+ Dispatch(frame);
+ }
+ catch (Exception exception) when (exception is not OutOfMemoryException and not OperationCanceledException)
+ {
+ // See the remarks.
+ }
+ }
+
+ /// Writes one input frame, without letting a session that is going away fault anything.
+ private async Task WriteInputAsync(TerminalSessionPump pump, byte[] input)
+ {
+ try
+ {
+ await pump.WriteInputAsync(input, lifetime.Token).ConfigureAwait(false);
+ }
+ catch (Exception exception) when (exception is not OutOfMemoryException)
+ {
+ // The shell ended or the connection dropped between the keystroke and the write. The session's
+ // own read loop reports that; a keystroke that had nowhere to go has nothing to add.
}
}
@@ -502,7 +576,7 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
case TerminalClientOpcode.Input:
// Fire and forget: a blocked write must not stall acknowledgements for other sessions
// sharing this socket.
- _ = pump.WriteInputAsync(payload.ToArray(), lifetime.Token).AsTask();
+ _ = WriteInputAsync(pump, payload.ToArray());
break;
case TerminalClientOpcode.Acknowledge:
diff --git a/src/DodoSSH.Client.Terminal/TerminalSessionPump.cs b/src/DodoSSH.Client.Terminal/TerminalSessionPump.cs
index e843eb6..5f95ce9 100644
--- a/src/DodoSSH.Client.Terminal/TerminalSessionPump.cs
+++ b/src/DodoSSH.Client.Terminal/TerminalSessionPump.cs
@@ -107,6 +107,19 @@ public sealed class TerminalSessionPump : IAsyncDisposable
private readonly OutputBacklog backlog;
private readonly CancellationTokenSource lifetime = new();
+
+ /// One write to the remote at a time, in the order they were asked for.
+ ///
+ /// The data plane hands each input frame over without waiting for the one before it — a blocked write
+ /// must not hold up the acknowledgements of every other session on the socket — so without this, two
+ /// frames' writes run at once. SSH.NET's ShellStream takes no lock around its write buffer, and its
+ /// flush sends what is there and then empties it by length: bytes added mid-flush are discarded unsent.
+ /// A snippet's RUN is its text and then a carriage return as two frames, which is exactly the shape that lost or
+ /// reordered keystrokes. Waiters on a are released in the order they arrived,
+ /// and the data plane calls this from one receive loop, so arrival order is the order typed.
+ ///
+ private readonly SemaphoreSlim inputGate = new(1, 1);
+
private int disposed;
/// Identifies this session in every frame.
@@ -267,9 +280,20 @@ public sealed class TerminalSessionPump : IAsyncDisposable
}
}
- /// Forwards keystrokes to the remote.
- public ValueTask WriteInputAsync(ReadOnlyMemory data, CancellationToken cancellationToken) =>
- session.WriteAsync(data, cancellationToken);
+ /// Forwards keystrokes to the remote, after any still being written.
+ public async ValueTask WriteInputAsync(ReadOnlyMemory data, CancellationToken cancellationToken)
+ {
+ await inputGate.WaitAsync(cancellationToken).ConfigureAwait(false);
+
+ try
+ {
+ await session.WriteAsync(data, cancellationToken).ConfigureAwait(false);
+ }
+ finally
+ {
+ inputGate.Release();
+ }
+ }
/// Tells the remote the terminal was resized.
public void Resize(TerminalSize size) => session.Resize(size);
diff --git a/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs b/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs
index a9df86a..840cb77 100644
--- a/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs
+++ b/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs
@@ -78,6 +78,14 @@ internal sealed class FakeShellSession : ISshShellSession
/// The last size the pump forwarded, or null if it forwarded none.
public TerminalSize? LastResize { get; private set; }
+ /// How long the first write takes, so a test can see whether a second one overtakes it.
+ public TimeSpan FirstWriteDelay { get; init; }
+
+ /// Whether a resize throws, as SSH.NET's does once it has disposed the shell's stream.
+ public bool ResizeThrows { get; init; }
+
+ private int writes;
+
/// How many resizes were forwarded, so a dropped one is observable.
public int ResizeCount { get; private set; }
@@ -119,19 +127,27 @@ internal sealed class FakeShellSession : ISshShellSession
}
///
- public ValueTask WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken)
+ public async ValueTask WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken)
{
+ if (Interlocked.Increment(ref writes) == 1 && FirstWriteDelay > TimeSpan.Zero)
+ {
+ await Task.Delay(FirstWriteDelay, cancellationToken).ConfigureAwait(false);
+ }
+
lock (gate)
{
written.AddRange(data.ToArray());
}
-
- return ValueTask.CompletedTask;
}
///
public void Resize(TerminalSize size)
{
+ if (ResizeThrows)
+ {
+ throw new ObjectDisposedException("ShellStream");
+ }
+
// Mirrors the real session: an unusable size is dropped rather than forwarded.
if (!size.IsUsable)
{
diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs
index 46cfe09..01e17be 100644
--- a/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs
+++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalDataPlaneTests.cs
@@ -285,6 +285,86 @@ public sealed class TerminalDataPlaneTests : IAsyncDisposable
Encoding.UTF8.GetString(session.Written).ShouldBe("whoami\r");
}
+ ///
+ /// A paste is one input frame however long it is, and one longer than a single read used to be dropped
+ /// — or worse, its tail parsed as a frame of its own.
+ ///
+ [Fact]
+ public async Task APasteLargerThanOneRead_ReachesTheRemoteWhole()
+ {
+ Start();
+
+ using var socket = await ConnectAsync();
+
+ await using var session = new FakeShellSession();
+ await using var pump = CreatePump(session);
+ plane.Register(SessionId, pump);
+
+ var paste = new byte[300 * 1024];
+ Random.Shared.NextBytes(paste);
+
+ await SendAsync(socket, (byte)TerminalClientOpcode.Input, paste);
+
+ await WaitUntilAsync(() => session.Written.Length >= paste.Length);
+
+ session.Written.ShouldBe(paste);
+ }
+
+ ///
+ /// Frames are handed over without waiting for the write before them, so a slow write must still be
+ /// followed rather than overtaken: a snippet's RUN is its text and then a carriage return.
+ ///
+ [Fact]
+ public async Task InputFrames_AreWrittenInTheOrderTheyArrived_EvenWhenAWriteIsSlow()
+ {
+ Start();
+
+ using var socket = await ConnectAsync();
+
+ await using var session = new FakeShellSession { FirstWriteDelay = TimeSpan.FromMilliseconds(300) };
+ await using var pump = CreatePump(session);
+ plane.Register(SessionId, pump);
+
+ await SendAsync(socket, (byte)TerminalClientOpcode.Input, "sudo systemctl restart api"u8.ToArray());
+ await SendAsync(socket, (byte)TerminalClientOpcode.Input, "\r"u8.ToArray());
+
+ await WaitUntilAsync(() => session.Written.Length >= 27);
+
+ Encoding.UTF8.GetString(session.Written).ShouldBe("sudo systemctl restart api\r");
+ }
+
+ ///
+ /// A resize that reaches a shell SSH.NET has already disposed is an ordinary moment at the end of a
+ /// session, and it must not close the socket every other session shares.
+ ///
+ [Fact]
+ public async Task AResizeThatThrows_LeavesTheSocketWorkingForEverythingElse()
+ {
+ Start();
+
+ using var socket = await ConnectAsync();
+
+ await using var ending = new FakeShellSession { ResizeThrows = true };
+ await using var endingPump = CreatePump(ending);
+ plane.Register(SessionId, endingPump);
+
+ await using var other = new FakeShellSession();
+ await using var otherPump = CreatePump(other);
+ plane.Register(SessionId + 1, otherPump);
+
+ await SendAsync(
+ socket,
+ (byte)TerminalClientOpcode.Resize,
+ TerminalFrame.CreateResizePayload(new TerminalSize(80, 24, 800, 600)));
+
+ await SendAsync(socket, (byte)TerminalClientOpcode.Input, "still here"u8.ToArray(), SessionId + 1);
+
+ await WaitUntilAsync(() => other.Written.Length > 0);
+
+ Encoding.UTF8.GetString(other.Written).ShouldBe("still here");
+ socket.State.ShouldBe(WebSocketState.Open);
+ }
+
[Fact]
public async Task AResize_ReachesTheRemote()
{