runtime: SslStream now does zero byte read of InnerStream which breaks WCF

Description

The change made in #87563 causes SslStream to execute a read of zero bytes on the InnerStream. This breaks the semantics of the Stream class. When calling any of the variants of Read on a Stream, the return value is overloaded in meaning. If it’s non-zero it returns how many bytes were actually read. But if it returns a 0, that indicates you’ve reached the end of the Stream. In the case of a Stream representing a network connection, this means the connection was closed.

Streams can be wrapped in layers, just like SslStream wraps an inner stream. If you have an intermediate Stream between SslStream and the actual network stream, if it were to pass through the zero byte read, the code which interprets the return value wouldn’t be able to know whether the inner stream was closed or not and would interpret zero bytes returned as the connection was closed.

WCF has multiple utility Stream implementations which wrap the connection Stream for various reasons. One example is a Stream which counts the bytes received and fails the request if too many bytes have been sent. Each of these utility streams, as well as our actual networking Stream (we don’t just use the built in ones as we need to toggle nagle on and off for performance reasons) do argument validation and throw an exception if you pass a zero length input into their Read methods.

Reproduction Steps

public class MyStream : Stream
{
    private bool _eof;
    private Stream _innerStream;

    public MyStream(Stream innerStream)
    {
        _innerStream = innerStream;
    }

    public override int Read(byte[] buffer, int offset, int count)
    {
        ThrowIfEof();
        int bytesRead = innerStream.Read(buffer, offset, count);
        if (bytesRead == 0) _eof = true;
        return bytesRead;
    }

    private void ThrowIfEof()
    {
        if (_eof) throw new ObjectDisposedException();
    }
}

If you used a stream similar to the code above (with the rest of the methods implemented), it wouldn’t work with SslStream as the zero byte read would cause the Read to consider the inner stream to have reached EOF and any subsequent calls will throw ODE.

Expected behavior

SslStream only calls Read with a buffer which is at least 1 byte long.

Actual behavior

SslStream calls Read with an empty buffer.

Regression?

This issue only appeared in a recent preview release of .NET 8

Known Workarounds

Add complicated logic to all wrapping Stream implementations where it only considers 0 bytes read to mean you’ve reached the end of the Stream when a non-zero length buffer has been passed in. This adds extra conditions and branching code in lots of places through our code base.

Configuration

No response

Other information

This is the specific line of code that WCF hit, but there might be other places this happens. https://github.com/dotnet/runtime/blob/d59af2cf097acb100ad6a9cba652be34be6f4a2e/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs#L708

About this issue

Original URL
State: closed
Created a year ago
Comments: 23 (22 by maintainers)

Most upvoted comments

It’s simply the case that the documentation is misleading, in that it’s written referring to the case where there’s a non-0 count requested. The docs don’t discuss the behavior when a 0 count is requested (but we should update the docs to do so). This is obvious when you consider statements in the docs like “Read returns 0 only when there is no more data in the stream and no more is expected (such as a closed socket or end of file)”… that’s obviously erroneous if count == 0 (which is a valid value), in which case it has to return 0.

As has been stated, it has always been the case that there are two valid implementations for count==0:

Regardless of whether it’s EOF, immediately return 0
Wait for data to be available or for EOF, then return 0

and we’ve long had streams that do one of those… we even have streams that can end up doing either behavior given the situation. That goes all the way back to .NET Framework 1.0. Streams following one of these two behaviors is not new. In .NET Framework, for example, DeflateStream implements (1), PipeStream implements (2), and FileStream implements either (1) or (2) depending on what kind of file it’s backed by.

stephentoub on Jul 17, 2023

The current documentation for Stream doesn’t allow for zero byte reads as it explicitly says it only returns when at least one byte has been read or you hit the end of the stream. Zero byte read optimizations are counter to documented behavior.

Changing the expectation that all existing Stream implementations must support zero byte reads without any change in documentation, or stating as such in any breaking changes documentation doesn’t seem like the best approach for the extended eco system.

And as I stated, in this particular case, as the intention was to improve memory consumption, an even more memory efficient implementation would actually eliminate the zero byte read.

mconnew on Jul 13, 2023