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

Commit d8c57d2

Browse files
benaadamsbenaadams
committed
Don't emit TE header or body for non-body responses
1 parent f2085b1 commit d8c57d2

File tree

8 files changed

+444
-69
lines changed

8 files changed

+444
-69
lines changed

‎src/Microsoft.AspNetCore.Server.Kestrel/BadHttpResponse.cs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using Microsoft.AspNetCore.Server.Kestrel.Internal.Http;
7+
8+
namespace Microsoft.AspNetCore.Server.Kestrel
9+
{
10+
public static class BadHttpResponse
11+
{
12+
internal static void ThrowException(ResponseRejectionReasons reason)
13+
{
14+
throw GetException(reason);
15+
}
16+
17+
internal static void ThrowException(ResponseRejectionReasons reason, int value)
18+
{
19+
throw GetException(reason, value.ToString());
20+
}
21+
22+
internal static void ThrowException(ResponseRejectionReasons reason, ResponseRejectionParameter parameter)
23+
{
24+
throw GetException(reason, parameter.ToString());
25+
}
26+
27+
internal static InvalidOperationException GetException(ResponseRejectionReasons reason, int value)
28+
{
29+
return GetException(reason, value.ToString());
30+
}
31+
32+
[MethodImpl(MethodImplOptions.NoInlining)]
33+
internal static InvalidOperationException GetException(ResponseRejectionReasons reason)
34+
{
35+
InvalidOperationException ex;
36+
switch (reason)
37+
{
38+
case ResponseRejectionReasons.HeadersReadonlyResponseStarted:
39+
ex = new InvalidOperationException("Headers are read-only, response has already started.");
40+
break;
41+
case ResponseRejectionReasons.OnStartingCannotBeSetResponseStarted:
42+
ex = new InvalidOperationException("OnStarting cannot be set, response has already started.");
43+
break;
44+
default:
45+
ex = new InvalidOperationException("Bad response.");
46+
break;
47+
}
48+
49+
return ex;
50+
}
51+
52+
[MethodImpl(MethodImplOptions.NoInlining)]
53+
private static InvalidOperationException GetException(ResponseRejectionReasons reason, string value)
54+
{
55+
InvalidOperationException ex;
56+
switch (reason)
57+
{
58+
case ResponseRejectionReasons.ValueCannotBeSetResponseStarted:
59+
ex = new InvalidOperationException(value + " cannot be set, response had already started.");
60+
break;
61+
case ResponseRejectionReasons.TransferEncodingSetOnNonBodyResponse:
62+
ex = new InvalidOperationException($"Transfer-Encoding set on a {value} non-body request.");
63+
break;
64+
case ResponseRejectionReasons.WriteToNonBodyResponse:
65+
ex = new InvalidOperationException($"Write to non-body {value} response.");
66+
break;
67+
default:
68+
ex = new InvalidOperationException("Bad response.");
69+
break;
70+
}
71+
72+
return ex;
73+
}
74+
}
75+
}

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

Lines changed: 133 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public abstract partial class Frame : ConnectionContext, IFrameControl
5858

5959
private RequestProcessingStatus _requestProcessingStatus;
6060
protected bool _keepAlive;
61+
private bool _canHaveBody;
6162
private bool _autoChunk;
6263
protected Exception _applicationException;
6364

