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

Commit

Permalink
Call ProduceEnd() before consuming request body if response has alrea…
Browse files Browse the repository at this point in the history 
…dy started (#1537, backported to 1.1.x).
  • Loading branch information
Cesar Blum Silveira authored and Nate McMaster committed Apr 13, 2017
1 parent a1ce476 commit 4457950
Show file tree
Hide file tree
Showing 3 changed files with 194 additions and 21 deletions.
5 changes: 4 additions & 1 deletion global.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{
"projects": ["src"],
"projects": [
"src",
"test"
],
"sdk": {
"version": "1.0.0-preview2-1-003177"
}
Expand Down
22 changes: 18 additions & 4 deletions src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,30 @@ public override async Task RequestProcessingAsync()
{
ResumeStreams();

if (HasResponseStarted)
{
// If the response has already started, call ProduceEnd() before
// consuming the rest of the request body to prevent
// delaying clients waiting for the chunk terminator:
//
// https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663
//
// ProduceEnd() must be called before _application.DisposeContext(), to ensure
// HttpContext.Response.StatusCode is correctly set when
// IHttpContextFactory.Dispose(HttpContext) is called.
await ProduceEnd();
}

if (_keepAlive)
{
// Finish reading the request body in case the app did not.
await messageBody.Consume();
}

// ProduceEnd() must be called before _application.DisposeContext(), to ensure
// HttpContext.Response.StatusCode is correctly set when
// IHttpContextFactory.Dispose(HttpContext) is called.
await ProduceEnd();
if (!HasResponseStarted)
{
await ProduceEnd();
}
}
else if (!HasResponseStarted)
{
Expand Down
188 changes: 172 additions & 16 deletions test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public async Task SingleErrorResponseSentWhenAppSwallowsBadRequestException()
{
readException = await Assert.ThrowsAsync<BadHttpRequestException>(
async () => await httpContext.Request.Body.ReadAsync(new byte[1], 0, 1));
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -376,7 +376,7 @@ public async Task TransferEncodingChunkedSetOnUnknownLengthHttp11Response()
{
await httpContext.Response.WriteAsync("hello, ");
await httpContext.Response.WriteAsync("world");
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -410,7 +410,7 @@ public async Task TransferEncodingChunkedNotSetOnNonBodyResponse(int statusCode)
{
httpContext.Response.StatusCode = statusCode;
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand All @@ -433,7 +433,7 @@ public async Task TransferEncodingNotSetOnHeadResponse()
using (var server = new TestServer(httpContext =>
{
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -805,7 +805,7 @@ public async Task HeadResponseCanContainContentLengthHeader()
{
httpContext.Response.ContentLength = 42;
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -833,7 +833,7 @@ public async Task HeadResponseBodyNotWrittenWithAsyncWrite()
httpContext.Response.ContentLength = 12;
await httpContext.Response.WriteAsync("hello, world");
await flushed.WaitAsync();
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -864,7 +864,7 @@ public async Task HeadResponseBodyNotWrittenWithSyncWrite()
httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello, world"), 0, 12);
flushed.Wait();
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -895,7 +895,7 @@ public async Task ZeroLengthWritesFlushHeaders()
await httpContext.Response.WriteAsync("");
flushed.Wait();
await httpContext.Response.WriteAsync("hello, world");
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -937,7 +937,7 @@ public async Task AppCanWriteOwnBadRequestResponse()
await httpContext.Response.WriteAsync(ex.Message);
responseWrittenTcs.SetResult(null);
}
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -968,7 +968,7 @@ public async Task ConnectionClosedWhenChunkedIsNotFinalTransferCoding(string res
{
httpContext.Response.Headers["Transfer-Encoding"] = responseTransferEncoding;
await httpContext.Response.WriteAsync("hello, world");
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1015,7 +1015,7 @@ public async Task ConnectionClosedWhenChunkedIsNotFinalTransferCodingEvenIfConne
httpContext.Response.Headers["Connection"] = "keep-alive";
httpContext.Response.Headers["Transfer-Encoding"] = responseTransferEncoding;
await httpContext.Response.WriteAsync("hello, world");
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1061,7 +1061,7 @@ public async Task ConnectionKeptAliveWhenChunkedIsFinalTransferCoding(string res

// App would have to chunk manually, but here we don't care
await httpContext.Response.WriteAsync("hello, world");
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1109,7 +1109,7 @@ public async Task FirstWriteVerifiedAfterOnStarting()
// If OnStarting is not run before verifying writes, an error response will be sent.
httpContext.Response.Body.Write(response, 0, response.Length);
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1150,7 +1150,7 @@ public async Task SubsequentWriteVerifiedAfterOnStarting()
httpContext.Response.Body.Write(response, 0, response.Length / 2);
httpContext.Response.Body.Write(response, response.Length / 2, response.Length - response.Length / 2);
return TaskCache.CompletedTask;
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1191,7 +1191,7 @@ public async Task FirstWriteAsyncVerifiedAfterOnStarting()

// If OnStarting is not run before verifying writes, an error response will be sent.
return httpContext.Response.Body.WriteAsync(response, 0, response.Length);
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand Down Expand Up @@ -1231,7 +1231,7 @@ public async Task SubsequentWriteAsyncVerifiedAfterOnStarting()
// If OnStarting is not run before verifying writes, an error response will be sent.
await httpContext.Response.Body.WriteAsync(response, 0, response.Length / 2);
await httpContext.Response.Body.WriteAsync(response, response.Length / 2, response.Length - response.Length / 2);
}, new TestServiceContext()))
}))
{
using (var connection = server.CreateConnection())
{
Expand All @@ -1255,6 +1255,162 @@ await connection.Receive(
}
}

[Fact]
public async Task WhenResponseAlreadyStartedResponseEndedBeforeConsumingRequestBody()
{
using (var server = new TestServer(async httpContext =>
{
await httpContext.Response.WriteAsync("hello, world");
}))
{
using (var connection = server.CreateConnection())
{
await connection.Send(
"POST / HTTP/1.1",
"Content-Length: 1",
"",
"");

await connection.Receive(
"HTTP/1.1 200 OK",
$"Date: {server.Context.DateHeaderValue}",
$"Transfer-Encoding: chunked",
"",
"c",
"hello, world",
"");

// If the expected behavior is regressed, this will hang because the
// server will try to consume the request body before flushing the chunked
// terminator.
await connection.Receive(
"0",
"",
"");
}
}
}

[Fact]
public async Task WhenResponseNotStartedResponseEndedAfterConsumingRequestBody()
{
using (var server = new TestServer(httpContext => TaskCache.CompletedTask))
{
using (var connection = server.CreateConnection())
{
await connection.Send(
"POST / HTTP/1.1",
"Transfer-Encoding: chunked",
"",
"gg");

// If the expected behavior is regressed, this will receive
// a success response because the server flushed the response
// before reading the malformed chunk header in the request.
await connection.ReceiveForcedEnd(
"HTTP/1.1 400 Bad Request",
"Connection: close",
$"Date: {server.Context.DateHeaderValue}",
"Content-Length: 0",
"",
"");
}
}
}

[Fact]
public async Task Sending100ContinueDoesNotStartResponse()
{
using (var server = new TestServer(httpContext =>
{
return httpContext.Request.Body.ReadAsync(new byte[1], 0, 1);
}))
{
using (var connection = server.CreateConnection())
{
await connection.Send(
"POST / HTTP/1.1",
"Transfer-Encoding: chunked",
"Expect: 100-continue",
"",
"");

await connection.Receive(
"HTTP/1.1 100 Continue",
"",
"");

// Let the app finish
await connection.Send(
"1",
"a",
"");

// This will be consumed by Frame when it attempts to
// consume the request body and will cause an error.
await connection.Send(
"gg");

// If 100 Continue sets Frame.HasResponseStarted to true,
// a success response will be produced before the server sees the
// bad chunk header above, making this test fail.
await connection.ReceiveForcedEnd(
"HTTP/1.1 400 Bad Request",
"Connection: close",
$"Date: {server.Context.DateHeaderValue}",
"Content-Length: 0",
"",
"");
}
}
}

[Fact]
public async Task Sending100ContinueAndResponseSendsChunkTerminatorBeforeConsumingRequestBody()
{
using (var server = new TestServer(async httpContext =>
{
await httpContext.Request.Body.ReadAsync(new byte[1], 0, 1);
await httpContext.Response.WriteAsync("hello, world");
}))
{
using (var connection = server.CreateConnection())
{
await connection.Send(
"POST / HTTP/1.1",
"Content-Length: 2",
"Expect: 100-continue",
"",
"");

await connection.Receive(
"HTTP/1.1 100 Continue",
"",
"");

await connection.Send(
"a");

await connection.Receive(
"HTTP/1.1 200 OK",
$"Date: {server.Context.DateHeaderValue}",
$"Transfer-Encoding: chunked",
"",
"c",
"hello, world",
"");

// If the expected behavior is regressed, this will hang because the
// server will try to consume the request body before flushing the chunked
// terminator.
await connection.Receive(
"0",
"",
"");
}
}
}

public static TheoryData<string, StringValues, string> NullHeaderData
{
get
Expand Down

0 comments on commit 4457950

Please sign in to comment.