Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Always flush headers on first response write (#1202). #1204

Merged
merged 2 commits into from
Nov 9, 2016
Merged

Always flush headers on first response write (#1202). #1204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0e75dc
Always flush headers on first response write (#1202).
halter73 Nov 7, 2016
3c7e7d1
Make ConnectionFilterTests more reliable
halter73 Nov 9, 2016
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
18 changes: 17 additions & 1 deletion src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,9 @@ public async Task FlushAsync(CancellationToken cancellationToken)

public void Write(ArraySegment<byte> data)
{
// For the first write, ensure headers are flushed if Write(Chunked)isn't called.
var firstWrite = !HasResponseStarted;

VerifyAndUpdateWrite(data.Count);
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();

Expand All @@ -529,6 +532,10 @@ public void Write(ArraySegment<byte> data)
{
if (data.Count == 0)
{
if (firstWrite)
{
Flush();
}
return;
}
WriteChunked(data);
Expand All @@ -541,6 +548,11 @@ public void Write(ArraySegment<byte> data)
else
{
HandleNonBodyResponseWrite();

if (firstWrite)
{
Flush();
}
}
}

Expand Down Expand Up @@ -581,14 +593,18 @@ public async Task WriteAsyncAwaited(ArraySegment<byte> 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
Expand All @@ -599,7 +615,7 @@ public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken c
else
{
HandleNonBodyResponseWrite();
return;
await FlushAsync(cancellationToken);
}
}

Expand Down
80 changes: 75 additions & 5 deletions test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand All @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add reasonable timeouts to the semaphore Waits?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calls to WaitAsync() are in the web app on the test. If the web app hangs, the test will not hang. Instead the test will fail when it times out waiting for a response. The response timeout here is 1 minute like for most of our tests.

}, 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

}
}
}

[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");
}
}
}
Expand Down
42 changes: 42 additions & 0 deletions test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedResponseTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 13 additions & 5 deletions test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<IOException>(async () => await connection.SendEnd(
"POST / HTTP/1.0",
"Content-Length: 12",
"",
"Hello World?"));
await Assert.ThrowsAsync<IOException>(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);
}
});
}
}
}
Expand Down