From 93a24b03bbda1aa0ab9b553a50b70dd36d554934 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Fri, 15 Feb 2019 16:05:49 -0800 Subject: [PATCH] Disable AllowSynchronousIO by default in all servers #4774 (#5120) * Disable AllowSynchronousIO by default in all servers --- .../TestHost/src/AsyncStreamWrapper.cs | 14 +- src/Hosting/TestHost/src/TestServer.cs | 4 +- ...mlDataContractSerializerOutputFormatter.cs | 7 + .../src/XmlSerializerOutputFormatter.cs | 7 + .../src/ViewComponentResultExecutor.cs | 7 + .../BasicWebSite/StartupRequestLimitSize.cs | 34 ++++- .../FormatterWebSite/StringInputFormatter.cs | 6 +- src/Servers/HttpSys/src/HttpSysOptions.cs | 6 +- .../test/FunctionalTests/HttpsTests.cs | 9 +- .../Listener/RequestBodyTests.cs | 12 +- .../Listener/ResponseBodyTests.cs | 20 ++- .../Core/IISHttpContext.FeatureCollection.cs | 2 +- src/Servers/IIS/IIS/src/IISServerOptions.cs | 4 +- .../IIS.Tests/ConnectionIdFeatureTests.cs | 41 ++---- .../IIS.Tests/HttpBodyControlFeatureTests.cs | 37 ++++- .../Kestrel/Core/src/KestrelServerOptions.cs | 4 +- .../Core/test/KestrelServerOptionsTests.cs | 6 +- .../InMemory.FunctionalTests/RequestTests.cs | 129 +++++++++++++----- .../InMemory.FunctionalTests/ResponseTests.cs | 78 ++++++++--- .../InMemory.FunctionalTests/UpgradeTests.cs | 2 + 20 files changed, 292 insertions(+), 137 deletions(-) diff --git a/src/Hosting/TestHost/src/AsyncStreamWrapper.cs b/src/Hosting/TestHost/src/AsyncStreamWrapper.cs index 32f1548d350a..4766f8fcf12c 100644 --- a/src/Hosting/TestHost/src/AsyncStreamWrapper.cs +++ b/src/Hosting/TestHost/src/AsyncStreamWrapper.cs @@ -21,13 +21,17 @@ internal AsyncStreamWrapper(Stream inner, Func allowSynchronousIO) public override bool CanRead => _inner.CanRead; - public override bool CanSeek => _inner.CanSeek; + public override bool CanSeek => false; public override bool CanWrite => _inner.CanWrite; - public override long Length => _inner.Length; + public override long Length => throw new NotSupportedException("The stream is not seekable."); - public override long Position { get => _inner.Position; set => _inner.Position = value; } + public override long Position + { + get => throw new NotSupportedException("The stream is not seekable."); + set => throw new NotSupportedException("The stream is not seekable."); + } public override void Flush() { @@ -72,12 +76,12 @@ public override int EndRead(IAsyncResult asyncResult) public override long Seek(long offset, SeekOrigin origin) { - return _inner.Seek(offset, origin); + throw new NotSupportedException("The stream is not seekable."); } public override void SetLength(long value) { - _inner.SetLength(value); + throw new NotSupportedException("The stream is not seekable."); } public override void Write(byte[] buffer, int offset, int count) 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/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs index c0a9cdab2d27..48e7a0d15b22 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlDataContractSerializerOutputFormatter.cs @@ -254,6 +254,13 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var dataContractSerializer = GetCachedSerializer(wrappingType); + // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 + var syncIOFeature = context.HttpContext.Features.Get(); + if (syncIOFeature != null) + { + syncIOFeature.AllowSynchronousIO = true; + } + using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) { using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) diff --git a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs index 9960eedbd68c..da1812e6e954 100644 --- a/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs +++ b/src/Mvc/Mvc.Formatters.Xml/src/XmlSerializerOutputFormatter.cs @@ -230,6 +230,13 @@ public override async Task WriteResponseBodyAsync(OutputFormatterWriteContext co var xmlSerializer = GetCachedSerializer(wrappingType); + // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 + var syncIOFeature = context.HttpContext.Features.Get(); + if (syncIOFeature != null) + { + syncIOFeature.AllowSynchronousIO = true; + } + using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding)) { using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings)) diff --git a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs index 6a4fd56ba434..0d839e7b6840 100644 --- a/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs +++ b/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs @@ -105,6 +105,13 @@ public virtual async Task ExecuteAsync(ActionContext context, ViewComponentResul response.StatusCode = result.StatusCode.Value; } + // Opt into sync IO support until we can work out an alternative https://github.com/aspnet/AspNetCore/issues/6397 + var syncIOFeature = context.HttpContext.Features.Get(); + if (syncIOFeature != null) + { + syncIOFeature.AllowSynchronousIO = true; + } + using (var writer = new HttpResponseStreamWriter(response.Body, resolvedContentTypeEncoding)) { var viewContext = new ViewContext( diff --git a/src/Mvc/test/WebSites/BasicWebSite/StartupRequestLimitSize.cs b/src/Mvc/test/WebSites/BasicWebSite/StartupRequestLimitSize.cs index 286169d54d9a..3e0e9aa16ee7 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/StartupRequestLimitSize.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/StartupRequestLimitSize.cs @@ -3,6 +3,8 @@ using System; using System.IO; +using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; @@ -48,6 +50,7 @@ private class RequestBodySizeCheckingStream : Stream { private readonly Stream _innerStream; private readonly IHttpMaxRequestBodySizeFeature _maxRequestBodySizeFeature; + private long _totalRead; public RequestBodySizeCheckingStream( Stream innerStream, @@ -78,12 +81,39 @@ public override void Flush() public override int Read(byte[] buffer, int offset, int count) { if (_maxRequestBodySizeFeature.MaxRequestBodySize != null - && _innerStream.Length > _maxRequestBodySizeFeature.MaxRequestBodySize) + && _innerStream.CanSeek && _innerStream.Length > _maxRequestBodySizeFeature.MaxRequestBodySize) { throw new InvalidOperationException("Request content size is greater than the limit size"); } - return _innerStream.Read(buffer, offset, count); + var read = _innerStream.Read(buffer, offset, count); + _totalRead += read; + + if (_maxRequestBodySizeFeature.MaxRequestBodySize != null + && _totalRead > _maxRequestBodySizeFeature.MaxRequestBodySize) + { + throw new InvalidOperationException("Request content size is greater than the limit size"); + } + return read; + } + + public override async Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + if (_maxRequestBodySizeFeature.MaxRequestBodySize != null + && _innerStream.CanSeek && _innerStream.Length > _maxRequestBodySizeFeature.MaxRequestBodySize) + { + throw new InvalidOperationException("Request content size is greater than the limit size"); + } + + var read = await _innerStream.ReadAsync(buffer, offset, count, cancellationToken); + _totalRead += read; + + if (_maxRequestBodySizeFeature.MaxRequestBodySize != null + && _totalRead > _maxRequestBodySizeFeature.MaxRequestBodySize) + { + throw new InvalidOperationException("Request content size is greater than the limit size"); + } + return read; } public override long Seek(long offset, SeekOrigin origin) diff --git a/src/Mvc/test/WebSites/FormatterWebSite/StringInputFormatter.cs b/src/Mvc/test/WebSites/FormatterWebSite/StringInputFormatter.cs index dc80e3dc6237..746b1ea569a8 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/StringInputFormatter.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/StringInputFormatter.cs @@ -20,13 +20,13 @@ public StringInputFormatter() SupportedEncodings.Add(Encoding.Unicode); } - public override Task ReadRequestBodyAsync(InputFormatterContext context, Encoding effectiveEncoding) + public override async Task ReadRequestBodyAsync(InputFormatterContext context, Encoding effectiveEncoding) { var request = context.HttpContext.Request; using (var reader = new StreamReader(request.Body, effectiveEncoding)) { - var stringContent = reader.ReadToEnd(); - return InputFormatterResult.SuccessAsync(stringContent); + var stringContent = await reader.ReadToEndAsync(); + return await InputFormatterResult.SuccessAsync(stringContent); } } } diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index 4500e244b041..82453398a8dc 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/HttpsTests.cs b/src/Servers/HttpSys/test/FunctionalTests/HttpsTests.cs index a010203f9b22..d3e4fa54ce71 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/HttpsTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/HttpsTests.cs @@ -51,14 +51,13 @@ public async Task Https_SendHelloWorld_Success() [ConditionalFact] public async Task Https_EchoHelloWorld_Success() { - using (Utilities.CreateDynamicHttpsServer(out var address, httpContext => + using (Utilities.CreateDynamicHttpsServer(out var address, async httpContext => { - string input = new StreamReader(httpContext.Request.Body).ReadToEnd(); + var input = await new StreamReader(httpContext.Request.Body).ReadToEndAsync(); Assert.Equal("Hello World", input); - byte[] body = Encoding.UTF8.GetBytes("Hello World"); + var body = Encoding.UTF8.GetBytes("Hello World"); httpContext.Response.ContentLength = body.Length; - httpContext.Response.Body.Write(body, 0, body.Length); - return Task.FromResult(0); + await httpContext.Response.Body.WriteAsync(body, 0, body.Length); })) { string response = await SendRequestAsync(address, "Hello World"); 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 0fb509bfea62..c482c63b4f0e 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -1077,45 +1077,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)); @@ -1132,21 +1116,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(); } @@ -1197,6 +1216,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 b38e97f465a4..dd7ce8dc1657 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3042,34 +3042,47 @@ 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); + } + } - await context.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello2"), 0, 6)); - } + [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; + context.Response.Body.Write(Encoding.ASCII.GetBytes("Hello1"), 0, 6); + return Task.CompletedTask; }, new TestServiceContext(LoggerFactory))) { using (var connection = server.CreateConnection()) @@ -3081,14 +3094,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(); } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs index 03e5dab1b5b0..f36d4e84a19d 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/UpgradeTests.cs @@ -33,6 +33,7 @@ public async Task ResponseThrowsAfterUpgrade() { await writer.WriteLineAsync("New protocol data"); await writer.FlushAsync(); + await writer.DisposeAsync(); } upgrade.TrySetResult(true); @@ -79,6 +80,7 @@ public async Task RequestBodyAlwaysEmptyAfterUpgrade() Assert.Equal(send, line); await writer.WriteLineAsync(recv); await writer.FlushAsync(); + await writer.DisposeAsync(); } upgrade.TrySetResult(true);