From 88257b67d757ac86c8b4c8abe99d1fe8f1de4789 Mon Sep 17 00:00:00 2001 From: Radek Falhar Date: Sun, 22 Mar 2020 11:20:08 +0100 Subject: [PATCH 1/5] Made TestHost fail when Response Flush is called with AllowSynchronousIO being dissabled. --- .../TestHost/src/ResponseBodyWriterStream.cs | 5 ++ .../TestHost/test/ResponseBodyTests.cs | 76 +++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/Hosting/TestHost/src/ResponseBodyWriterStream.cs b/src/Hosting/TestHost/src/ResponseBodyWriterStream.cs index 8c3d61c561c2..2be73e0650c7 100644 --- a/src/Hosting/TestHost/src/ResponseBodyWriterStream.cs +++ b/src/Hosting/TestHost/src/ResponseBodyWriterStream.cs @@ -46,6 +46,11 @@ public override void SetLength(long value) public override void Flush() { + if (!_allowSynchronousIO()) + { + throw new InvalidOperationException("Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true."); + } + FlushAsync().GetAwaiter().GetResult(); } diff --git a/src/Hosting/TestHost/test/ResponseBodyTests.cs b/src/Hosting/TestHost/test/ResponseBodyTests.cs index fd7ff2dd2995..718187a6cb4d 100644 --- a/src/Hosting/TestHost/test/ResponseBodyTests.cs +++ b/src/Hosting/TestHost/test/ResponseBodyTests.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; +using System.Net.Http; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; @@ -46,6 +48,80 @@ public async Task BodyWriter_StartAsyncGetMemoryAdvance_AutoCompleted() Assert.Equal(length, bytes.Length); } + [Fact] + public async Task BodyStream_fails_synchronous_write_when_not_enabled() + { + var contentBytes = new byte[] {32}; + using var host = await CreateHost(async httpContext => + { + await httpContext.Response.StartAsync(); + httpContext.Response.Body.Write(contentBytes, 0, contentBytes.Length); + await httpContext.Response.CompleteAsync(); + }); + + var client = host.GetTestServer().CreateClient(); + var ex = await Assert.ThrowsAsync(()=> client.GetAsync("/")); + Assert.Contains("Synchronous operations are disallowed.", ex.Message); + } + + [Fact] + public async Task BodyStream_succeeds_synchronous_write_when_enabled() + { + var contentBytes = new byte[] {32}; + using var host = await CreateHost(async httpContext => + { + await httpContext.Response.StartAsync(); + httpContext.Response.Body.Write(contentBytes, 0, contentBytes.Length); + await httpContext.Response.CompleteAsync(); + }); + + host.GetTestServer().AllowSynchronousIO = true; + + var client = host.GetTestServer().CreateClient(); + var response = await client.GetAsync("/"); + var responseBytes = await response.Content.ReadAsByteArrayAsync(); + Assert.Equal(contentBytes, responseBytes); + } + + [Fact] + public async Task BodyStream_fails_synchronous_flush_when_not_enabled() + { + var contentBytes = new byte[] {32}; + using var host = await CreateHost(async httpContext => + { + await httpContext.Response.StartAsync(); + await httpContext.Response.Body.WriteAsync(contentBytes, 0, contentBytes.Length); + httpContext.Response.Body.Flush(); + await httpContext.Response.CompleteAsync(); + }); + + var client = host.GetTestServer().CreateClient(); + var requestException = await Assert.ThrowsAsync(()=> client.GetAsync("/")); + var ex = (InvalidOperationException) requestException?.InnerException?.InnerException; + Assert.NotNull(ex); + Assert.Contains("Synchronous operations are disallowed.", ex.Message); + } + + [Fact] + public async Task BodyStream_succeeds_synchronous_flush_when_enabled() + { + var contentBytes = new byte[] {32}; + using var host = await CreateHost(async httpContext => + { + await httpContext.Response.StartAsync(); + await httpContext.Response.Body.WriteAsync(contentBytes, 0, contentBytes.Length); + httpContext.Response.Body.Flush(); + await httpContext.Response.CompleteAsync(); + }); + + host.GetTestServer().AllowSynchronousIO = true; + + var client = host.GetTestServer().CreateClient(); + var response = await client.GetAsync("/"); + var responseBytes = await response.Content.ReadAsByteArrayAsync(); + Assert.Equal(contentBytes, responseBytes); + } + private Task CreateHost(RequestDelegate appDelegate) { return new HostBuilder() From 8d4e188b220cbe4152af743f30417a8b633f2e2f Mon Sep 17 00:00:00 2001 From: Radek Falhar Date: Wed, 25 Mar 2020 07:38:46 +0100 Subject: [PATCH 2/5] TestHost test should use asynchronous Flush on Response Body. --- src/Hosting/TestHost/test/ClientHandlerTests.cs | 14 +++++++------- .../TestHost/test/HttpContextBuilderTests.cs | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Hosting/TestHost/test/ClientHandlerTests.cs b/src/Hosting/TestHost/test/ClientHandlerTests.cs index 5cf9664b69e4..e94ffc16f401 100644 --- a/src/Hosting/TestHost/test/ClientHandlerTests.cs +++ b/src/Hosting/TestHost/test/ClientHandlerTests.cs @@ -289,7 +289,7 @@ public async Task FlushSendsHeaders() var handler = new ClientHandler(PathString.Empty, new DummyApplication(async context => { context.Response.Headers["TestHeader"] = "TestValue"; - context.Response.Body.Flush(); + await context.Response.Body.FlushAsync(); await block.Task; await context.Response.WriteAsync("BodyFinished"); })); @@ -305,11 +305,11 @@ public async Task FlushSendsHeaders() public async Task ClientDisposalCloses() { var block = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var handler = new ClientHandler(PathString.Empty, new DummyApplication(context => + var handler = new ClientHandler(PathString.Empty, new DummyApplication(async context => { context.Response.Headers["TestHeader"] = "TestValue"; - context.Response.Body.Flush(); - return block.Task; + await context.Response.Body.FlushAsync(); + await block.Task; })); var httpClient = new HttpClient(handler); HttpResponseMessage response = await httpClient.GetAsync("https://example.com/", @@ -327,11 +327,11 @@ public async Task ClientDisposalCloses() public async Task ClientCancellationAborts() { var block = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var handler = new ClientHandler(PathString.Empty, new DummyApplication(context => + var handler = new ClientHandler(PathString.Empty, new DummyApplication(async context => { context.Response.Headers["TestHeader"] = "TestValue"; - context.Response.Body.Flush(); - return block.Task; + await context.Response.Body.FlushAsync(); + await block.Task; })); var httpClient = new HttpClient(handler); HttpResponseMessage response = await httpClient.GetAsync("https://example.com/", diff --git a/src/Hosting/TestHost/test/HttpContextBuilderTests.cs b/src/Hosting/TestHost/test/HttpContextBuilderTests.cs index 7adc75329e4d..d1e740dafbd9 100644 --- a/src/Hosting/TestHost/test/HttpContextBuilderTests.cs +++ b/src/Hosting/TestHost/test/HttpContextBuilderTests.cs @@ -176,7 +176,7 @@ public async Task FlushSendsHeaders() app.Run(async c => { c.Response.Headers["TestHeader"] = "TestValue"; - c.Response.Body.Flush(); + await c.Response.Body.FlushAsync(); await block.Task; await c.Response.WriteAsync("BodyFinished"); }); @@ -198,7 +198,7 @@ public async Task ClientDisposalCloses() app.Run(async c => { c.Response.Headers["TestHeader"] = "TestValue"; - c.Response.Body.Flush(); + await c.Response.Body.FlushAsync(); await block.Task; await c.Response.WriteAsync("BodyFinished"); }); @@ -247,7 +247,7 @@ public async Task ClientCancellationAbortsReadAsync() app.Run(async c => { c.Response.Headers["TestHeader"] = "TestValue"; - c.Response.Body.Flush(); + await c.Response.Body.FlushAsync(); await block.Task; await c.Response.WriteAsync("BodyFinished"); }); From 3f62b886ab157373a0fc0778d242107e5379ac0c Mon Sep 17 00:00:00 2001 From: Radek Falhar Date: Wed, 25 Mar 2020 07:40:39 +0100 Subject: [PATCH 3/5] Fix test naming according to convention. --- src/Hosting/TestHost/test/ResponseBodyTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Hosting/TestHost/test/ResponseBodyTests.cs b/src/Hosting/TestHost/test/ResponseBodyTests.cs index 718187a6cb4d..114172d42a07 100644 --- a/src/Hosting/TestHost/test/ResponseBodyTests.cs +++ b/src/Hosting/TestHost/test/ResponseBodyTests.cs @@ -49,7 +49,7 @@ public async Task BodyWriter_StartAsyncGetMemoryAdvance_AutoCompleted() } [Fact] - public async Task BodyStream_fails_synchronous_write_when_not_enabled() + public async Task BodyStream_SyncDisabled_WriteThrows() { var contentBytes = new byte[] {32}; using var host = await CreateHost(async httpContext => @@ -65,7 +65,7 @@ public async Task BodyStream_fails_synchronous_write_when_not_enabled() } [Fact] - public async Task BodyStream_succeeds_synchronous_write_when_enabled() + public async Task BodyStream_SyncEnabled_WriteSucceeds() { var contentBytes = new byte[] {32}; using var host = await CreateHost(async httpContext => @@ -84,7 +84,7 @@ public async Task BodyStream_succeeds_synchronous_write_when_enabled() } [Fact] - public async Task BodyStream_fails_synchronous_flush_when_not_enabled() + public async Task BodyStream_SyncDisabled_FlushThrows() { var contentBytes = new byte[] {32}; using var host = await CreateHost(async httpContext => @@ -103,7 +103,7 @@ public async Task BodyStream_fails_synchronous_flush_when_not_enabled() } [Fact] - public async Task BodyStream_succeeds_synchronous_flush_when_enabled() + public async Task BodyStream_SyncEnabled_FlushSucceeds() { var contentBytes = new byte[] {32}; using var host = await CreateHost(async httpContext => From 20bd6f67cd09c2f83497c20d15db8e73e290fc5b Mon Sep 17 00:00:00 2001 From: Radek Falhar Date: Wed, 25 Mar 2020 08:08:15 +0100 Subject: [PATCH 4/5] ResponseCompression test should use FlushAsync on Response Body. --- .../test/ResponseCompressionMiddlewareTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs index 91ae0ce8fba0..cbc35250d7a4 100644 --- a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs +++ b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs @@ -592,7 +592,7 @@ public async Task FlushHeaders_SendsHeaders_Compresses(string encoding, int expe { context.Response.Headers[HeaderNames.ContentMD5] = "MD5"; context.Response.ContentType = TextPlain; - context.Response.Body.Flush(); + await context.Response.Body.FlushAsync(); await responseReceived.Task.TimeoutAfter(TimeSpan.FromSeconds(3)); await context.Response.WriteAsync(new string('a', 100)); }); From 8d28528cd9d8c30c39e0449bc394359f94920a79 Mon Sep 17 00:00:00 2001 From: Radek Falhar Date: Wed, 25 Mar 2020 19:40:36 +0100 Subject: [PATCH 5/5] Returned ResponseCompression Flush to synchronous, but with AllowSynchronousIO enabled. --- .../test/ResponseCompressionMiddlewareTest.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs index cbc35250d7a4..c6ad14a66c13 100644 --- a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs +++ b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs @@ -592,13 +592,16 @@ public async Task FlushHeaders_SendsHeaders_Compresses(string encoding, int expe { context.Response.Headers[HeaderNames.ContentMD5] = "MD5"; context.Response.ContentType = TextPlain; - await context.Response.Body.FlushAsync(); + context.Response.Body.Flush(); await responseReceived.Task.TimeoutAfter(TimeSpan.FromSeconds(3)); await context.Response.WriteAsync(new string('a', 100)); }); }); - var server = new TestServer(builder); + var server = new TestServer(builder) + { + AllowSynchronousIO = true // needed for synchronous flush + }; var client = server.CreateClient(); var request = new HttpRequestMessage(HttpMethod.Get, "");