diff --git a/src/Hosting/TestHost/src/TestServer.cs b/src/Hosting/TestHost/src/TestServer.cs index c5668185ac87..783184a8de41 100644 --- a/src/Hosting/TestHost/src/TestServer.cs +++ b/src/Hosting/TestHost/src/TestServer.cs @@ -81,9 +81,9 @@ public IWebHost Host /// Gets or sets a value that controls whether synchronous IO is allowed for the and /// /// - /// Defaults to true. + /// Defaults to false. /// - public bool AllowSynchronousIO { get; set; } = true; + public bool AllowSynchronousIO { get; set; } = false; private IHttpApplication Application { diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index 93d6e2dd8218..4ebf941ee7a8 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -136,9 +136,9 @@ public long? MaxRequestBodySize /// /// Gets or sets a value that controls whether synchronous IO is allowed for the HttpContext.Request.Body and HttpContext.Response.Body. - /// The default is `true`. + /// The default is `false`. /// - public bool AllowSynchronousIO { get; set; } = true; + public bool AllowSynchronousIO { get; set; } = false; /// /// Gets or sets a value that controls how http.sys reacts when rejecting requests due to throttling conditions - like when the request diff --git a/src/Servers/HttpSys/test/FunctionalTests/Listener/RequestBodyTests.cs b/src/Servers/HttpSys/test/FunctionalTests/Listener/RequestBodyTests.cs index c805818931f9..bdefc3b87f7d 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/Listener/RequestBodyTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/Listener/RequestBodyTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -17,26 +17,26 @@ namespace Microsoft.AspNetCore.Server.HttpSys.Listener public class RequestBodyTests { [ConditionalFact] - public async Task RequestBody_SyncReadEnabledByDefault_ThrowsWhenDisabled() + public async Task RequestBody_SyncReadDisabledByDefault_WorksWhenEnabled() { string address; using (var server = Utilities.CreateHttpServer(out address)) { Task responseTask = SendRequestAsync(address, "Hello World"); - Assert.True(server.Options.AllowSynchronousIO); + Assert.False(server.Options.AllowSynchronousIO); var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask); byte[] input = new byte[100]; + Assert.Throws(() => context.Request.Body.Read(input, 0, input.Length)); + + context.AllowSynchronousIO = true; Assert.True(context.AllowSynchronousIO); var read = context.Request.Body.Read(input, 0, input.Length); context.Response.ContentLength = read; context.Response.Body.Write(input, 0, read); - context.AllowSynchronousIO = false; - Assert.Throws(() => context.Request.Body.Read(input, 0, input.Length)); - string response = await responseTask; Assert.Equal("Hello World", response); } diff --git a/src/Servers/HttpSys/test/FunctionalTests/Listener/ResponseBodyTests.cs b/src/Servers/HttpSys/test/FunctionalTests/Listener/ResponseBodyTests.cs index af044d829702..be87889ee75f 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/Listener/ResponseBodyTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/Listener/ResponseBodyTests.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys.Listener public class ResponseBodyTests { [ConditionalFact] - public async Task ResponseBody_SyncWriteEnabledByDefault_ThrowsWhenDisabled() + public async Task ResponseBody_SyncWriteDisabledByDefault_WorksWhenEnabled() { string address; using (var server = Utilities.CreateHttpServer(out address)) @@ -26,19 +26,17 @@ public async Task ResponseBody_SyncWriteEnabledByDefault_ThrowsWhenDisabled() var context = await server.AcceptAsync(Utilities.DefaultTimeout).Before(responseTask); - Assert.True(context.AllowSynchronousIO); - - context.Response.Body.Flush(); - context.Response.Body.Write(new byte[10], 0, 10); - context.Response.Body.Flush(); - - context.AllowSynchronousIO = false; + Assert.False(context.AllowSynchronousIO); Assert.Throws(() => context.Response.Body.Flush()); Assert.Throws(() => context.Response.Body.Write(new byte[10], 0, 10)); Assert.Throws(() => context.Response.Body.Flush()); - await context.Response.Body.WriteAsync(new byte[10], 0, 10); + context.AllowSynchronousIO = true; + + context.Response.Body.Flush(); + context.Response.Body.Write(new byte[10], 0, 10); + context.Response.Body.Flush(); context.Dispose(); var response = await responseTask; @@ -47,7 +45,7 @@ public async Task ResponseBody_SyncWriteEnabledByDefault_ThrowsWhenDisabled() IEnumerable ignored; Assert.False(response.Content.Headers.TryGetValues("content-length", out ignored), "Content-Length"); Assert.True(response.Headers.TransferEncodingChunked.Value, "Chunked"); - Assert.Equal(new byte[20], await response.Content.ReadAsByteArrayAsync()); + Assert.Equal(new byte[10], await response.Content.ReadAsByteArrayAsync()); } } @@ -477,4 +475,4 @@ public async Task ResponseBody_ClientDisconnectsBeforeSecondWriteAsync_WriteComp } } } -} \ No newline at end of file +} diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.FeatureCollection.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.FeatureCollection.cs index b2e39fccd7f6..1f872b655fa5 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.FeatureCollection.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.FeatureCollection.cs @@ -320,7 +320,7 @@ unsafe X509Certificate2 ITlsConnectionFeature.ClientCertificate IEnumerator IEnumerable.GetEnumerator() => FastEnumerable().GetEnumerator(); - bool IHttpBodyControlFeature.AllowSynchronousIO { get; set; } = true; + bool IHttpBodyControlFeature.AllowSynchronousIO { get; set; } void IHttpBufferingFeature.DisableRequestBuffering() { diff --git a/src/Servers/IIS/IIS/src/IISServerOptions.cs b/src/Servers/IIS/IIS/src/IISServerOptions.cs index 47d7619f8a51..65f86240bdf3 100644 --- a/src/Servers/IIS/IIS/src/IISServerOptions.cs +++ b/src/Servers/IIS/IIS/src/IISServerOptions.cs @@ -11,9 +11,9 @@ public class IISServerOptions /// Gets or sets a value that controls whether synchronous IO is allowed for the and /// /// - /// Defaults to true. + /// Defaults to false. /// - public bool AllowSynchronousIO { get; set; } = true; + public bool AllowSynchronousIO { get; set; } = false; /// /// If true the server should set HttpContext.User. If false the server will only provide an diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ConnectionIdFeatureTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ConnectionIdFeatureTests.cs index 37e69b3a32d1..27688b5e588d 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ConnectionIdFeatureTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ConnectionIdFeatureTests.cs @@ -1,7 +1,6 @@ // 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.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Testing.xunit; @@ -11,44 +10,22 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { [SkipIfHostableWebCoreNotAvailable] [OSSkipCondition(OperatingSystems.Windows, WindowsVersions.Win7, "https://github.com/aspnet/IISIntegration/issues/866")] - public class HttpBodyControlFeatureTests : StrictTestServerTests + public class ConnectionIdFeatureTests : StrictTestServerTests { [ConditionalFact] - public async Task ThrowsOnSyncReadOrWrite() + public async Task ProvidesConnectionId() { - Exception writeException = null; - Exception readException = null; - using (var testServer = await TestServer.Create( - ctx => { - var bodyControl = ctx.Features.Get(); - bodyControl.AllowSynchronousIO = false; - - try - { - ctx.Response.Body.Write(new byte[10]); - } - catch (Exception ex) - { - writeException = ex; - } - - try - { - ctx.Request.Body.Read(new byte[10]); - } - catch (Exception ex) - { - readException = ex; - } - - return Task.CompletedTask; - }, LoggerFactory)) + string connectionId = null; + using (var testServer = await TestServer.Create(ctx => { + var connectionIdFeature = ctx.Features.Get(); + connectionId = connectionIdFeature.ConnectionId; + return Task.CompletedTask; + }, LoggerFactory)) { await testServer.HttpClient.GetStringAsync("/"); } - Assert.IsType(readException); - Assert.IsType(writeException); + Assert.NotNull(connectionId); } } } diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/HttpBodyControlFeatureTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/HttpBodyControlFeatureTests.cs index 3d91c445caf7..8ac359a21bd4 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/HttpBodyControlFeatureTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/HttpBodyControlFeatureTests.cs @@ -1,6 +1,7 @@ // 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.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Testing.xunit; @@ -10,22 +11,44 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { [SkipIfHostableWebCoreNotAvailable] [OSSkipCondition(OperatingSystems.Windows, WindowsVersions.Win7, "https://github.com/aspnet/IISIntegration/issues/866")] - public class ConnectionIdFeatureTests : StrictTestServerTests + public class HttpBodyControlFeatureTests : StrictTestServerTests { [ConditionalFact] - public async Task ProvidesConnectionId() + public async Task ThrowsOnSyncReadOrWrite() { - string connectionId = null; - using (var testServer = await TestServer.Create(ctx => { - var connectionIdFeature = ctx.Features.Get(); - connectionId = connectionIdFeature.ConnectionId; + Exception writeException = null; + Exception readException = null; + using (var testServer = await TestServer.Create( + ctx => { + var bodyControl = ctx.Features.Get(); + Assert.False(bodyControl.AllowSynchronousIO); + + try + { + ctx.Response.Body.Write(new byte[10]); + } + catch (Exception ex) + { + writeException = ex; + } + + try + { + ctx.Request.Body.Read(new byte[10]); + } + catch (Exception ex) + { + readException = ex; + } + return Task.CompletedTask; }, LoggerFactory)) { await testServer.HttpClient.GetStringAsync("/"); } - Assert.NotNull(connectionId); + Assert.IsType(readException); + Assert.IsType(writeException); } } } diff --git a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs index 0471b0d1c9bb..aa36ee7ff1ff 100644 --- a/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs +++ b/src/Servers/Kestrel/Core/src/KestrelServerOptions.cs @@ -50,9 +50,9 @@ public class KestrelServerOptions /// Gets or sets a value that controls whether synchronous IO is allowed for the and /// /// - /// Defaults to true. + /// Defaults to false. /// - public bool AllowSynchronousIO { get; set; } = true; + public bool AllowSynchronousIO { get; set; } = false; /// /// Enables the Listen options callback to resolve and use services registered by the application during startup. diff --git a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs index 75739a4c7bbc..02e52db7f0a5 100644 --- a/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs +++ b/src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs @@ -23,11 +23,11 @@ public void NoDelayDefaultsToTrue() } [Fact] - public void AllowSynchronousIODefaultsToTrue() + public void AllowSynchronousIODefaultsToFalse() { var options = new KestrelServerOptions(); - Assert.True(options.AllowSynchronousIO); + Assert.False(options.AllowSynchronousIO); } [Fact] @@ -65,4 +65,4 @@ public void ConfigureEndpointDefaultsAppliesToNewEndpoints() Assert.False(options.ListenOptions[3].NoDelay); } } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index a89f5526960c..cdeea5500029 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -1026,45 +1026,29 @@ await connection.ReceiveEnd( } [Fact] - public async Task SynchronousReadsAllowedByDefault() + public async Task SynchronousReadsDisallowedByDefault() { - var firstRequest = true; - using (var server = new TestServer(async context => { var bodyControlFeature = context.Features.Get(); - Assert.True(bodyControlFeature.AllowSynchronousIO); + Assert.False(bodyControlFeature.AllowSynchronousIO); var buffer = new byte[6]; var offset = 0; // The request body is 5 bytes long. The 6th byte (buffer[5]) is only used for writing the response body. - buffer[5] = (byte)(firstRequest ? '1' : '2'); + buffer[5] = (byte)'1'; - if (firstRequest) - { - while (offset < 5) - { - offset += context.Request.Body.Read(buffer, offset, 5 - offset); - } - - firstRequest = false; - } - else - { - bodyControlFeature.AllowSynchronousIO = false; - - // Synchronous reads now throw. - var ioEx = Assert.Throws(() => context.Request.Body.Read(new byte[1], 0, 1)); - Assert.Equal(CoreStrings.SynchronousReadsDisallowed, ioEx.Message); + // Synchronous reads throw. + var ioEx = Assert.Throws(() => context.Request.Body.Read(new byte[1], 0, 1)); + Assert.Equal(CoreStrings.SynchronousReadsDisallowed, ioEx.Message); - var ioEx2 = Assert.Throws(() => context.Request.Body.CopyTo(Stream.Null)); - Assert.Equal(CoreStrings.SynchronousReadsDisallowed, ioEx2.Message); + var ioEx2 = Assert.Throws(() => context.Request.Body.CopyTo(Stream.Null)); + Assert.Equal(CoreStrings.SynchronousReadsDisallowed, ioEx2.Message); - while (offset < 5) - { - offset += await context.Request.Body.ReadAsync(buffer, offset, 5 - offset); - } + while (offset < 5) + { + offset += await context.Request.Body.ReadAsync(buffer, offset, 5 - offset); } Assert.Equal(0, await context.Request.Body.ReadAsync(new byte[1], 0, 1)); @@ -1081,21 +1065,56 @@ await connection.Send( "Host:", "Content-Length: 5", "", - "HelloPOST / HTTP/1.1", - "Host:", - "Content-Length: 5", - "", "Hello"); await connection.Receive( "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", "Content-Length: 6", "", - "Hello1HTTP/1.1 200 OK", + "Hello1"); + } + } + } + + [Fact] + public async Task SynchronousReadsAllowedByOptIn() + { + using (var server = new TestServer(async context => + { + var bodyControlFeature = context.Features.Get(); + Assert.False(bodyControlFeature.AllowSynchronousIO); + + var buffer = new byte[5]; + var offset = 0; + + bodyControlFeature.AllowSynchronousIO = true; + + while (offset < 5) + { + offset += context.Request.Body.Read(buffer, offset, 5 - offset); + } + + Assert.Equal(0, await context.Request.Body.ReadAsync(new byte[1], 0, 1)); + Assert.Equal("Hello", Encoding.ASCII.GetString(buffer, 0, 5)); + + context.Response.ContentLength = 5; + await context.Response.Body.WriteAsync(buffer, 0, 5); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Content-Length: 5", + "", + "Hello"); + await connection.Receive( + "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", - "Content-Length: 6", + "Content-Length: 5", "", - "Hello2"); + "Hello"); } await server.StopAsync(); } @@ -1146,6 +1165,48 @@ await connection.Receive( } } + [Fact] + public async Task SynchronousReadsCanBeAllowedGlobally() + { + var testContext = new TestServiceContext(LoggerFactory) + { + ServerOptions = { AllowSynchronousIO = true } + }; + + using (var server = new TestServer(async context => + { + var bodyControlFeature = context.Features.Get(); + Assert.True(bodyControlFeature.AllowSynchronousIO); + + int offset = 0; + var buffer = new byte[5]; + while (offset < 5) + { + offset += context.Request.Body.Read(buffer, offset, 5 - offset); + } + + Assert.Equal(0, await context.Request.Body.ReadAsync(new byte[1], 0, 1)); + Assert.Equal("Hello", Encoding.ASCII.GetString(buffer, 0, 5)); + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Content-Length: 5", + "", + "Hello"); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + } + } + public static TheoryData HostHeaderData => HttpParsingData.HostHeaderData; } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index a098277e687b..70fd69dd05bc 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -2347,34 +2347,48 @@ await connection.Receive( Assert.Equal(2, callOrder.Pop()); } - [Fact] - public async Task SynchronousWritesAllowedByDefault() + public async Task SynchronousWritesDisallowedByDefault() { - var firstRequest = true; - using (var server = new TestServer(async context => { var bodyControlFeature = context.Features.Get(); - Assert.True(bodyControlFeature.AllowSynchronousIO); + Assert.False(bodyControlFeature.AllowSynchronousIO); context.Response.ContentLength = 6; - if (firstRequest) + // Synchronous writes now throw. + var ioEx = Assert.Throws(() => context.Response.Body.Write(Encoding.ASCII.GetBytes("What!?"), 0, 6)); + Assert.Equal(CoreStrings.SynchronousWritesDisallowed, ioEx.Message); + await context.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello1"), 0, 6); + + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) { - context.Response.Body.Write(Encoding.ASCII.GetBytes("Hello1"), 0, 6); - firstRequest = false; + await connection.SendEmptyGet(); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 6", + "", + "Hello1"); } - else - { - bodyControlFeature.AllowSynchronousIO = false; + } + } - // Synchronous writes now throw. - var ioEx = Assert.Throws(() => context.Response.Body.Write(Encoding.ASCII.GetBytes("What!?"), 0, 6)); - Assert.Equal(CoreStrings.SynchronousWritesDisallowed, ioEx.Message); + [Fact] + public async Task SynchronousWritesAllowedByOptIn() + { + using (var server = new TestServer(context => + { + var bodyControlFeature = context.Features.Get(); + Assert.False(bodyControlFeature.AllowSynchronousIO); + bodyControlFeature.AllowSynchronousIO = true; + context.Response.ContentLength = 6; - await context.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello2"), 0, 6); - } + context.Response.Body.Write(Encoding.ASCII.GetBytes("Hello1"), 0, 6); + return Task.CompletedTask; }, new TestServiceContext(LoggerFactory))) { using (var connection = server.CreateConnection()) @@ -2386,14 +2400,41 @@ await connection.Receive( "Content-Length: 6", "", "Hello1"); + } + } + } - await connection.SendEmptyGet(); + [Fact] + public async Task SynchronousWritesCanBeAllowedGlobally() + { + var testContext = new TestServiceContext(LoggerFactory) + { + ServerOptions = { AllowSynchronousIO = true } + }; + + using (var server = new TestServer(context => + { + var bodyControlFeature = context.Features.Get(); + Assert.True(bodyControlFeature.AllowSynchronousIO); + + context.Response.ContentLength = 6; + context.Response.Body.Write(Encoding.ASCII.GetBytes("Hello!"), 0, 6); + return Task.CompletedTask; + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); await connection.Receive( "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", "Content-Length: 6", "", - "Hello2"); + "Hello!"); } await server.StopAsync(); }