Skip to content

Add minimum remaining-data checks for fixed-size binary reads#125700

Closed
FUTURE-SL wants to merge 1 commit intodotnet:mainfrom
FUTURE-SL:binaryreader-fixed-size-read-checks
Closed

Add minimum remaining-data checks for fixed-size binary reads#125700
FUTURE-SL wants to merge 1 commit intodotnet:mainfrom
FUTURE-SL:binaryreader-fixed-size-read-checks

Conversation

@FUTURE-SL
Copy link

This change adds pre-read validation for operations where the required byte count is known in advance.

What changed:

  • added EnsureCanRead(int byteCount)
  • used it in InternalRead(Span<byte>)
  • used it in ReadExactly(Span<byte>)
  • used it in ReadString() after reading the encoded byte length

Why:

  • fail earlier on seekable streams when there is not enough remaining data
  • avoid starting partial reads for fixed-size values and length-prefixed strings
  • improve behavior for truncated or malformed inputs without introducing hard size limits

Added EnsureCanRead method to validate byte count before reading.
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 18, 2026
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

@huoyaoyuan
Copy link
Member

This is unexpected behaviour change and degrades performance. It's the stream's responsibility to reject oversized reads.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream.Length will often perform a sys-call. This is a performance regression.
Stream.Length can throw an exception for seekable Streams. This is a breaking change.

The PR has no tests that show the benefits. Please open an issue first and describe the problem you are facing.

@adamsitnik adamsitnik closed this Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.IO community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants