Skip to content

MemoryStream.CopyToAsync ignores buffer size which can cause BufferedStream to throw OverflowException #117789

@mconnew

Description

@mconnew

Description

If MemoryStream is holding around 1GB of data, calling MemoryStream.CopyToAsync attempts to write the entire buffered data in a single write to the destination stream. If the buffer passed to BufferedStream.WriteAsync would overflow its internal buffer, it calls BufferedStream.WriteToUnderlyingStreamAsync. This method has the following bounds check:

    int totalUserBytes;
    bool useBuffer;
    checked
    {
        // We do not expect buffer sizes big enough for an overflow, but if it happens, lets fail early:
        totalUserBytes = _writePos + buffer.Length;
        useBuffer = (totalUserBytes + buffer.Length < (_bufferSize + _bufferSize));
    }

The left side of the less than comparison ends up being _writePos + (2 * buffer.Length). If buffer.Length is close to (depending on the current _writePos value) or larger than 1GB, the value will overflow an int32.
This happens because MemoryStream.CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) has the following code:

  // If destination is not a memory stream, write there asynchronously:
  if (!(destination is MemoryStream memStrDest))
      return destination.WriteAsync(_buffer, pos, n, cancellationToken);

This results in MemoryStream not writing its contents to the destination stream in chunks that are bufferSize big.

Another scenario where this might cause problems is when the destination stream doesn't override WriteAsync, instead depending on the default Stream.WriteAsync which wraps the BeginWrite/EndWrite methods in a Task. The default wrapper checks the state of the cancellationToken before calling BeginWrite/EndWrite. By calling destination.WriteAsync on the entire buffer instead of breaking it up like the semantics of CopyToAsync require, any amount of data stored in a MemoryStream would only check the cancellationToken at initial call. One example of this is HttpResponseStream which doesn't override WriteAsync. If for example I had a more modest 5MB of data to write as a response, the cancellationToken would only get checked on first calling WriteAsync. If the request is sending very slowly, it would complete the entire send of the response body regardless of how much time it took, ignoring the cancellationToken. If MemoryStream.CopyToAsync honored the bufferSize, then a slow send would have a chance between each buffer being sent to check the cancellationToken.

Reproduction Steps

byte[] dummyData = new byte[1024 * 1024 * 511 * 3]; // 512MB
using var stream = new MemoryStream(dummyData);

var destStream = new MemoryStream(dummyData.Length);
var bufferedStream = new BufferedStream(destStream, 65536);
await stream.CopyToAsync(bufferedStream, 65536);

Expected behavior

For the MemoryStream to successfully write to the BufferedStream.

Actual behavior

This results in a System.OverflowException being thrown.

Regression?

Based on examining code, this looks to have existed all the way back to .NET Framework.

Known Workarounds

Create your own class derived from MemoryStream to use in its place. The type check in MemoryStream.CopyToAsync is written such that it avoids the "fast path" in the code and does the correct thing.

    public class FixedMemoryStream : MemoryStream
    {
        public FixedMemoryStream(byte[] buffer) : base(buffer) { }
        public FixedMemoryStream(byte[] buffer, int index, int count) : base(buffer, index, count) { }
        public FixedMemoryStream(int capacity) : base(capacity) { }
        public FixedMemoryStream() : base() { }
    }

Configuration

No response

Other information

No response

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions