Skip to content

Commit

Permalink
Disable AllowSynchronousIO by default in all servers #4774 (#5120)
Browse files Browse the repository at this point in the history
* Disable AllowSynchronousIO by default in all servers
  • Loading branch information
Tratcher authored Feb 16, 2019
1 parent 7daa0e0 commit 93a24b0
Show file tree
Hide file tree
Showing 20 changed files with 292 additions and 137 deletions.
14 changes: 9 additions & 5 deletions src/Hosting/TestHost/src/AsyncStreamWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ internal AsyncStreamWrapper(Stream inner, Func<bool> 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()
{
Expand Down Expand Up @@ -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)
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -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<Http.Features.IHttpBodyControlFeature>();
if (syncIOFeature != null)
{
syncIOFeature.AllowSynchronousIO = true;
}

using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding))
{
using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Http.Features.IHttpBodyControlFeature>();
if (syncIOFeature != null)
{
syncIOFeature.AllowSynchronousIO = true;
}

using (var textWriter = context.WriterFactory(context.HttpContext.Response.Body, writerSettings.Encoding))
{
using (var xmlWriter = CreateXmlWriter(context, textWriter, writerSettings))
Expand Down
7 changes: 7 additions & 0 deletions src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Http.Features.IHttpBodyControlFeature>();
if (syncIOFeature != null)
{
syncIOFeature.AllowSynchronousIO = true;
}

using (var writer = new HttpResponseStreamWriter(response.Body, resolvedContentTypeEncoding))
{
var viewContext = new ViewContext(
Expand Down
34 changes: 32 additions & 2 deletions src/Mvc/test/WebSites/BasicWebSite/StartupRequestLimitSize.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,6 +50,7 @@ private class RequestBodySizeCheckingStream : Stream
{
private readonly Stream _innerStream;
private readonly IHttpMaxRequestBodySizeFeature _maxRequestBodySizeFeature;
private long _totalRead;

public RequestBodySizeCheckingStream(
Stream innerStream,
Expand Down Expand Up @@ -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<int> 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ public StringInputFormatter()
SupportedEncodings.Add(Encoding.Unicode);
}

public override Task<InputFormatterResult> ReadRequestBodyAsync(InputFormatterContext context, Encoding effectiveEncoding)
public override async Task<InputFormatterResult> 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);
}
}
}
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
9 changes: 4 additions & 5 deletions src/Servers/HttpSys/test/FunctionalTests/HttpsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
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);
}
}
}
Loading

0 comments on commit 93a24b0

Please sign in to comment.