fix: Makes SpanWriter/SpanReader exceptions clearer (#2262)

This commit is contained in:
Kamron Batman 2025-11-12 23:34:13 -08:00 committed by GitHub
parent 5d920a25b1
commit 68caaab92c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 55 additions and 96 deletions

View file

@ -1,6 +1,6 @@
/************************************************************************* /*************************************************************************
* ModernUO * * ModernUO *
* Copyright 2019-2023 - ModernUO Development Team * * Copyright 2019-2025 - ModernUO Development Team *
* Email: hi@modernuo.com * * Email: hi@modernuo.com *
* File: SpanReader.cs * * File: SpanReader.cs *
* * * *
@ -14,7 +14,6 @@
*************************************************************************/ *************************************************************************/
using System.Buffers.Binary; using System.Buffers.Binary;
using System.Diagnostics;
using System.IO; using System.IO;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Text; using System.Text;
@ -45,7 +44,7 @@ public ref struct SpanReader
{ {
if (Position >= Length) if (Position >= Length)
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
return _buffer[Position++]; return _buffer[Position++];
@ -62,7 +61,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadInt16BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadInt16BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 2; Position += 2;
@ -74,7 +73,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadInt16LittleEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadInt16LittleEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 2; Position += 2;
@ -86,7 +85,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadUInt16BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadUInt16BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 2; Position += 2;
@ -98,7 +97,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadUInt16LittleEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadUInt16LittleEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 2; Position += 2;
@ -110,7 +109,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadInt32BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadInt32BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 4; Position += 4;
@ -122,7 +121,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadUInt32BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadUInt32BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 4; Position += 4;
@ -134,7 +133,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadUInt32LittleEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadUInt32LittleEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 4; Position += 4;
@ -146,7 +145,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadInt64BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadInt64BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 8; Position += 8;
@ -158,7 +157,7 @@ public ref struct SpanReader
{ {
if (!BinaryPrimitives.TryReadUInt64BigEndian(_buffer[Position..], out var value)) if (!BinaryPrimitives.TryReadUInt64BigEndian(_buffer[Position..], out var value))
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
Position += 8; Position += 8;
@ -173,9 +172,9 @@ public ref struct SpanReader
return ""; return "";
} }
int byteLength = encoding.GetByteLengthForEncoding(); var byteLength = encoding.GetByteLengthForEncoding();
bool isFixedLength = fixedLength > -1; var isFixedLength = fixedLength > -1;
var remaining = Remaining; var remaining = Remaining;
int size; int size;
@ -184,7 +183,7 @@ public ref struct SpanReader
size = fixedLength * byteLength; size = fixedLength * byteLength;
if (size > Remaining) if (size > Remaining)
{ {
throw new OutOfMemoryException(); throw new EndOfStreamException("Cannot read past the end of the buffer.");
} }
} }
else else
@ -255,42 +254,24 @@ public ref struct SpanReader
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public int Seek(int offset, SeekOrigin origin) public int Seek(int offset, SeekOrigin origin)
{ {
Debug.Assert( var newPosition = origin switch
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
{ {
SeekOrigin.Current => Position + offset, SeekOrigin.Current => Position + offset,
SeekOrigin.End => _buffer.Length + offset, SeekOrigin.End => _buffer.Length + offset,
_ => offset // Begin _ => 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)] [MethodImpl(MethodImplOptions.AggressiveInlining)]

View file

@ -1,6 +1,6 @@
/************************************************************************* /*************************************************************************
* ModernUO * * ModernUO *
* Copyright 2019-2023 - ModernUO Development Team * * Copyright 2019-2025 - ModernUO Development Team *
* Email: hi@modernuo.com * * Email: hi@modernuo.com *
* File: SpanWriter.cs * * File: SpanWriter.cs *
* * * *
@ -14,7 +14,6 @@
*************************************************************************/ *************************************************************************/
using System.Buffers.Binary; using System.Buffers.Binary;
using System.Diagnostics;
using System.IO; using System.IO;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Runtime.InteropServices; using System.Runtime.InteropServices;
@ -56,14 +55,14 @@ public ref struct SpanWriter
public Span<byte> RawBuffer => _buffer; public Span<byte> RawBuffer => _buffer;
/** /**
* Converts the writer to a Span<byte> using a SpanOwner. * Converts the writer to a Span<byte> using a SpanOwner.
* If the buffer was stackalloc, it will be copied to a rented buffer. * If the buffer was stackalloc, it will be copied to a rented buffer.
* Otherwise the existing rented buffer is used. * Otherwise the existing rented buffer is used.
* *
* Note: * Note:
* Do not use the SpanWriter after calling this method. * Do not use the SpanWriter after calling this method.
* This method will effectively dispose of the SpanWriter and is therefore considered terminal. * This method will effectively dispose of the SpanWriter and is therefore considered terminal.
*/ */
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public SpanOwner ToSpan() public SpanOwner ToSpan()
{ {
@ -117,11 +116,11 @@ public ref struct SpanWriter
private void Grow(int additionalCapacity) private void Grow(int additionalCapacity)
{ {
var newSize = Math.Max(BytesWritten + additionalCapacity, _buffer.Length * 2); var newSize = Math.Max(BytesWritten + additionalCapacity, _buffer.Length * 2);
byte[] poolArray = STArrayPool<byte>.Shared.Rent(newSize); var poolArray = STArrayPool<byte>.Shared.Rent(newSize);
_buffer[..BytesWritten].CopyTo(poolArray); _buffer[..BytesWritten].CopyTo(poolArray);
byte[] toReturn = _arrayToReturnToPool; var toReturn = _arrayToReturnToPool;
_buffer = _arrayToReturnToPool = poolArray; _buffer = _arrayToReturnToPool = poolArray;
if (toReturn != null) if (toReturn != null)
{ {
@ -136,7 +135,7 @@ public ref struct SpanWriter
{ {
if (!_resize) if (!_resize)
{ {
throw new OutOfMemoryException(); throw new InvalidOperationException("Buffer is full and resizing is disabled.");
} }
Grow(count); Grow(count);
@ -151,7 +150,7 @@ public ref struct SpanWriter
{ {
if (!_resize) if (!_resize)
{ {
throw new OutOfMemoryException(); throw new InvalidOperationException("Buffer is full and resizing is disabled.");
} }
Grow(capacity - BytesWritten); Grow(capacity - BytesWritten);
@ -400,46 +399,25 @@ public ref struct SpanWriter
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public int Seek(int offset, SeekOrigin origin) public int Seek(int offset, SeekOrigin origin)
{ {
Debug.Assert( var newPosition = origin switch
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
{ {
SeekOrigin.Current => _position + offset, SeekOrigin.Current => _position + offset,
SeekOrigin.End => BytesWritten + offset, SeekOrigin.End => BytesWritten + offset,
_ => offset // Begin _ => offset // Begin
}); };
if (newPosition < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset), "Seek operation would result in a negative position.");
}
if (newPosition > _buffer.Length) 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); Grow(newPosition - _buffer.Length + 1);
} }
@ -449,7 +427,7 @@ public ref struct SpanWriter
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose() 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 this = default; // for safety, to avoid using pooled array if this instance is erroneously appended to again
if (toReturn != null) if (toReturn != null)
{ {
@ -478,7 +456,7 @@ public ref struct SpanWriter
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Dispose() public void Dispose()
{ {
byte[] toReturn = _arrayToReturnToPool; var toReturn = _arrayToReturnToPool;
this = default; this = default;
if (_length > 0) if (_length > 0)
{ {