Skip to content

Commit 1a7b2c2

Browse files
committed
Feedback
1 parent 177d34d commit 1a7b2c2

File tree

3 files changed

+201
-25
lines changed

3 files changed

+201
-25
lines changed

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -195,48 +195,48 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe
195195
{
196196
get
197197
{
198-
if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody))
199-
{
200-
var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool);
201-
ResponsePipeWriter = responsePipeWriter;
202-
_cachedResponseBodyStream = ResponseBody;
203-
if (_wrapperObjectsToDispose == null)
204-
{
205-
_wrapperObjectsToDispose = new List<IDisposable>();
206-
}
207-
_wrapperObjectsToDispose.Add(responsePipeWriter);
208-
}
209-
210198
return ResponsePipeWriter;
211199
}
212200
set
213201
{
214202
ResponsePipeWriter = value;
215-
}
216-
}
217203

218-
Stream IHttpResponseFeature.Body
219-
{
220-
get
221-
{
222204
if (!object.ReferenceEquals(_cachedResponsePipeWriter, ResponsePipeWriter))
223205
{
224206
var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter);
225207
ResponseBody = responseBody;
226-
_cachedResponsePipeWriter = ResponsePipeWriter;
227-
if (_wrapperObjectsToDispose == null)
228-
{
229-
_wrapperObjectsToDispose = new List<IDisposable>();
230-
}
231-
_wrapperObjectsToDispose.Add(responseBody);
208+
209+
RegisterWrapperForDisposal(responseBody);
232210
}
211+
}
212+
}
233213

214+
Stream IHttpResponseFeature.Body
215+
{
216+
get
217+
{
234218
return ResponseBody;
235219
}
236220
set
237221
{
238222
ResponseBody = value;
223+
if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody))
224+
{
225+
var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool);
226+
ResponsePipeWriter = responsePipeWriter;
227+
_cachedResponseBodyStream = ResponseBody;
228+
RegisterWrapperForDisposal(responsePipeWriter);
229+
}
230+
}
231+
}
232+
233+
private void RegisterWrapperForDisposal(IDisposable responseBody)
234+
{
235+
if (_wrapperObjectsToDispose == null)
236+
{
237+
_wrapperObjectsToDispose = new List<IDisposable>();
239238
}
239+
_wrapperObjectsToDispose.Add(responseBody);
240240
}
241241

242242
protected void ResetHttp1Features()