@@ -135,7 +136,7 @@ public int StatusCode
135136
{
136137
if (HasResponseStarted)
137138
{
138-
ThrowResponseAlreadyStartedException(nameof(StatusCode));
139+
BadHttpResponse.ThrowException(ResponseRejectionReasons.ValueCannotBeSetResponseStarted, ResponseRejectionParameter.StatusCode);
139140
}
140141

141142
_statusCode = value;
@@ -153,7 +154,7 @@ public string ReasonPhrase
153154
{
154155
if (HasResponseStarted)
155156
{
156-
ThrowResponseAlreadyStartedException(nameof(ReasonPhrase));
157+
BadHttpResponse.ThrowException(ResponseRejectionReasons.ValueCannotBeSetResponseStarted, ResponseRejectionParameter.ReasonPhrase);
157158
}
158159

159160
_reasonPhrase = value;
@@ -388,7 +389,7 @@ public void OnStarting(Func<object, Task> callback, object state)
388389
{
389390
if (HasResponseStarted)
390391
{
391-
ThrowResponseAlreadyStartedException(nameof(OnStarting));
392+
BadHttpResponse.ThrowException(ResponseRejectionReasons.OnStartingCannotBeSetResponseStarted, ResponseRejectionParameter.OnStarting);
392393
}
393394

394395
if (_onStarting == null)
@@ -475,17 +476,24 @@ public void Write(ArraySegment<byte> data)
475476
{
476477
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
477478

478-
if (_autoChunk)
479+
if (_canHaveBody)
479480
{
480-
if (data.Count == 0)
481+
if (_autoChunk)
482+
{
483+
if (data.Count == 0)
484+
{
485+
return;
486+
}
487+
WriteChunked(data);
488+
}
489+
else
481490
{
482-
return;
491+
SocketOutput.Write(data);
483492
}
484-
WriteChunked(data);
485493
}
486494
else
487495
{
488-
SocketOutput.Write(data);
496+
HandleNonBodyResponseWrite(data.Count);
489497
}
490498
}
491499

@@ -496,36 +504,53 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
496504
return WriteAsyncAwaited(data, cancellationToken);
497505
}
498506

499-
if (_autoChunk)
507+
if (_canHaveBody)
500508
{
501-
if (data.Count == 0)
509+
if (_autoChunk)
502510
{
503-
return TaskUtilities.CompletedTask;
511+
if (data.Count == 0)
512+
{
513+
return TaskUtilities.CompletedTask;
514+
}
515+
return WriteChunkedAsync(data, cancellationToken);
516+
}
517+
else
518+
{
519+
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
504520
}
505-
return WriteChunkedAsync(data, cancellationToken);
506521
}
507522
else
508523
{
509-
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
524+
HandleNonBodyResponseWrite(data.Count);
525+
return TaskUtilities.CompletedTask;
510526
}
511527
}
512528

513529
public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken cancellationToken)
514530
{
515531
await ProduceStartAndFireOnStarting();
516532

517-
if (_autoChunk)
533+
if (_canHaveBody)
518534
{
519-
if (data.Count == 0)
535+
if (_autoChunk)
536+
{
537+
if (data.Count == 0)
538+
{
539+
return;
540+
}
541+
await WriteChunkedAsync(data, cancellationToken);
542+
}
543+
else
520544
{
521-
return;
545+
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
522546
}
523-
await WriteChunkedAsync(data, cancellationToken);
524547
}
525548
else
526549
{
527-
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
550+
HandleNonBodyResponseWrite(data.Count);
551+
return;
528552
}
553+
529554
}
530555

531556
private void WriteChunked(ArraySegment<byte> data)
@@ -640,28 +665,14 @@ protected Task ProduceEnd()
640665

641666
if (_requestRejected)
642667
{
643-
// 400 Bad Request
644-
StatusCode = 400;
645668
_keepAlive = false;
669+
// 400 Bad Request
670+
ErrorResetHeadersToDefaults(statusCode: 400);
646671
}
647672
else
648673
{
649674
// 500 Internal Server Error
650-
StatusCode = 500;
651-
}
652-
653-
ReasonPhrase = null;
654-
655-
var responseHeaders = FrameResponseHeaders;
656-
responseHeaders.Reset();
657-
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();
658-
659-
responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
660-
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
661-
662-
if (ServerOptions.AddServerHeader)
663-
{
664-
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
675+
ErrorResetHeadersToDefaults(statusCode: 500);
665676
}
666677
}
667678

