Skip to content

Disable AllowSynchronousIO by default in all servers #5120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -50,14 +50,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