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

Commit 4360f5d

Browse files
author
Cesar Blum Silveira
committed
Address PR feedback.
- Set _canHaveBody to true when StatusCode == 101 - Add test to check that upgraded connections are always closed when the app is done
1 parent 96857f4 commit 4360f5d

File tree

2 files changed

+51
-15
lines changed
  • src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http
  • test/Microsoft.AspNetCore.Server.KestrelTests

2 files changed

+51
-15
lines changed

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ public void Write(ArraySegment<byte> data)
514514
{
515515
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
516516

517-
if (_canHaveBody || StatusCode == 101)
517+
if (_canHaveBody)
518518
{
519519
if (_autoChunk)
520520
{
@@ -542,7 +542,7 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
542542
return WriteAsyncAwaited(data, cancellationToken);
543543
}
544544

545-
if (_canHaveBody || StatusCode == 101)
545+
if (_canHaveBody)
546546
{
547547
if (_autoChunk)
548548
{
@@ -568,7 +568,7 @@ public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken c
568568
{
569569
await ProduceStartAndFireOnStarting();
570570

571-
if (_canHaveBody || StatusCode == 101)
571+
if (_canHaveBody)
572572
{
573573
if (_autoChunk)
574574
{
@@ -771,11 +771,13 @@ private void CreateResponseHeader(
771771
// Set whether response can have body
772772
_canHaveBody = StatusCanHaveBody(StatusCode) && Method != "HEAD";
773773

774+
// Don't set the Content-Length or Transfer-Encoding headers
775+
// automatically for HEAD requests or 204, 205, 304 responses.
774776
if (_canHaveBody)
775777
{
776778
if (!responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
777779
{
778-
if (appCompleted)
780+
if (appCompleted && StatusCode != 101)
779781
{
780782
// Since the app has completed and we are only now generating
781783
// the headers we can safely set the Content-Length to 0.
@@ -790,7 +792,7 @@ private void CreateResponseHeader(
790792
//
791793
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
792794
// request indicates HTTP/1.1 (or later).
793-
if (_httpVersion == Http.HttpVersion.Http11)
795+
if (_httpVersion == Http.HttpVersion.Http11 && StatusCode != 101)
794796
{
795797
_autoChunk = true;
796798
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
@@ -804,8 +806,6 @@ private void CreateResponseHeader(
804806
}
805807
else
806808
{
807-
// Don't set the Content-Length or Transfer-Encoding headers
808-
// automatically for HEAD requests or 204, 205, 304 responses.
809809
if (responseHeaders.HasTransferEncoding)
810810
{
811811
RejectNonBodyTransferEncodingResponse(appCompleted);
@@ -1271,8 +1271,7 @@ public bool TakeMessageHeaders(SocketInput input, FrameRequestHeaders requestHea
12711271
public bool StatusCanHaveBody(int statusCode)
12721272
{
12731273
// List of status codes taken from Microsoft.Net.Http.Server.Response
1274-
return statusCode != 101 &&
1275-
statusCode != 204 &&
1274+
return statusCode != 204 &&
12761275
statusCode != 205 &&
12771276
statusCode != 304;
12781277
}

test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -548,9 +548,6 @@ await connection.SendEnd(
548548
"POST / HTTP/1.1",
549549
"Content-Length: 3",
550550
"",
551-
"101POST / HTTP/1.1",
552-
"Content-Length: 3",
553-
"",
554551
"204POST / HTTP/1.1",
555552
"Content-Length: 3",
556553
"",
@@ -562,9 +559,6 @@ await connection.SendEnd(
562559
"",
563560
"200");
564561
await connection.ReceiveEnd(
565-
"HTTP/1.1 101 Switching Protocols",
566-
$"Date: {testContext.DateHeaderValue}",
567-
"",
568562
"HTTP/1.1 204 No Content",
569563
$"Date: {testContext.DateHeaderValue}",
570564
"",
@@ -583,6 +577,49 @@ await connection.ReceiveEnd(
583577
}
584578
}
585579

580+
[Theory]
581+
[MemberData(nameof(ConnectionFilterData))]
582+
public async Task ConnectionClosedAfter101Response(TestServiceContext testContext)
583+
{
584+
using (var server = new TestServer(async httpContext =>
585+
{
586+
var request = httpContext.Request;
587+
var stream = await httpContext.Features.Get<IHttpUpgradeFeature>().UpgradeAsync();
588+
var response = Encoding.ASCII.GetBytes("hello, world");
589+
await stream.WriteAsync(response, 0, response.Length);
590+
}, testContext))
591+
{
592+
using (var connection = server.CreateConnection())
593+
{
594+
await connection.SendEnd(
595+
"GET / HTTP/1.1",
596+
"",
597+
"");
598+
await connection.ReceiveEnd(
599+
"HTTP/1.1 101 Switching Protocols",
600+
"Connection: Upgrade",
601+
$"Date: {testContext.DateHeaderValue}",
602+
"",
603+
"hello, world");
604+
}
605+
606+
using (var connection = server.CreateConnection())
607+
{
608+
await connection.SendEnd(
609+
"GET / HTTP/1.0",
610+
"Connection: kee-alive",
611+
"",
612+
"");
613+
await connection.ReceiveEnd(
614+
"HTTP/1.1 101 Switching Protocols",
615+
"Connection: Upgrade",
616+
$"Date: {testContext.DateHeaderValue}",
617+
"",
618+
"hello, world");
619+
}
620+
}
621+
}
622+
586623
[Theory]
587624
[MemberData(nameof(ConnectionFilterData))]
588625
public async Task ThrowingResultsIn500Response(TestServiceContext testContext)

0 commit comments

Comments
 (0)