From c0e75dcbf86be380526c7028e8a3607b3f5ddf84 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 7 Nov 2016 15:55:46 -0800 Subject: [PATCH 1/2] Always flush headers on first response write (#1202). --- .../Internal/Http/Frame.cs | 18 ++++- .../ResponseTests.cs | 80 +++++++++++++++++-- .../ChunkedResponseTests.cs | 42 ++++++++++ 3 files changed, 134 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index 725e284bc..307ce7bc4 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -520,6 +520,9 @@ public async Task FlushAsync(CancellationToken cancellationToken) public void Write(ArraySegment data) { + // For the first write, ensure headers are flushed if Write(Chunked)isn't called. + var firstWrite = !HasResponseStarted; + VerifyAndUpdateWrite(data.Count); ProduceStartAndFireOnStarting().GetAwaiter().GetResult(); @@ -529,6 +532,10 @@ public void Write(ArraySegment data) { if (data.Count == 0) { + if (firstWrite) + { + Flush(); + } return; } WriteChunked(data); @@ -541,6 +548,11 @@ public void Write(ArraySegment data) else { HandleNonBodyResponseWrite(); + + if (firstWrite) + { + Flush(); + } } } @@ -581,14 +593,18 @@ public async Task WriteAsyncAwaited(ArraySegment data, CancellationToken c await ProduceStartAndFireOnStarting(); + // WriteAsyncAwaited is only called for the first write to the body. + // Ensure headers are flushed if Write(Chunked)Async isn't called. if (_canHaveBody) { if (_autoChunk) { if (data.Count == 0) { + await FlushAsync(cancellationToken); return; } + await WriteChunkedAsync(data, cancellationToken); } else @@ -599,7 +615,7 @@ public async Task WriteAsyncAwaited(ArraySegment data, CancellationToken c else { HandleNonBodyResponseWrite(); - return; + await FlushAsync(cancellationToken); } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs index 5b4b5b73f..dbf6f5bdd 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Net.Sockets; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; @@ -767,11 +768,11 @@ public async Task HeadResponseCanContainContentLengthHeader() { using (var connection = server.CreateConnection()) { - await connection.SendEnd( + await connection.Send( "HEAD / HTTP/1.1", "", ""); - await connection.ReceiveEnd( + await connection.Receive( "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", "Content-Length: 42", @@ -782,26 +783,95 @@ await connection.ReceiveEnd( } [Fact] - public async Task HeadResponseCanContainContentLengthHeaderButBodyNotWritten() + public async Task HeadResponseBodyNotWrittenWithAsyncWrite() { + var flushed = new SemaphoreSlim(0, 1); + using (var server = new TestServer(async httpContext => { httpContext.Response.ContentLength = 12; await httpContext.Response.WriteAsync("hello, world"); + await flushed.WaitAsync(); }, new TestServiceContext())) { using (var connection = server.CreateConnection()) { - await connection.SendEnd( + await connection.Send( "HEAD / HTTP/1.1", "", ""); - await connection.ReceiveEnd( + await connection.Receive( "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", "Content-Length: 12", "", ""); + + flushed.Release(); + } + } + } + + [Fact] + public async Task HeadResponseBodyNotWrittenWithSyncWrite() + { + var flushed = new SemaphoreSlim(0, 1); + + using (var server = new TestServer(httpContext => + { + httpContext.Response.ContentLength = 12; + httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello, world"), 0, 12); + flushed.Wait(); + return TaskCache.CompletedTask; + }, new TestServiceContext())) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "HEAD / HTTP/1.1", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 12", + "", + ""); + + flushed.Release(); + } + } + } + + [Fact] + public async Task ZeroLengthWritesFlushHeaders() + { + var flushed = new SemaphoreSlim(0, 1); + + using (var server = new TestServer(async httpContext => + { + httpContext.Response.ContentLength = 12; + await httpContext.Response.WriteAsync(""); + flushed.Wait(); + await httpContext.Response.WriteAsync("hello, world"); + }, new TestServiceContext())) + { + using (var connection = server.CreateConnection()) + { + await connection.SendEnd( + "GET / HTTP/1.1", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 12", + "", + ""); + + flushed.Release(); + + await connection.ReceiveEnd("hello, world"); } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedResponseTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedResponseTests.cs index 469f17051..7ce2297b0 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedResponseTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedResponseTests.cs @@ -192,6 +192,48 @@ await connection.ReceiveEnd( } } + [Theory] + [MemberData(nameof(ConnectionFilterData))] + public async Task ZeroLengthWritesFlushHeaders(TestServiceContext testContext) + { + var flushed = new SemaphoreSlim(0, 1); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + await response.WriteAsync(""); + + await flushed.WaitAsync(); + + await response.WriteAsync("Hello World!"); + }, testContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.SendEnd( + "GET / HTTP/1.1", + "", + ""); + + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {testContext.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + ""); + + flushed.Release(); + + await connection.ReceiveEnd( + "c", + "Hello World!", + "0", + "", + ""); + } + } + } + [Theory] [MemberData(nameof(ConnectionFilterData))] public async Task EmptyResponseBodyHandledCorrectlyWithZeroLengthWrite(TestServiceContext testContext) From 3c7e7d1f6cfb3d3d19cc0e956f621673c68f899e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 8 Nov 2016 23:38:58 -0800 Subject: [PATCH 2/2] Make ConnectionFilterTests more reliable --- .../ConnectionFilterTests.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs index ea7664656..6ef4b9de2 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs @@ -89,11 +89,19 @@ public async Task ThrowingSynchronousConnectionFilterDoesNotCrashServer() using (var connection = server.CreateConnection()) { // Will throw because the exception in the connection filter will close the connection. - await Assert.ThrowsAsync(async () => await connection.SendEnd( - "POST / HTTP/1.0", - "Content-Length: 12", - "", - "Hello World?")); + await Assert.ThrowsAsync(async () => + { + await connection.Send( + "POST / HTTP/1.0", + "Content-Length: 1000", + "\r\n"); + + for (var i = 0; i < 1000; i++) + { + await connection.Send("a"); + await Task.Delay(5); + } + }); } } }