Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnableBuffering/FileBufferingReadStream doesn't support 0-byte reads #41287

Closed
1 task done
Tratcher opened this issue Apr 20, 2022 · 1 comment
Closed
1 task done

EnableBuffering/FileBufferingReadStream doesn't support 0-byte reads #41287

Tratcher opened this issue Apr 20, 2022 · 1 comment
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug.
Milestone

Comments

@Tratcher
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The 0-byte read pattern does not work with FileBufferingReadStream.

On first read with a zero-length buffer it returns 0, but that causes the stream to think it's completely buffered and to stop reading from the inner stream.

if (read > 0)
{
await _buffer.WriteAsync(buffer.Slice(0, read), cancellationToken);
}
else
{
_completelyBuffered = true;
}

Expected Behavior

0-byte reads should return 0, but subsequent reads should still return data.

Steps To Reproduce


    [Fact]
    public async Task FileBufferingReadStream_Async0ByteReadUnderThreshold_DoesntCreateFile()
    {
        var inner = MakeStream(1024);
        using (var stream = new FileBufferingReadStream(inner, 1024 * 2, null, Directory.GetCurrentDirectory()))
        {
            var bytes = new byte[1000];
            var read0 = await stream.ReadAsync(bytes, 0, 0);
            Assert.Equal(0, read0);
            Assert.Equal(read0, stream.Length);
            Assert.Equal(read0, stream.Position);
            Assert.True(stream.InMemory);
            Assert.Null(stream.TempFileName);

            var read1 = await stream.ReadAsync(bytes, 0, bytes.Length);
            Assert.Equal(bytes.Length, read1);
            Assert.Equal(read0 + read1, stream.Length);
            Assert.Equal(read0 + read1, stream.Position);
            Assert.True(stream.InMemory);
            Assert.Null(stream.TempFileName);

            var read2 = await stream.ReadAsync(bytes, 0, bytes.Length);
            Assert.Equal(inner.Length - read0 - read1, read2);
            Assert.Equal(read0 + read1 + read2, stream.Length);
            Assert.Equal(read0 + read1 + read2, stream.Position);
            Assert.True(stream.InMemory);
            Assert.Null(stream.TempFileName);

            var read3 = await stream.ReadAsync(bytes, 0, bytes.Length);
            Assert.Equal(0, read3);
        }
    }

Exceptions (if any)

No response

.NET Version

6.0

Anything else?

Discovered by YARP users: dotnet/yarp#1657

@Tratcher Tratcher self-assigned this Apr 20, 2022
@Tratcher Tratcher added help wanted Up for grabs. We would accept a PR to help resolve this issue area-runtime good first issue Good for newcomers. labels Apr 20, 2022
@Tratcher Tratcher added the bug This issue describes a behavior which is not expected - a bug. label Apr 20, 2022
@Tratcher Tratcher changed the title FileBufferingReadStream doesn't support 0-byte reads EnableBuffering/FileBufferingReadStream doesn't support 0-byte reads Apr 20, 2022
@Tratcher Tratcher removed their assignment Apr 20, 2022
@adityamandaleeka adityamandaleeka added this to the 7.0-preview5 milestone Apr 20, 2022
@adityamandaleeka
Copy link
Member

We should consider backporting this to 6.

Tratcher added a commit to Tratcher/aspnetcore that referenced this issue Apr 21, 2022
@Tratcher Tratcher modified the milestones: 7.0-preview5, 6.0.x Apr 25, 2022
@Tratcher Tratcher removed help wanted Up for grabs. We would accept a PR to help resolve this issue good first issue Good for newcomers. labels Apr 25, 2022
@Tratcher Tratcher self-assigned this Apr 27, 2022
Tratcher added a commit to Tratcher/aspnetcore that referenced this issue Apr 27, 2022
@Tratcher Tratcher modified the milestones: 6.0.x, 6.0.6 May 5, 2022
@Tratcher Tratcher closed this as completed May 5, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 4, 2022
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug.
Projects
None yet
Development

No branches or pull requests

3 participants