Skip to content

Commit

Permalink
Disable AllowSynchronousIO by default in all servers #4774
Browse files Browse the repository at this point in the history
  • Loading branch information
Tratcher committed Jan 15, 2019
1 parent 1f892d7 commit 6db3005
Show file tree
Hide file tree
Showing 12 changed files with 221 additions and 121 deletions.
4 changes: 2 additions & 2 deletions src/Hosting/TestHost/src/TestServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ public IWebHost Host
/// Gets or sets a value that controls whether synchronous IO is allowed for the <see cref="HttpContext.Request"/> and <see cref="HttpContext.Response"/>
/// </summary>
/// <remarks>
/// Defaults to true.
/// Defaults to false.
/// </remarks>
public bool AllowSynchronousIO { get; set; } = true;
public bool AllowSynchronousIO { get; set; } = false;

private IHttpApplication<Context> Application
{
Expand Down
6 changes: 3 additions & 3 deletions src/Servers/HttpSys/src/HttpSysOptions.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -136,9 +136,9 @@ public long? MaxRequestBodySize

/// <summary>
/// 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`.
/// </summary>
public bool AllowSynchronousIO { get; set; } = true;
public bool AllowSynchronousIO { get; set; } = false;

/// <summary>
/// Gets or sets a value that controls how http.sys reacts when rejecting requests due to throttling conditions - like when the request
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<string> 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<InvalidOperationException>(() => 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<InvalidOperationException>(() => context.Request.Body.Read(input, 0, input.Length));

string response = await responseTask;
Assert.Equal("Hello World", response);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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<InvalidOperationException>(() => context.Response.Body.Flush());
Assert.Throws<InvalidOperationException>(() => context.Response.Body.Write(new byte[10], 0, 10));
Assert.Throws<InvalidOperationException>(() => 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;
Expand All @@ -47,7 +45,7 @@ public async Task ResponseBody_SyncWriteEnabledByDefault_ThrowsWhenDisabled()
IEnumerable<string> 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());
}
}

Expand Down Expand Up @@ -477,4 +475,4 @@ public async Task ResponseBody_ClientDisconnectsBeforeSecondWriteAsync_WriteComp
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
4 changes: 2 additions & 2 deletions src/Servers/IIS/IIS/src/IISServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ public class IISServerOptions
/// Gets or sets a value that controls whether synchronous IO is allowed for the <see cref="HttpContext.Request"/> and <see cref="HttpContext.Response"/>
/// </summary>
/// <remarks>
/// Defaults to true.
/// Defaults to false.
/// </remarks>
public bool AllowSynchronousIO { get; set; } = true;
public bool AllowSynchronousIO { get; set; } = false;

/// <summary>
/// If true the server should set HttpContext.User. If false the server will only provide an
Expand Down
41 changes: 9 additions & 32 deletions src/Servers/IIS/IIS/test/IIS.Tests/ConnectionIdFeatureTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<IHttpBodyControlFeature>();
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<IHttpConnectionFeature>();
connectionId = connectionIdFeature.ConnectionId;
return Task.CompletedTask;
}, LoggerFactory))
{
await testServer.HttpClient.GetStringAsync("/");
}

Assert.IsType<InvalidOperationException>(readException);
Assert.IsType<InvalidOperationException>(writeException);
Assert.NotNull(connectionId);
}
}
}
37 changes: 30 additions & 7 deletions src/Servers/IIS/IIS/test/IIS.Tests/HttpBodyControlFeatureTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<IHttpConnectionFeature>();
connectionId = connectionIdFeature.ConnectionId;
Exception writeException = null;
Exception readException = null;
using (var testServer = await TestServer.Create(
ctx => {
var bodyControl = ctx.Features.Get<IHttpBodyControlFeature>();
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<InvalidOperationException>(readException);
Assert.IsType<InvalidOperationException>(writeException);
}
}
}
4 changes: 2 additions & 2 deletions src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ public class KestrelServerOptions
/// Gets or sets a value that controls whether synchronous IO is allowed for the <see cref="HttpContext.Request"/> and <see cref="HttpContext.Response"/>
/// </summary>
/// <remarks>
/// Defaults to true.
/// Defaults to false.
/// </remarks>
public bool AllowSynchronousIO { get; set; } = true;
public bool AllowSynchronousIO { get; set; } = false;

/// <summary>
/// Enables the Listen options callback to resolve and use services registered by the application during startup.
Expand Down
6 changes: 3 additions & 3 deletions src/Servers/Kestrel/Core/test/KestrelServerOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -65,4 +65,4 @@ public void ConfigureEndpointDefaultsAppliesToNewEndpoints()
Assert.False(options.ListenOptions[3].NoDelay);
}
}
}
}
Loading

0 comments on commit 6db3005

Please sign in to comment.