@@ -715,50 +726,61 @@ private void CreateResponseHeader(
715726
bool appCompleted)
716727
{
717728
var responseHeaders = FrameResponseHeaders;
718-
responseHeaders.SetReadOnly();
719729

720730
var hasConnection = responseHeaders.HasConnection;
721731

732+
// Set whether response can have body
733+
_canHaveBody = StatusCanHaveBody(StatusCode) && Method != "HEAD";
734+
722735
var end = SocketOutput.ProducingStart();
723736
if (_keepAlive && hasConnection)
724737
{
725738
var connectionValue = responseHeaders.HeaderConnection.ToString();
726739
_keepAlive = connectionValue.Equals("keep-alive", StringComparison.OrdinalIgnoreCase);
727740
}
728-
729-
if (!responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
741+
742+
if (_canHaveBody)
730743
{
731-
if (appCompleted)
744+
if (!responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
732745
{
733-
// Don't set the Content-Length or Transfer-Encoding headers
734-
// automatically for HEAD requests or 101, 204, 205, 304 responses.
735-
if (Method != "HEAD" && StatusCanHaveBody(StatusCode))
746+
if (appCompleted)
736747
{
737748
// Since the app has completed and we are only now generating
738749
// the headers we can safely set the Content-Length to 0.
739750
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
740751
}
741-
}
742-
else if(_keepAlive)
743-
{
744-
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
745-
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
746-
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
747-
// client would break compliance with RFC 7230 (section 3.3.1):
748-
//
749-
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
750-
// request indicates HTTP/1.1 (or later).
751-
if (_httpVersion == Http.HttpVersion.Http11)
752-
{
753-
_autoChunk = true;
754-
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
755-
}
756752
else
757753
{
758-
_keepAlive = false;
754+
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
755+
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
756+
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
757+
// client would break compliance with RFC 7230 (section 3.3.1):
758+
//
759+
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
760+
// request indicates HTTP/1.1 (or later).
761+
if (_httpVersion == Http.HttpVersion.Http11)
762+
{
763+
_autoChunk = true;
764+
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
765+
}
766+
else
767+
{
768+
_keepAlive = false;
769+
}
759770
}
760771
}
761772
}
773+
else
774+
{
775+
// Don't set the Content-Length or Transfer-Encoding headers
776+
// automatically for HEAD requests or 101, 204, 205, 304 responses.
777+
if (responseHeaders.HasTransferEncoding)
778+
{
779+
RejectNonBodyTransferEncodingResponse(appCompleted);
780+
}
781+
}
782+
783+
responseHeaders.SetReadOnly();
762784

763785
if (!_keepAlive && !hasConnection)
764786
{
@@ -1215,12 +1237,63 @@ public bool StatusCanHaveBody(int statusCode)
12151237
statusCode != 304;
12161238
}
12171239

1218-
private void ThrowResponseAlreadyStartedException(string value)
1240+
private void RejectNonBodyTransferEncodingResponse(bool appCompleted)
12191241
{
1220-
throw new InvalidOperationException(value + " cannot be set, response had already started.");
1242+
var ex = BadHttpResponse.GetException(ResponseRejectionReasons.TransferEncodingSetOnNonBodyResponse, StatusCode);
1243+
if (!appCompleted)
1244+
{
1245+
// Back out of header creation surface exeception in user code
1246+
_requestProcessingStatus = RequestProcessingStatus.RequestStarted;
1247+
throw ex;
1248+
}
1249+
else
1250+
{
1251+
ReportApplicationError(ex);
1252+
// 500 Internal Server Error
1253+
ErrorResetHeadersToDefaults(statusCode: 500);
1254+
}
1255+
}
1256+
1257+
private void ErrorResetHeadersToDefaults(int statusCode)
1258+
{
1259+
StatusCode = statusCode;
1260+
ReasonPhrase = null;
1261+
1262+
var responseHeaders = FrameResponseHeaders;
1263+
responseHeaders.Reset();
1264+
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();
1265+
1266+
responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
1267+
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
1268+
1269+
if (ServerOptions.AddServerHeader)
1270+
{
1271+
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
1272+
}
1273+
}
1274+
1275+
public void HandleNonBodyResponseWrite(int count)
1276+
{
1277+
if (Method == "HEAD")
1278+
{
1279+
// Don't write to body for HEAD requests.
1280+
Log.ConnectionHeadResponseBodyWrite(ConnectionId, count);
1281+
}
1282+
else
1283+
{
1284+
// Throw Exception for 101, 204, 205, 304 responses.
1285+
BadHttpResponse.ThrowException(ResponseRejectionReasons.WriteToNonBodyResponse, StatusCode);
1286+
}
12211287
}
12221288

12231289
private void ThrowResponseAbortedException()
1290+
{
1291+
throw new ObjectDisposedException(
1292+
"The response has been aborted due to an unhandled application exception.",
1293+
_applicationException);
1294+
}
1295+
1296+
public void RejectRequest(string message)
12241297
{
12251298
throw new ObjectDisposedException(
12261299
"The response has been aborted due to an unhandled application exception.",

0 commit comments

Comments
 (0)