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

Handle non-body responses #1134

Merged
merged 2 commits into from
Oct 3, 2016
Merged

Handle non-body responses #1134

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
e7e6b89
Don't emit TE header or body for non-body responses
benaadams Jul 9, 2016
4117ad8
Refactor non-body response handling.
Sep 30, 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
210 changes: 152 additions & 58 deletions 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 @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net;
Expand Down Expand Up @@ -60,6 +61,7 @@ public abstract partial class Frame : IFrameControl

private RequestProcessingStatus _requestProcessingStatus;
protected bool _keepAlive;
private bool _canHaveBody;
private bool _autoChunk;
protected Exception _applicationException;

Expand All @@ -73,6 +75,8 @@ public abstract partial class Frame : IFrameControl
protected readonly long _keepAliveMilliseconds;
private readonly long _requestHeadersTimeoutMilliseconds;

private int _responseBytesWritten;
Copy link
Member

Choose a reason for hiding this comment

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

This should be long. Maybe call this _responseBytesIgnored since we're only using it to log the number of bytes not written.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use it when fixing #175, so less diff when I make that change 😄


public Frame(ConnectionContext context)
{
ConnectionContext = context;
Expand Down Expand Up @@ -345,6 +349,8 @@ public void Reset()

_remainingRequestHeadersBytesAllowed = ServerOptions.Limits.MaxRequestHeadersTotalSize;
_requestHeadersParsed = 0;

_responseBytesWritten = 0;
}

/// <summary>
Expand Down Expand Up @@ -511,18 +517,26 @@ public async Task FlushAsync(CancellationToken cancellationToken)
public void Write(ArraySegment<byte> data)
{
ProduceStartAndFireOnStarting().GetAwaiter().GetResult();
_responseBytesWritten += data.Count;

if (_autoChunk)
if (_canHaveBody)
{
if (data.Count == 0)
if (_autoChunk)
{
if (data.Count == 0)
{
return;
}
WriteChunked(data);
}
else
{
return;
SocketOutput.Write(data);
}
WriteChunked(data);
}
else
{
SocketOutput.Write(data);
HandleNonBodyResponseWrite();
}
}

Expand All @@ -533,35 +547,54 @@ public Task WriteAsync(ArraySegment<byte> data, CancellationToken cancellationTo
return WriteAsyncAwaited(data, cancellationToken);
}

if (_autoChunk)
_responseBytesWritten += data.Count;
Copy link
Member

Choose a reason for hiding this comment

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

Could this be done just in the !_canHaveBody case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.


if (_canHaveBody)
{
if (data.Count == 0)
if (_autoChunk)
{
return TaskCache.CompletedTask;
if (data.Count == 0)
{
return TaskCache.CompletedTask;
}
return WriteChunkedAsync(data, cancellationToken);
}
else
{
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
}
return WriteChunkedAsync(data, cancellationToken);
}
else
{
return SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
HandleNonBodyResponseWrite();
return TaskCache.CompletedTask;
}
}

public async Task WriteAsyncAwaited(ArraySegment<byte> data, CancellationToken cancellationToken)
{
await ProduceStartAndFireOnStarting();
_responseBytesWritten += data.Count;

if (_autoChunk)
if (_canHaveBody)
{
if (data.Count == 0)
if (_autoChunk)
{
if (data.Count == 0)
{
return;
}
await WriteChunkedAsync(data, cancellationToken);
}
else
{
return;
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
}
await WriteChunkedAsync(data, cancellationToken);
}
else
{
await SocketOutput.WriteAsync(data, cancellationToken: cancellationToken);
HandleNonBodyResponseWrite();
return;
}
}

Expand Down Expand Up @@ -675,25 +708,11 @@ protected Task ProduceEnd()
return TaskCache.CompletedTask;
}

// If the request was rejected, StatusCode has already been set by SetBadRequestState
// If the request was rejected, the error state has already been set by SetBadRequestState
if (!_requestRejected)
{
// 500 Internal Server Error
StatusCode = 500;
}

ReasonPhrase = null;

var responseHeaders = FrameResponseHeaders;
responseHeaders.Reset();
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();

responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);

if (ServerOptions.AddServerHeader)
{
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
SetErrorResponseHeaders(statusCode: 500);
}
}

Expand Down Expand Up @@ -729,6 +748,11 @@ private Task WriteSuffix()
ConnectionControl.End(ProduceEndType.ConnectionKeepAlive);
}

if (HttpMethods.IsHead(Method) && _responseBytesWritten > 0)
{
Log.ConnectionHeadResponseBodyWrite(ConnectionId, _responseBytesWritten);
}

return TaskCache.CompletedTask;
}

