From ee600d02ac059c1b15e76de1b3b275e5ccd1b532 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:55:37 -0700 Subject: [PATCH] fix(network): stop silently dropping packets when the send buffer is full NetState.Send had three failure modes once the send buffer filled, none of them visible: - No writable space: GetSendBuffer returned false and the packet was dropped with no log and no disconnect. The client stayed connected while quietly missing game state. - Some space but not enough: NetworkCompression.Compress hit its guard and returned 0, so CommitWrite(0) ran and the packet was dropped the same way. - One to three bytes writable: Compress computes safeOutputLength = (nuint)output.Length - 4, which underflows for those sizes. The hot loop's bounds check never trips and it writes past the span, corrupting the in-flight region of the ring buffer. Reachable because callers only check for non-zero space. Compress now refuses an output too small to bound, and Send reports exhaustion instead of dropping: it logs and disconnects with the bytes needed, writable, awaiting acknowledgement, and total capacity. Those numbers separate a slow client holding the buffer from a buffer genuinely too small for the shard, which is the case that warrants raising network.sendBufferSize. Tests cover the underflow via sentinel bytes around the output window and were confirmed to fail without the guard. Co-Authored-By: Claude Opus 5 (1M context) --- .../Network/NetworkCompressionBoundsTests.cs | 86 +++++++++++++++++++ Projects/Server/Network/NetState/NetState.cs | 46 +++++++++- Projects/Server/Network/NetworkCompression.cs | 4 +- 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 Projects/Server.Tests/Tests/Network/NetworkCompressionBoundsTests.cs diff --git a/Projects/Server.Tests/Tests/Network/NetworkCompressionBoundsTests.cs b/Projects/Server.Tests/Tests/Network/NetworkCompressionBoundsTests.cs new file mode 100644 index 000000000..c1bc6a800 --- /dev/null +++ b/Projects/Server.Tests/Tests/Network/NetworkCompressionBoundsTests.cs @@ -0,0 +1,86 @@ +using System; +using Server.Network; +using Xunit; + +namespace Server.Tests.Network; + +/// +/// Bounds behaviour of the Huffman compressor when the destination is too small. +/// +/// This is reachable in production: NetState only checks that the send buffer has *some* writable +/// space before handing the remainder to Compress, so a nearly-full buffer can offer a span of one +/// to three bytes. The internal guard is computed as an unsigned output.Length - 4, which +/// underflows for those sizes and stops bounding the writes at all. +/// +public class NetworkCompressionBoundsTests +{ + [Theory] + [InlineData(0)] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + public void RefusesOutputTooSmallToBound(int outputSize) + { + var input = new byte[64]; + Array.Fill(input, (byte)'A'); + + // Sentinel-filled backing array; only the middle window is offered to the compressor, so + // any write past the span shows up as a modified sentinel rather than silent corruption. + var backing = new byte[256]; + Array.Fill(backing, (byte)0xCC); + + const int windowStart = 64; + var output = backing.AsSpan(windowStart, outputSize); + + var written = NetworkCompression.Compress(input, output); + + Assert.Equal(0, written); + + for (var i = 0; i < backing.Length; i++) + { + Assert.Equal(0xCC, backing[i]); + } + } + + [Fact] + public void StillCompressesWhenOutputIsLargeEnough() + { + var input = new byte[64]; + Array.Fill(input, (byte)'A'); + + var output = new byte[256]; + + var written = NetworkCompression.Compress(input, output); + + Assert.True(written > 0); + Assert.True(written <= output.Length); + } + + [Fact] + public void ReportsFailureRatherThanOverrunningATightOutput() + { + // Large input against a small-but-bounded output: the guard is well-defined here, so this + // must fail cleanly rather than write past the end. + var input = new byte[4096]; + Array.Fill(input, (byte)'A'); + + var backing = new byte[256]; + Array.Fill(backing, (byte)0xCC); + + const int windowStart = 64; + const int windowSize = 16; + var output = backing.AsSpan(windowStart, windowSize); + + NetworkCompression.Compress(input, output); + + for (var i = 0; i < windowStart; i++) + { + Assert.Equal(0xCC, backing[i]); + } + + for (var i = windowStart + windowSize; i < backing.Length; i++) + { + Assert.Equal(0xCC, backing[i]); + } + } +} diff --git a/Projects/Server/Network/NetState/NetState.cs b/Projects/Server/Network/NetState/NetState.cs index 6c6262ae4..05d8a8f16 100755 --- a/Projects/Server/Network/NetState/NetState.cs +++ b/Projects/Server/Network/NetState/NetState.cs @@ -444,17 +444,36 @@ public partial class NetState : IComparable, IValueLinkListNode buffer.Length) + { + SendBufferExhausted(span.Length, buffer.Length); + return; } else { @@ -484,6 +503,31 @@ public partial class NetState : IComparable, IValueLinkListNode + /// Handles a packet that cannot be placed in the send buffer. + /// + /// + /// High unacked means a slow client holding the buffer; needed approaching capacity means the + /// buffer is too small for this shard and network.sendBufferSize should be raised. + /// + private void SendBufferExhausted(int needed, int writable) + { + var sendBuffer = _socket?.SendBuffer; + var unacked = sendBuffer?.InFlightBytes ?? 0; + var capacity = sendBuffer?.PhysicalSize ?? 0; + + logger.Warning( + "{NetState}: send buffer exhausted - needed {Needed} bytes, {Writable} writable, {Unacked} awaiting acknowledgement, {Capacity} capacity. Raise network.sendBufferSize (power of two) if this recurs on healthy connections.", + this, + needed, + writable, + unacked, + capacity + ); + + Disconnect($"Send buffer exhausted (needed {needed}, writable {writable}, unacked {unacked}, capacity {capacity})"); + } + private void StartPacketLog() { try diff --git a/Projects/Server/Network/NetworkCompression.cs b/Projects/Server/Network/NetworkCompression.cs index 1a0154d40..12b4108da 100644 --- a/Projects/Server/Network/NetworkCompression.cs +++ b/Projects/Server/Network/NetworkCompression.cs @@ -73,7 +73,9 @@ public static class NetworkCompression public static int Compress(ReadOnlySpan input, Span output) { - if (input.Length > DefiniteOverflow) + // output.Length < 4 underflows safeOutputLength below (nuint), defeating the hot loop's + // bounds check. Reachable whenever the send buffer is nearly full. + if (input.Length > DefiniteOverflow || output.Length < 4) { return 0; }