src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ private async ValueTask<FlushResult> WriteDataAsync(int streamId, StreamOutputFl
319319
while (dataLength > 0)
320320
{
321321
OutputFlowControlAwaitable availabilityAwaitable;
322-
var writeTask = new ValueTask<FlushResult>(new FlushResult());
322+
var writeTask = default(ValueTask<FlushResult>);
323323

324324
lock (_writeLock)
325325
{

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

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3487,6 +3487,182 @@ await connection.Receive(
34873487
}
34883488
}
34893489

3490+
[Fact]
3491+
public async Task ResponseSetBodyAndPipeBodyIsWrapped()
3492+
{
3493+
using (var server = new TestServer(async httpContext =>
3494+
{
3495+
httpContext.Response.Body = new MemoryStream();
3496+
httpContext.Response.BodyPipe = new Pipe().Writer;
3497+
Assert.IsType<WriteOnlyPipeStream>(httpContext.Response.Body);
3498+
3499+
await Task.CompletedTask;
3500+
}, new TestServiceContext(LoggerFactory)))
3501+
{
3502+
using (var connection = server.CreateConnection())
3503+
{
3504+
await connection.Send(
3505+
"GET / HTTP/1.1",
3506+
"Host:",
3507+
"",
3508+
"");
3509+
await connection.Receive(
3510+
"HTTP/1.1 200 OK",
3511+
$"Date: {server.Context.DateHeaderValue}",
3512+
"Content-Length: 0",
3513+
"",
3514+
"");
3515+
}
3516+
await server.StopAsync();
3517+
}
3518+
}
3519+
3520+
[Fact]
3521+
public async Task ResponseSetPipeAndBodyPipeIsWrapped()
3522+
{
3523+
using (var server = new TestServer(async httpContext =>
3524+
{
3525+
httpContext.Response.BodyPipe = new Pipe().Writer;
3526+
httpContext.Response.Body = new MemoryStream();
3527+
Assert.IsType<StreamPipeWriter>(httpContext.Response.BodyPipe);
3528+
Assert.IsType<MemoryStream>(httpContext.Response.Body);
3529+
3530+
await Task.CompletedTask;
3531+
}, new TestServiceContext(LoggerFactory)))
3532+
{
3533+
using (var connection = server.CreateConnection())
3534+
{
3535+
await connection.Send(
3536+
"GET / HTTP/1.1",
3537+
"Host:",
3538+
"",
3539+
"");
3540+
await connection.Receive(
3541+
"HTTP/1.1 200 OK",
3542+
$"Date: {server.Context.DateHeaderValue}",
3543+
"Content-Length: 0",
3544+
"",
3545+
"");
3546+
}
3547+
await server.StopAsync();
3548+
}
3549+
}
3550+
3551+
[Fact]
3552+
public async Task ResponseWriteToBodyPipeAndStreamAllBlocksDisposed()
3553+
{
3554+
using (var server = new TestServer(async httpContext =>
3555+
{
3556+
for (var i = 0; i < 3; i++)
3557+
{
3558+
httpContext.Response.BodyPipe = new Pipe().Writer;
3559+
await httpContext.Response.Body.WriteAsync(new byte[1]);
3560+
httpContext.Response.Body = new MemoryStream();
3561+
await httpContext.Response.BodyPipe.WriteAsync(new byte[1]);
3562+
}
3563+
3564+
// TestMemoryPool will confirm that all rented blocks have been disposed, meaning dispose was called.
3565+
3566+
await Task.CompletedTask;
3567+
}, new TestServiceContext(LoggerFactory)))
3568+
{
3569+
using (var connection = server.CreateConnection())
3570+
{
3571+
await connection.Send(
3572+
"GET / HTTP/1.1",
3573+
"Host:",
3574+
"",
3575+
"");
3576+
await connection.Receive(
3577+
"HTTP/1.1 200 OK",
3578+
$"Date: {server.Context.DateHeaderValue}",
3579+
"Content-Length: 0",
3580+
"",
3581+
"");
3582+
}
3583+
await server.StopAsync();
3584+
}
3585+
}
3586+
3587+
[Fact]
3588+
public async Task ResponseStreamWrappingWorks()
3589+
{
3590+
using (var server = new TestServer(async httpContext =>
3591+
{
3592+
var oldBody = httpContext.Response.Body;
3593+
httpContext.Response.Body = new MemoryStream();
3594+
3595+
await httpContext.Response.BodyPipe.WriteAsync(new byte[1]);
3596+
await httpContext.Response.Body.WriteAsync(new byte[1]);
3597+
3598+
Assert.Equal(2, httpContext.Response.Body.Length);
3599+
3600+
httpContext.Response.Body = oldBody;
3601+
3602+
// Even though we are restoring the original response body, we will create a
3603+
// wrapper rather than restoring the original pipe.
3604+
Assert.IsType<StreamPipeWriter>(httpContext.Response.BodyPipe);
3605+
3606+
}, new TestServiceContext(LoggerFactory)))
3607+
{
3608+
using (var connection = server.CreateConnection())
3609+
{
3610+
await connection.Send(
3611+
"GET / HTTP/1.1",
3612+
"Host:",
3613+
"",
3614+
"");
3615+
await connection.Receive(
3616+
"HTTP/1.1 200 OK",
3617+
$"Date: {server.Context.DateHeaderValue}",
3618+
"Content-Length: 0",
3619+
"",
3620+
"");
3621+
}
3622+
await server.StopAsync();
3623+
}
3624+
}
3625+
3626+
[Fact]
3627+
public async Task ResponsePipeWrappingWorks()
3628+
{
3629+
using (var server = new TestServer(async httpContext =>
3630+
{
3631+
var oldPipeWriter = httpContext.Response.BodyPipe;
3632+
var pipe = new Pipe();
3633+
httpContext.Response.BodyPipe = pipe.Writer;
3634+
3635+
await httpContext.Response.Body.WriteAsync(new byte[1]);
3636+
await httpContext.Response.BodyPipe.WriteAsync(new byte[1]);
3637+
3638+
var readResult = await pipe.Reader.ReadAsync();
3639+
Assert.Equal(2, readResult.Buffer.Length);
3640+
3641+
httpContext.Response.BodyPipe = oldPipeWriter;
3642+
3643+
// Even though we are restoring the original response body, we will create a
3644+
// wrapper rather than restoring the original pipe.
3645+
Assert.IsType<WriteOnlyPipeStream>(httpContext.Response.Body);
3646+
}, new TestServiceContext(LoggerFactory)))
3647+
{
3648+
using (var connection = server.CreateConnection())
3649+
{
3650+
await connection.Send(
3651+
"GET / HTTP/1.1",
3652+
"Host:",
3653+
"",
3654+
"");
3655+
await connection.Receive(
3656+
"HTTP/1.1 200 OK",
3657+
$"Date: {server.Context.DateHeaderValue}",
3658+
"Content-Length: 0",
3659+
"",
3660+
"");
3661+
}
3662+
await server.StopAsync();
3663+
}
3664+
}
3665+
34903666
private static async Task ResponseStatusCodeSetBeforeHttpContextDispose(
34913667
ITestSink testSink,
34923668
ILoggerFactory loggerFactory,

0 commit comments

Comments
 (0)