Skip to content

Commit 532a47f

Browse files
benaadamsbenaadams
committed
Don't emit TE header or body for non-body responses
Resolves aspnet#952
1 parent d9b55e7 commit 532a47f

File tree

5 files changed

+156
-52
lines changed

5 files changed

+156
-52
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
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+
6+
namespace Microsoft.AspNetCore.Server.Kestrel
7+
{
8+
public sealed class BadHttpResponseException : InvalidOperationException
9+
{
10+
internal BadHttpResponseException(string message)
11+
: base(message)
12+
{
13+
14+
}
15+
}
16+
}

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

Lines changed: 130 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public abstract partial class Frame : ConnectionContext, IFrameControl
5959

6060
protected RequestProcessingStatus _requestProcessingStatus;
6161
protected bool _keepAlive;
62+
private bool _canHaveBody;
6263
private bool _autoChunk;
6364
protected Exception _applicationException;
6465

@@ -486,17 +487,24 @@ public void Write(ArraySegment<byte> data)
486487
{
487488
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
488489

489-
if (_autoChunk)
490+
if (_canHaveBody)
490491
{
491-
if (data.Count == 0)
492+
if (_autoChunk)
492493
{
493-
return;
494+
if (data.Count == 0)
495+
{
496+
return;
497+
}
498+
WriteChunked(data);
499+
}
500+
else
501+
{
502+
SocketOutput.Write(data);
494503
}
495-
WriteChunked(data);
496504
}
497505
else
498506
{
499-
SocketOutput.Write(data);
507+
HandleNonBodyResponseWrite(data.Count);
500508
}
501509
}
502510

@@ -507,36 +515,53 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
507515
return WriteAsyncAwaited(data, cancellationToken);
508516
}
509517

510-
if (_autoChunk)
518+
if (_canHaveBody)
511519
{
512-
if (data.Count == 0)
520+
if (_autoChunk)
513521
{
514-
return TaskUtilities.CompletedTask;
522+
if (data.Count == 0)
523+
{
524+
return TaskUtilities.CompletedTask;
525+
}
526+
return WriteChunkedAsync(data, cancellationToken);
527+
}
528+
else
529+
{
530+
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
515531
}
516-
return WriteChunkedAsync(data, cancellationToken);
517532
}
518533
else
519534
{
520-
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
535+
HandleNonBodyResponseWrite(data.Count);
536+
return TaskUtilities.CompletedTask;
521537
}
522538
}
523539

524540
public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken cancellationToken)
525541
{
526542
await ProduceStartAndFireOnStarting();
527543

528-
if (_autoChunk)
544+
if (_canHaveBody)
529545
{
530-
if (data.Count == 0)
546+
if (_autoChunk)
531547
{
532-
return;
548+
if (data.Count == 0)
549+
{
550+
return;
551+
}
552+
await WriteChunkedAsync(data, cancellationToken);
553+
}
554+
else
555+
{
556+
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
533557
}
534-
await WriteChunkedAsync(data, cancellationToken);
535558
}
536559
else
537560
{
538-
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
561+
HandleNonBodyResponseWrite(data.Count);
562+
return;
539563
}
564+
540565
}
541566

542567
private void WriteChunked(ArraySegment<byte> data)
@@ -664,19 +689,7 @@ protected Task ProduceEnd()
664689
StatusCode = 500;
665690
}
666691

667-
ReasonPhrase = null;
668-
669-
var responseHeaders = FrameResponseHeaders;
670-
responseHeaders.Reset();
671-
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();
672-
673-
responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
674-
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
675-
676-
if (ServerOptions.AddServerHeader)
677-
{
678-
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
679-
}
692+
ErrorResetHeadersToDefaults();
680693
}
681694

