From 68caaab92caa0bc0bb600ca7ba3189a5e8ad1de1 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Wed, 12 Nov 2025 23:34:13 -0800 Subject: [PATCH] fix: Makes SpanWriter/SpanReader exceptions clearer (#2262) --- Projects/Server/Buffers/SpanReader.cs | 75 ++++++++++---------------- Projects/Server/Buffers/SpanWriter.cs | 76 ++++++++++----------------- 2 files changed, 55 insertions(+), 96 deletions(-) diff --git a/Projects/Server/Buffers/SpanReader.cs b/Projects/Server/Buffers/SpanReader.cs index 31c5fc51d..290ba5a94 100644 --- a/Projects/Server/Buffers/SpanReader.cs +++ b/Projects/Server/Buffers/SpanReader.cs @@ -1,6 +1,6 @@ /************************************************************************* * ModernUO * - * Copyright 2019-2023 - ModernUO Development Team * + * Copyright 2019-2025 - ModernUO Development Team * * Email: hi@modernuo.com * * File: SpanReader.cs * * * @@ -14,7 +14,6 @@ *************************************************************************/ using System.Buffers.Binary; -using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Text; @@ -45,7 +44,7 @@ public ref struct SpanReader { if (Position >= Length) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } return _buffer[Position++]; @@ -62,7 +61,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadInt16BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 2; @@ -74,7 +73,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadInt16LittleEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 2; @@ -86,7 +85,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadUInt16BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 2; @@ -98,7 +97,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadUInt16LittleEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 2; @@ -110,7 +109,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadInt32BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 4; @@ -122,7 +121,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadUInt32BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 4; @@ -134,7 +133,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadUInt32LittleEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 4; @@ -146,7 +145,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadInt64BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 8; @@ -158,7 +157,7 @@ public ref struct SpanReader { if (!BinaryPrimitives.TryReadUInt64BigEndian(_buffer[Position..], out var value)) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } Position += 8; @@ -173,9 +172,9 @@ public ref struct SpanReader return ""; } - int byteLength = encoding.GetByteLengthForEncoding(); + var byteLength = encoding.GetByteLengthForEncoding(); - bool isFixedLength = fixedLength > -1; + var isFixedLength = fixedLength > -1; var remaining = Remaining; int size; @@ -184,7 +183,7 @@ public ref struct SpanReader size = fixedLength * byteLength; if (size > Remaining) { - throw new OutOfMemoryException(); + throw new EndOfStreamException("Cannot read past the end of the buffer."); } } else @@ -255,42 +254,24 @@ public ref struct SpanReader [MethodImpl(MethodImplOptions.AggressiveInlining)] public int Seek(int offset, SeekOrigin origin) { - Debug.Assert( - origin != SeekOrigin.End || offset <= 0, - "Attempting to seek to a position beyond capacity using SeekOrigin.End" - ); - - Debug.Assert( - origin != SeekOrigin.End || offset >= -_buffer.Length, - "Attempting to seek to a negative position using SeekOrigin.End" - ); - - Debug.Assert( - origin != SeekOrigin.Begin || offset >= 0, - "Attempting to seek to a negative position using SeekOrigin.Begin" - ); - - Debug.Assert( - origin != SeekOrigin.Begin || offset <= _buffer.Length, - "Attempting to seek to a position beyond the capacity using SeekOrigin.Begin" - ); - - Debug.Assert( - origin != SeekOrigin.Current || Position + offset >= 0, - "Attempting to seek to a negative position using SeekOrigin.Current" - ); - - Debug.Assert( - origin != SeekOrigin.Current || Position + offset <= _buffer.Length, - "Attempting to seek to a position beyond the capacity using SeekOrigin.Current" - ); - - return Position = Math.Max(0, origin switch + var newPosition = origin switch { SeekOrigin.Current => Position + offset, SeekOrigin.End => _buffer.Length + offset, _ => offset // Begin - }); + }; + + if (newPosition < 0) + { + throw new ArgumentOutOfRangeException(nameof(offset), "Seek operation would result in a negative position."); + } + + if (newPosition > _buffer.Length) + { + throw new ArgumentOutOfRangeException(nameof(offset), $"Cannot seek to position {newPosition} beyond buffer length {_buffer.Length}."); + } + + return Position = newPosition; } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/Projects/Server/Buffers/SpanWriter.cs b/Projects/Server/Buffers/SpanWriter.cs index 4ce423c5b..ccdaddd38 100644 --- a/Projects/Server/Buffers/SpanWriter.cs +++ b/Projects/Server/Buffers/SpanWriter.cs @@ -1,6 +1,6 @@ /************************************************************************* * ModernUO * - * Copyright 2019-2023 - ModernUO Development Team * + * Copyright 2019-2025 - ModernUO Development Team * * Email: hi@modernuo.com * * File: SpanWriter.cs * * * @@ -14,7 +14,6 @@ *************************************************************************/ using System.Buffers.Binary; -using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -56,14 +55,14 @@ public ref struct SpanWriter public Span RawBuffer => _buffer; /** - * Converts the writer to a Span using a SpanOwner. - * If the buffer was stackalloc, it will be copied to a rented buffer. - * Otherwise the existing rented buffer is used. - * - * Note: - * Do not use the SpanWriter after calling this method. - * This method will effectively dispose of the SpanWriter and is therefore considered terminal. - */ + * Converts the writer to a Span using a SpanOwner. + * If the buffer was stackalloc, it will be copied to a rented buffer. + * Otherwise the existing rented buffer is used. + * + * Note: + * Do not use the SpanWriter after calling this method. + * This method will effectively dispose of the SpanWriter and is therefore considered terminal. + */ [MethodImpl(MethodImplOptions.AggressiveInlining)] public SpanOwner ToSpan() { @@ -117,11 +116,11 @@ public ref struct SpanWriter private void Grow(int additionalCapacity) { var newSize = Math.Max(BytesWritten + additionalCapacity, _buffer.Length * 2); - byte[] poolArray = STArrayPool.Shared.Rent(newSize); + var poolArray = STArrayPool.Shared.Rent(newSize); _buffer[..BytesWritten].CopyTo(poolArray); - byte[] toReturn = _arrayToReturnToPool; + var toReturn = _arrayToReturnToPool; _buffer = _arrayToReturnToPool = poolArray; if (toReturn != null) { @@ -136,7 +135,7 @@ public ref struct SpanWriter { if (!_resize) { - throw new OutOfMemoryException(); + throw new InvalidOperationException("Buffer is full and resizing is disabled."); } Grow(count); @@ -151,7 +150,7 @@ public ref struct SpanWriter { if (!_resize) { - throw new OutOfMemoryException(); + throw new InvalidOperationException("Buffer is full and resizing is disabled."); } Grow(capacity - BytesWritten); @@ -400,46 +399,25 @@ public ref struct SpanWriter [MethodImpl(MethodImplOptions.AggressiveInlining)] public int Seek(int offset, SeekOrigin origin) { - Debug.Assert( - origin != SeekOrigin.End || _resize || offset <= 0, - "Attempting to seek to a position beyond capacity using SeekOrigin.End without resize" - ); - - Debug.Assert( - origin != SeekOrigin.End || offset >= -_buffer.Length, - - "Attempting to seek to a negative position using SeekOrigin.End" - ); - - Debug.Assert( - origin != SeekOrigin.Begin || offset >= 0, - "Attempting to seek to a negative position using SeekOrigin.Begin" - ); - - Debug.Assert( - origin != SeekOrigin.Begin || _resize || offset <= _buffer.Length, - "Attempting to seek to a position beyond the capacity using SeekOrigin.Begin without resize" - ); - - Debug.Assert( - origin != SeekOrigin.Current || _position + offset >= 0, - "Attempting to seek to a negative position using SeekOrigin.Current" - ); - - Debug.Assert( - origin != SeekOrigin.Current || _resize || _position + offset <= _buffer.Length, - "Attempting to seek to a position beyond the capacity using SeekOrigin.Current without resize" - ); - - var newPosition = Math.Max(0, origin switch + var newPosition = origin switch { SeekOrigin.Current => _position + offset, SeekOrigin.End => BytesWritten + offset, _ => offset // Begin - }); + }; + + if (newPosition < 0) + { + throw new ArgumentOutOfRangeException(nameof(offset), "Seek operation would result in a negative position."); + } if (newPosition > _buffer.Length) { + if (!_resize) + { + throw new InvalidOperationException($"Cannot seek to position {newPosition} beyond buffer capacity {_buffer.Length} when resizing is disabled."); + } + Grow(newPosition - _buffer.Length + 1); } @@ -449,7 +427,7 @@ public ref struct SpanWriter [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Dispose() { - byte[] toReturn = _arrayToReturnToPool; + var toReturn = _arrayToReturnToPool; this = default; // for safety, to avoid using pooled array if this instance is erroneously appended to again if (toReturn != null) { @@ -478,7 +456,7 @@ public ref struct SpanWriter [MethodImpl(MethodImplOptions.AggressiveInlining)] public void Dispose() { - byte[] toReturn = _arrayToReturnToPool; + var toReturn = _arrayToReturnToPool; this = default; if (_length > 0) {