Provide a version of Stream.Read that reads as much as possible

[svick]

Background and motivation

One of the most common mistakes when using Stream.Read() is that the programmer doesn't realize that Read() may return less data than what is available in the Stream and less data than the buffer being passed in. And even for programmers who are aware of this, having to write the same loop every single time they want to read from a Stream is annoying.

With the .NET 6 breaking change: Partial and zero-byte reads in DeflateStream, GZipStream, and CryptoStream, it has become apparent that a Stream.Read API that ensures you get at least n bytes read is valuable.

API Proposal

namespace System.IO
{
    public class Stream
    {
+        public int ReadAtLeast(Span<byte> buffer, int minimumBytes, bool throwOnEndOfStream = true);
+        public ValueTask<int> ReadAtLeastAsync(Memory<byte> buffer, int minimumBytes, bool throwOnEndOfStream = true, CancellationToken cancellationToken = default);
    }
}

• ReadAtLeast will return the number of bytes read into buffer.
• When throwOnEndOfStream == true, the return value will always be minimumBytes <= bytesRead <= buffer.Length. If the end of the stream is detected, an EndOfStreamException will be thrown.
• When throwOnEndOfStream == false, the return value will always be bytesRead <= buffer.Length. Callers can check for end of the stream by checking bytesRead < minimumBytes.

API Usage

Example 1

Stream stream = ...;
Span<byte> buffer = ...;

stream.ReadAtLeast(buffer, buffer.Length);
// Do something with the bytes in `buffer`.

Example 2

Stream stream = ...;
Span<byte> buffer = ...;

// make sure we have at least 4 bytes
const int bytesToRead = 4;
int read = stream.ReadAtLeast(buffer, bytesToRead, throwOnEndOfStream: false);
if (read < bytesToRead)
{
    // Handle an early end of the stream. E.g. throw your own exception, set a flag, etc.
}
else
{
    // Do something with the bytes in `buffer`
    // `read` may be more than 4
}

Alternative Designs

1. We could add a convenience wrapper (ReadAll or Fill) that doesn't take int minimumBytes, and uses buffer.Length as the minimumBytes.

   1. This can be accomplished by simply passing in buffer.Length
   2. The APIs wouldn't be overloads since the names (ReadAtLeast and ReadAll) wouldn't match. I can't think of a decent common name that would work for both operations.
   3. We can always add it later, if we get enough feedback that it is necessary.

2. There is a question of whether these methods should be virtual or not. From scanning the Stream implementations in dotnet/runtime, I don't see anything special a Stream could do for ReadAtLeast. They already get passed the buffer length, if they want a hint of how much data is being requested.

   1. We can always add it later, if we get enough feedback that it is necessary.
   2. The one place I did find that could possibly override ReadAtLeast is in PipeReader.AsStream()'s implementation. Since PipeReader has a ReadAtLeastAsync API, the PipeReaderStream could override ReadAtLeastAsync and forward the call directly to PipeReader.ReadAtLeastAsync. However, there is no synchronous API on PipeReader, so it would just be implemented for the async API.

---

Original Proposal

I believe one of the most common mistakes when using Stream.Read() is that the programmer doesn't realize that Read() may return less data than what is available in the Stream. And even for programmers who are aware of this, having to write the same loop every single time they want to read from a Stream is annoying.

So, my proposal is to add the following methods to Stream (mirroring the existing Read methods):

public int ReadAll(byte[] buffer, int offset, int count);
public Task<int> ReadAllAsync(byte[] buffer, int offset, int count);
public Task<int> ReadAllAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken);

Each ReadAll method would call the corresponding Read method in a loop, until the buffer was filled or until Read returned 0.

Questions:

• Is there a better name than ReadAll?
• These methods would work as well if they were extension methods. Should they be?
• Very similar functionality is already exposed through BinaryReader.ReadBytes(). Why doesn't it have async version?

[eadordzhiev]

+1 for async version of BinaryReader

[davidsh]

While .NET Stream.Read does have the behavior you mention, the WinRT IInputStream.ReadAsync method actually takes a enum parameter that can vary the behavior from "return as quick as possible" to "read as much as possible filling the buffer":

https://msdn.microsoft.com/en-us/library/hh438388(v=vs.85).aspx

InputStreamOptions:
https://msdn.microsoft.com/en-us/library/windows/apps/windows.storage.streams.inputstreamoptions.aspx

None 0 No options are specified.
Partial 1 The asynchronous read operation completes when one or more bytes is available.
ReadAhead 2 The asynchronous read operation may optionally read ahead and prefetch additional bytes.

So, this is an example of an API pattern that might be useful in some way for .NET.

[svick]

@davidsh Interesting. Though I think that having a guarantee that everything that could be read was read is more useful than what ReadAhead is documented to do.

[JonHanna]

@svick though we can provide that guarantee to ourselves right now (and already have the greater guarantee of a given number of transcoded characters when going through a StreamReader), but not give the options @davidsh mentions. I'd rather gain that new ability than an extra wrapper method, if I could pick only one.

[terrajobst]

We see two options:

First one:

public int ReadRequestedCount(byte[] buffer, int offset, int count);
public Task<int> ReadRequestedCountAsync(byte[] buffer, int offset, int count);
public Task<int> ReadRequestedCountAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken);

Second one:

enum StreamOptions
{
    Partial = 0,
    RequestedCount = 1
}

public virtual int Read(byte[] buffer, int offset, int count, StreamOptions options);
public Task<int> ReadAsync(byte[] buffer, int offset, int count, StreamOptions options);
public virtual Task<int> ReadAsync(byte[] buffer, int offset, int count, StreamOptions options, CancellationToken cancellationToken);

The nice thing about the second one is that we can add more policy if we need to; Stream is abstract and adding new concept is generally somewhat harder. By making it an enum its a bit easier.

Secondly, do we need to do anything for Write? Our current behavior is that Write is essentially WriteAll. Do we need a partial version as well?

[ianhays]

Stylistically I prefer the second of the two options, but I'm a bit of a sucker for enums. ReadRequestedCount is an awfully verbose name for what is a conceptually simple modification of the existing Read.

Secondly, do we need to do anything for Write? Our current behavior is that Write is essentially WriteAll. Do we need a partial version as well?

I'd argue that our existing Write is already adequate because our default behavior is to write all bytes; if you want a partial write you could just decrease the count you pass to the function. The only way I can see a WritePartial function providing value is if you wanted to break up a Write of X bytes into X/Y smaller writes. That alone doesn't seem like a common enough scenario to warrant additional API (unlike the ReadAll function which I'm all +1 for).

[JeremyKuhne]

Triage: seems valuable, let's finish addressing the API review feedback.