Address PR feedback and add tests

aspnet · Jun 28, 2017 · 34da3ea · 34da3ea
1 parent 5b0741f
commit 34da3ea
Show file tree

Hide file tree

Showing 5 changed files with 221 additions and 1 deletion.
diff --git a/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerOptions.cs b/src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServerOptions.cs
@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Net;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Core
@@ -36,7 +37,7 @@ public class KestrelServerOptions
         public SchedulingMode ApplicationSchedulingMode { get; set; } = SchedulingMode.Default;
 
         /// <summary>
-        /// Gets or sets a value that controls whether synchronous IO is allowed for the <see cref="Http.Features.IHttpRequestFeature.Body"/> and <see cref="Http.Features.IHttpResponseFeature.Body"/> 
+        /// Gets or sets a value that controls whether synchronous IO is allowed for the <see cref="HttpContext.Request"/> and <see cref="HttpContext.Response"/> 
         /// </summary>
         public bool AllowSynchronousIO { get; set; }
 

diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameRequestStreamTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameRequestStreamTests.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.IO;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
@@ -110,6 +111,27 @@ public async Task FlushAsyncThrows()
             await Assert.ThrowsAsync<NotSupportedException>(() => stream.FlushAsync());
         }
 
+        [Fact]
+        public async Task SynchronousReadsThrowByDefault()
+        {
+            var allowSynchronousIO = false;
+            var mockBodyControl = new Mock<IHttpBodyControlFeature>();
+            mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(() => allowSynchronousIO);
+            var mockMessageBody = new Mock<MessageBody>((Frame)null);
+            mockMessageBody.Setup(m => m.ReadAsync(It.IsAny<ArraySegment<byte>>(), CancellationToken.None)).ReturnsAsync(0);
+
+            var stream = new FrameRequestStream(mockBodyControl.Object);
+            stream.StartAcceptingReads(mockMessageBody.Object);
+
+            Assert.Equal(0, await stream.ReadAsync(new byte[1], 0, 1));
+
+            var ioEx = Assert.Throws<InvalidOperationException>(() => stream.Read(new byte[1], 0, 1));
+            Assert.Equal("Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead.", ioEx.Message);
+
+            allowSynchronousIO = true;
+            Assert.Equal(0, stream.Read(new byte[1], 0, 1));
+        }
+
         [Fact]
         public void AbortCausesReadToCancel()
         {

diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameResponseStreamTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/FrameResponseStreamTests.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.IO;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
@@ -91,5 +92,37 @@ public void PositionThrows()
             Assert.Throws<NotSupportedException>(() => stream.Position);
             Assert.Throws<NotSupportedException>(() => stream.Position = 0);
         }
+
+        [Fact]
+        public void StopAcceptingWritesCausesWriteToThrowObjectDisposedException()
+        {
+            var stream = new FrameResponseStream(Mock.Of<IHttpBodyControlFeature>(), Mock.Of<IFrameControl>());
+            stream.StartAcceptingWrites();
+            stream.StopAcceptingWrites();
+            Assert.Throws<ObjectDisposedException>(() => { stream.WriteAsync(new byte[1], 0, 1); });
+        }
+
+        [Fact]
+        public async Task SynchronousWritesThrowByDefault()
+        {
+            var allowSynchronousIO = false;
+            var mockBodyControl = new Mock<IHttpBodyControlFeature>();
+            mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(() => allowSynchronousIO);
+            var mockFrameControl = new Mock<IFrameControl>();
+            mockFrameControl.Setup(m => m.WriteAsync(It.IsAny<ArraySegment<byte>>(), CancellationToken.None)).Returns(Task.CompletedTask);
+
+            var stream = new FrameResponseStream(mockBodyControl.Object, mockFrameControl.Object);
+            stream.StartAcceptingWrites();
+
+            // WriteAsync doesn't throw.
+            await stream.WriteAsync(new byte[1], 0, 1);
+
+            var ioEx = Assert.Throws<InvalidOperationException>(() => stream.Write(new byte[1], 0, 1));
+            Assert.Equal("Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.", ioEx.Message);
+
+            allowSynchronousIO = true;
+            // If IHttpBodyControlFeature.AllowSynchronousIO is true, Write no longer throws.
+            stream.Write(new byte[1], 0, 1);
+        }
     }
 }
diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/RequestTests.cs
@@ -1479,6 +1479,95 @@ await connection.ReceiveForcedEnd(
             }
         }
 
+        [Fact]
+        public async Task SynchronousReadsThrowByDefault()
+        {
+            using (var server = new TestServer(context =>
+            {
+                var ioEx = Assert.Throws<InvalidOperationException>(() => context.Request.Body.Read(new byte[1], 0, 1));
+                Assert.Equal("Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead.", ioEx.Message);
+
+                var ioEx2 = Assert.Throws<InvalidOperationException>(() => context.Request.Body.CopyTo(Stream.Null));
+                Assert.Equal("Synchronous operations are disallowed. Call ReadAsync or set AllowSynchronousIO to true instead.", ioEx2.Message);
+
+                var bodyControlFeature = context.Features.Get<IHttpBodyControlFeature>();
+                Assert.False(bodyControlFeature.AllowSynchronousIO);
+
+                bodyControlFeature.AllowSynchronousIO = true;
+
+                // Read now no longer throws.
+                var buffer = new byte[5];
+                var offset = 0;
+                while (offset < 5)
+                {
+                    offset += context.Request.Body.Read(buffer, offset, 5 - offset);
+                }
+
+                Assert.Equal(0, context.Request.Body.Read(new byte[1], 0, 1));
+                Assert.Equal("Hello", Encoding.ASCII.GetString(buffer));
+
+                return Task.CompletedTask;
+            }))
+            {
+                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",
+                        "",
+                        "");
+                }
+            }
+        }
+
+        [Fact]
+        public async Task SynchronousReadsCanBeEnabledGlobally()
+        {
+            var testContext = new TestServiceContext
+            {
+                ServerOptions = { AllowSynchronousIO = true }
+            };
+
+            using (var server = new TestServer(context =>
+            {
+                var buffer = new byte[5];
+                var offset = 0;
+                while (offset < 5)
+                {
+                    offset += context.Request.Body.Read(buffer, offset, 5 - offset);
+                }
+
+                Assert.Equal(0, context.Request.Body.Read(new byte[1], 0, 1));
+                Assert.Equal("Hello", Encoding.ASCII.GetString(buffer));
+
+                return Task.CompletedTask;
+            }, 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",
+                        "",
+                        "");
+                }
+            }
+        }
+
         private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress)
         {
             var builder = new WebHostBuilder()

diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs
@@ -2330,6 +2330,81 @@ await connection.ReceiveEnd(
             Assert.Equal(2, callOrder.Pop());
         }
 
+
+        [Fact]
+        public async Task SynchronousWritesThrowByDefault()
+        {
+            using (var server = new TestServer(context =>
+            {
+                var helloBuffer = Encoding.ASCII.GetBytes("Hello");
+                context.Response.ContentLength = 5;
+
+                var ioEx = Assert.Throws<InvalidOperationException>(() => context.Response.Body.Write(helloBuffer, 0, 5));
+                Assert.Equal("Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.", ioEx.Message);
+
+                var bodyControlFeature = context.Features.Get<IHttpBodyControlFeature>();
+                Assert.False(bodyControlFeature.AllowSynchronousIO);
+
+                bodyControlFeature.AllowSynchronousIO = true;
+
+                // Write now now longer throws.
+                context.Response.Body.Write(helloBuffer, 0, 5);
+
+                return Task.CompletedTask;
+            }))
+            {
+                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: 5",
+                        "",
+                        "Hello");
+                }
+            }
+        }
+
+        [Fact]
+        public async Task SynchronousWritesCanBeEnabledGlobally()
+        {
+            var testContext = new TestServiceContext
+            {
+                ServerOptions = { AllowSynchronousIO = true }
+            };
+
+            using (var server = new TestServer(context =>
+            {
+                var helloBuffer = Encoding.ASCII.GetBytes("Hello");
+                context.Response.ContentLength = 5;
+
+                context.Response.Body.Write(helloBuffer, 0, 5);
+
+                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: 5",
+                        "",
+                        "Hello");
+                }
+            }
+        }
+
         public static TheoryData<string, StringValues, string> NullHeaderData
         {
             get