682695
if (!HasResponseStarted)
@@ -729,10 +742,12 @@ private void CreateResponseHeader(
729742
bool appCompleted)
730743
{
731744
var responseHeaders = FrameResponseHeaders;
732-
responseHeaders.SetReadOnly();
733745

734746
var hasConnection = responseHeaders.HasConnection;
735747

748+
// Set whether response can have body
749+
_canHaveBody = StatusCanHaveBody(StatusCode) && !ReferenceEquals(Method, KnownStrings.HttpHeadMethod);
750+
736751
var end = SocketOutput.ProducingStart();
737752
if (_keepAlive && hasConnection)
738753
{
@@ -746,39 +761,48 @@ private void CreateResponseHeader(
746761
}
747762
}
748763

749-
if (_keepAlive && !responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
764+
if (_canHaveBody)
750765
{
751-
if (appCompleted)
766+
if (_keepAlive && !responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
752767
{
753-
// Don't set the Content-Length or Transfer-Encoding headers
754-
// automatically for HEAD requests or 101, 204, 205, 304 responses.
755-
if (Method != "HEAD" && StatusCanHaveBody(StatusCode))
768+
if (appCompleted)
756769
{
757770
// Since the app has completed and we are only now generating
758771
// the headers we can safely set the Content-Length to 0.
759772
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
760773
}
761-
}
762-
else
763-
{
764-
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
765-
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
766-
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
767-
// client would break compliance with RFC 7230 (section 3.3.1):
768-
//
769-
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
770-
// request indicates HTTP/1.1 (or later).
771-
if (_httpVersion == HttpVersionType.Http11)
772-
{
773-
_autoChunk = true;
774-
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
775-
}
776774
else
777775
{
778-
_keepAlive = false;
776+
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
777+
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
778+
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
779+
// client would break compliance with RFC 7230 (section 3.3.1):
780+
//
781+
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
782+
// request indicates HTTP/1.1 (or later).
783+
if (_httpVersion == HttpVersionType.Http11)
784+
{
785+
_autoChunk = true;
786+
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
787+
}
788+
else
789+
{
790+
_keepAlive = false;
791+
}
779792
}
780793
}
781794
}
795+
else
796+
{
797+
// Don't set the Content-Length or Transfer-Encoding headers
798+
// automatically for HEAD requests or 101, 204, 205, 304 responses.
799+
if (responseHeaders.HasTransferEncoding)
800+
{
801+
RejectNonBodyTransferEncodingResponse(appCompleted);
802+
}
803+
}
804+
805+
responseHeaders.SetReadOnly();
782806

783807
if (!_keepAlive && !hasConnection && _httpVersion != HttpVersionType.Http10)
784808
{
@@ -1232,10 +1256,65 @@ public bool StatusCanHaveBody(int statusCode)
12321256
statusCode != 304;
12331257
}
12341258

1259+
[MethodImpl(MethodImplOptions.NoInlining)]
1260+
private void RejectNonBodyTransferEncodingResponse(bool appCompleted)
1261+
{
1262+
var ex = new BadHttpResponseException($"Transfer-Encoding set on a {StatusCode} non-body request.");
1263+
if (!appCompleted)
1264+
{
1265+
// Back out of header creation surface exeception in user code
1266+
_requestProcessingStatus = RequestProcessingStatus.RequestStarted;
1267+
throw ex;
1268+
}
1269+
else
1270+
{
1271+
ReportApplicationError(ex);
1272+
// 500 Internal Server Error
1273+
StatusCode = 500;
1274+
1275+
ErrorResetHeadersToDefaults();
1276+
}
1277+
}
1278+
1279+
private void ErrorResetHeadersToDefaults()
1280+
{
1281+
ReasonPhrase = null;
1282+
1283+
var responseHeaders = FrameResponseHeaders;
1284+
responseHeaders.Reset();
1285+
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();
1286+
1287+
responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
1288+
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
1289+
1290+
if (ServerOptions.AddServerHeader)
1291+
{
1292+
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
1293+
}
1294+
}
1295+
1296+
[MethodImpl(MethodImplOptions.NoInlining)]
1297+
public void HandleNonBodyResponseWrite(int count)
1298+
{
1299+
if (ReferenceEquals(Method, KnownStrings.HttpHeadMethod))
1300+
{
1301+
// Don't write to body for HEAD requests.
1302+
if (Log.IsEnabled(LogLevel.Debug))
1303+
{
1304+
Log.ConnectionHeadResponseBodyWrite(ConnectionId, count);
1305+
}
1306+
}
1307+
else
1308+
{
1309+
// Throw Exception for 101, 204, 205, 304 responses.
1310+
throw new BadHttpResponseException($"Write to non-body {StatusCode} response.");
1311+
}
1312+
}
1313+
12351314
[MethodImpl(MethodImplOptions.NoInlining)]
12361315
private void ThrowResponseAlreadyStartedException(string value)
12371316
{
1238-
throw new InvalidOperationException(value + " cannot be set, response had already started.");
1317+
throw new BadHttpResponseException(value + " cannot be set, response had already started.");
12391318
}
12401319

12411320
[MethodImpl(MethodImplOptions.NoInlining)]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ StringValues IDictionary<string, StringValues>.this[string key]
5252
[MethodImpl(MethodImplOptions.NoInlining)]
5353
protected void ThrowHeadersReadOnlyException()
5454
{
55-
throw new InvalidOperationException("Headers are read-only, response has already started.");
55+
throw new BadHttpResponseException("Headers are read-only, response has already started.");
5656
}
5757

5858
[MethodImpl(MethodImplOptions.NoInlining)]

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ public interface IKestrelTrace : ILogger
3333

3434
void ConnectionDisconnectedWrite(string connectionId, int count, Exception ex);
3535

36+
void ConnectionHeadResponseBodyWrite(string connectionId, int count);
37+
3638
void ConnectionBadRequest(string connectionId, BadHttpRequestException ex);
3739

3840
void NotAllConnectionsClosedGracefully();

‎src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ public class KestrelTrace : IKestrelTrace
2424
private static readonly Action<ILogger, string, Exception> _applicationError;
2525
private static readonly Action<ILogger, string, Exception> _connectionError;
2626
private static readonly Action<ILogger, string, int, Exception> _connectionDisconnectedWrite;
27+
private static readonly Action<ILogger, string, int, Exception> _connectionHeadResponseBodyWrite;
2728
private static readonly Action<ILogger, Exception> _notAllConnectionsClosedGracefully;
2829
private static readonly Action<ILogger, string, string, Exception> _connectionBadRequest;
2930

@@ -48,6 +49,7 @@ static KestrelTrace()
4849
_connectionDisconnectedWrite = LoggerMessage.Define<string, int>(LogLevel.Debug, 15, @"Connection id ""{ConnectionId}"" write of ""{count}"" bytes to disconnected client.");
4950
_notAllConnectionsClosedGracefully = LoggerMessage.Define(LogLevel.Debug, 16, "Some connections failed to close gracefully during server shutdown.");
5051
_connectionBadRequest = LoggerMessage.Define<string, string>(LogLevel.Information, 17, @"Connection id ""{ConnectionId}"" bad request data: ""{message}""");
52+
_connectionHeadResponseBodyWrite = LoggerMessage.Define<string, int>(LogLevel.Debug, 18, @"Connection id ""{ConnectionId}"" write of ""{count}"" body bytes to non-body HEAD response.");
5153
}
5254

5355
public KestrelTrace(ILogger logger)
@@ -133,6 +135,11 @@ public virtual void ConnectionDisconnectedWrite(string connectionId, int count,
133135
_connectionDisconnectedWrite(_logger, connectionId, count, ex);
134136
}
135137

138+
public virtual void ConnectionHeadResponseBodyWrite(string connectionId, int count)
139+
{
140+
_connectionHeadResponseBodyWrite(_logger, connectionId, count, null);
141+
}
142+
136143
public virtual void NotAllConnectionsClosedGracefully()
137144
{
138145
_notAllConnectionsClosedGracefully(_logger, null);

0 commit comments

Comments
 (0)