Expand All @@ -747,50 +771,61 @@ private void CreateResponseHeader(
bool appCompleted)
{
var responseHeaders = FrameResponseHeaders;
responseHeaders.SetReadOnly();

var hasConnection = responseHeaders.HasConnection;

var end = SocketOutput.ProducingStart();

if (_keepAlive && hasConnection)
{
var connectionValue = responseHeaders.HeaderConnection.ToString();
_keepAlive = connectionValue.Equals("keep-alive", StringComparison.OrdinalIgnoreCase);
}

if (!responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
// Set whether response can have body
_canHaveBody = StatusCanHaveBody(StatusCode) && Method != "HEAD";
Copy link
Member

Choose a reason for hiding this comment

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

Since 101 status can have a body of sorts, I would prefer _canHaveBody is true in that case. When deciding whether or not to set Content-Length: 0 we can check for 101 specifically.


// Don't set the Content-Length or Transfer-Encoding headers
// automatically for HEAD requests or 204, 205, 304 responses.
if (_canHaveBody)
{
if (appCompleted)
if (!responseHeaders.HasTransferEncoding && !responseHeaders.HasContentLength)
{
// Don't set the Content-Length or Transfer-Encoding headers
// automatically for HEAD requests or 101, 204, 205, 304 responses.
if (Method != "HEAD" && StatusCanHaveBody(StatusCode))
if (appCompleted && StatusCode != 101)
{
// Since the app has completed and we are only now generating
// the headers we can safely set the Content-Length to 0.
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);
}
}
else if (_keepAlive)
{
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
// client would break compliance with RFC 7230 (section 3.3.1):
//
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
// request indicates HTTP/1.1 (or later).
if (_httpVersion == Http.HttpVersion.Http11)
{
_autoChunk = true;
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
}
else
{
_keepAlive = false;
// Note for future reference: never change this to set _autoChunk to true on HTTP/1.0
// connections, even if we were to infer the client supports it because an HTTP/1.0 request
// was received that used chunked encoding. Sending a chunked response to an HTTP/1.0
// client would break compliance with RFC 7230 (section 3.3.1):
//
// A server MUST NOT send a response containing Transfer-Encoding unless the corresponding
// request indicates HTTP/1.1 (or later).
if (_httpVersion == Http.HttpVersion.Http11 && StatusCode != 101)
{
_autoChunk = true;
responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked);
}
else
{
_keepAlive = false;
}
}
}
}
else
{
if (responseHeaders.HasTransferEncoding)
{
RejectNonBodyTransferEncodingResponse(appCompleted);
}
}

responseHeaders.SetReadOnly();

if (!_keepAlive && !hasConnection)
{
Expand Down Expand Up @@ -1249,18 +1284,77 @@ public bool TakeMessageHeaders(SocketInput input, FrameRequestHeaders requestHea
public bool StatusCanHaveBody(int statusCode)
{
// List of status codes taken from Microsoft.Net.Http.Server.Response
return statusCode != 101 &&
statusCode != 204 &&
return statusCode != 204 &&
statusCode != 205 &&
statusCode != 304;
}

private void ThrowResponseAlreadyStartedException(string value)
{
throw new InvalidOperationException(value + " cannot be set, response had already started.");
throw new InvalidOperationException($"{value} cannot be set, response has already started.");
}

private void RejectNonBodyTransferEncodingResponse(bool appCompleted)
{
var ex = new InvalidOperationException($"Transfer-Encoding set on a {StatusCode} non-body request.");
if (!appCompleted)
{
// Back out of header creation surface exeception in user code
_requestProcessingStatus = RequestProcessingStatus.RequestStarted;
throw ex;
}
else
{
ReportApplicationError(ex);

// 500 Internal Server Error
SetErrorResponseHeaders(statusCode: 500);
}
}

private void SetErrorResponseHeaders(int statusCode)
{
Debug.Assert(!HasResponseStarted, $"{nameof(SetErrorResponseHeaders)} called after response had already started.");

StatusCode = statusCode;
ReasonPhrase = null;

if (FrameResponseHeaders == null)
{
InitializeHeaders();
}

var responseHeaders = FrameResponseHeaders;
responseHeaders.Reset();
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();

responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
responseHeaders.SetRawContentLength("0", _bytesContentLengthZero);

if (ServerOptions.AddServerHeader)
{
responseHeaders.SetRawServer(Constants.ServerName, _bytesServer);
}
}

public void HandleNonBodyResponseWrite()
{
// Writes to HEAD response are ignored and logged at the end of the request
if (Method != "HEAD")
{
// Throw Exception for 204, 205, 304 responses.
throw new InvalidOperationException($"Write to non-body {StatusCode} response.");
}
}

private void ThrowResponseAbortedException()
{
throw new ObjectDisposedException(
"The response has been aborted due to an unhandled application exception.",
_applicationException);
}

public void RejectRequest(string message)
{
throw new ObjectDisposedException(
"The response has been aborted due to an unhandled application exception.",
Expand Down Expand Up @@ -1291,7 +1385,7 @@ public void SetBadRequestState(BadHttpRequestException ex)
// Setting status code will throw if response has already started
if (!HasResponseStarted)
{
StatusCode = ex.StatusCode;
SetErrorResponseHeaders(statusCode: ex.StatusCode);
}

_keepAlive = false;
Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/IKestrelTrace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public interface IKestrelTrace : ILogger

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

void ConnectionHeadResponseBodyWrite(string connectionId, int count);

void ConnectionBadRequest(string connectionId, BadHttpRequestException ex);

void NotAllConnectionsClosedGracefully();
Expand Down
7 changes: 7 additions & 0 deletions src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/KestrelTrace.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class KestrelTrace : IKestrelTrace
private static readonly Action<ILogger, string, Exception> _applicationError;
private static readonly Action<ILogger, string, Exception> _connectionError;
private static readonly Action<ILogger, string, int, Exception> _connectionDisconnectedWrite;
private static readonly Action<ILogger, string, int, Exception> _connectionHeadResponseBodyWrite;
private static readonly Action<ILogger, Exception> _notAllConnectionsClosedGracefully;
private static readonly Action<ILogger, string, string, Exception> _connectionBadRequest;

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

public KestrelTrace(ILogger logger)
Expand Down Expand Up @@ -133,6 +135,11 @@ public virtual void ConnectionDisconnectedWrite(string connectionId, int count,
_connectionDisconnectedWrite(_logger, connectionId, count, ex);
}

public virtual void ConnectionHeadResponseBodyWrite(string connectionId, int count)
{
_connectionHeadResponseBodyWrite(_logger, connectionId, count, null);
}

public virtual void NotAllConnectionsClosedGracefully()
{
_notAllConnectionsClosedGracefully(_logger, null);
Expand Down
Loading