Skip to content

Commit 35e5294

Browse files
[release/6.0] Don't reuse Pipe buffer when body is read (#39376)
* Don't reuse Pipe buffer when body is read Fixes #38771 * Fix spacing after merge * Fix spacing * Update Http1ContentLengthMessageBody.cs Co-authored-by: Sebastien Ros <[email protected]>
1 parent 340a143 commit 35e5294

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Buffers;
56
using System.Diagnostics;
67
using System.Globalization;
78
using System.IO.Pipelines;
@@ -213,17 +214,25 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami
213214

214215
_isReading = false;
215216

217+
// The current body is read, though there might be more bytes to read on the stream with pipelining
216218
if (_readCompleted)
217219
{
218-
// If the old stored _readResult was canceled, it's already been observed. Do not store a canceled read result permanently.
219-
_readResult = new ReadResult(_readResult.Buffer.Slice(consumed, _readResult.Buffer.End), isCanceled: false, isCompleted: true);
220+
var buffer = _readResult.Buffer.Slice(consumed, _readResult.Buffer.End);
220221

221-
if (!_finalAdvanceCalled && _readResult.Buffer.Length == 0)
222+
if (!_finalAdvanceCalled && buffer.IsEmpty)
222223
{
224+
// Don't reference the old buffer as it will be released by the pipe after calling AdvanceTo
225+
_readResult = new ReadResult(new ReadOnlySequence<byte>(), isCanceled: false, isCompleted: true);
226+
223227
_context.Input.AdvanceTo(consumed);
224228
_finalAdvanceCalled = true;
225229
_context.OnTrailersComplete();
226230
}
231+
else
232+
{
233+
// If the old stored _readResult was canceled, it's already been observed. Do not store a canceled read result permanently.
234+
_readResult = new ReadResult(buffer, isCanceled: false, isCompleted: true);
235+
}
227236

228237
return;
229238
}

src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,6 +1083,43 @@ await connection.ReceiveEnd(
10831083
}
10841084
}
10851085

1086+
[Fact]
1087+
public async Task ContentLengthReadAsyncPipeReaderReadsCompletedBody()
1088+
{
1089+
var testContext = new TestServiceContext(LoggerFactory);
1090+
1091+
await using (var server = new TestServer(async httpContext =>
1092+
{
1093+
using var ms1 = new MemoryStream();
1094+
using var ms2 = new MemoryStream();
1095+
1096+
// Read the body completely, and ensure the second read doesn't fail
1097+
await httpContext.Request.BodyReader.CopyToAsync(ms1);
1098+
await httpContext.Request.BodyReader.CopyToAsync(ms2);
1099+
1100+
Assert.Equal(22, ms1.ToArray().Length);
1101+
Assert.Empty(ms2.ToArray());
1102+
}, testContext))
1103+
{
1104+
using (var connection = server.CreateConnection())
1105+
{
1106+
await connection.Send(
1107+
"POST / HTTP/1.0",
1108+
"Host:",
1109+
"Content-Length: 22",
1110+
"",
1111+
"MyVariableOne=ValueOne");
1112+
await connection.Receive(
1113+
"HTTP/1.1 200 OK",
1114+
"Content-Length: 0",
1115+
"Connection: close",
1116+
$"Date: {testContext.DateHeaderValue}",
1117+
"",
1118+
"");
1119+
}
1120+
}
1121+
}
1122+
10861123
[Fact]
10871124
public async Task ContentLengthReadAsyncSingleBytesAtATime()
10881125
{

0 commit comments

Comments
 (0)