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

Set Date and Server headers on response start #862

Merged
halter73 merged 1 commit into from
May 26, 2016
Merged

Set Date and Server headers on response start #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 10 additions & 8 deletions src/Microsoft.AspNetCore.Server.Kestrel/Http/DateHeaderValueManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,24 @@ public class DateHeaderValueManager : IDisposable
/// Initializes a new instance of the <see cref="DateHeaderValueManager"/> class.
/// </summary>
public DateHeaderValueManager()
: this(
systemClock: new SystemClock(),
timeWithoutRequestsUntilIdle: TimeSpan.FromSeconds(10),
timerInterval: TimeSpan.FromSeconds(1))
: this(systemClock: new SystemClock())
{
}

// Internal for testing
internal DateHeaderValueManager(
ISystemClock systemClock,
TimeSpan timeWithoutRequestsUntilIdle,
TimeSpan timerInterval)
TimeSpan? timeWithoutRequestsUntilIdle = null,
TimeSpan? timerInterval = null)
{
if (systemClock == null)
{
throw new ArgumentNullException(nameof(systemClock));
}

_systemClock = systemClock;
_timeWithoutRequestsUntilIdle = timeWithoutRequestsUntilIdle;
_timerInterval = timerInterval;
_timeWithoutRequestsUntilIdle = timeWithoutRequestsUntilIdle ?? TimeSpan.FromSeconds(10);
_timerInterval = timerInterval ?? TimeSpan.FromSeconds(1);
_dateValueTimer = new Timer(TimerLoop, state: null, dueTime: Timeout.Infinite, period: Timeout.Infinite);
}

Expand Down
40 changes: 27 additions & 13 deletions src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public abstract partial class Frame : ConnectionContext, IFrameControl
private static readonly byte[] _bytesHttpVersion11 = Encoding.ASCII.GetBytes("HTTP/1.1 ");
private static readonly byte[] _bytesContentLengthZero = Encoding.ASCII.GetBytes("\r\nContent-Length: 0");
private static readonly byte[] _bytesEndHeaders = Encoding.ASCII.GetBytes("\r\n\r\n");
private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: Kestrel");

private static Vector<byte> _vectorCRs = new Vector<byte>((byte)'\r');
private static Vector<byte> _vectorColons = new Vector<byte>((byte)':');
Expand All @@ -44,7 +45,6 @@ public abstract partial class Frame : ConnectionContext, IFrameControl
private readonly object _onCompletedSync = new Object();

private bool _requestRejected;
private Headers _frameHeaders;
private Streams _frameStreams;

protected List<KeyValuePair<Func<object, Task>, object>> _onStarting;
Expand Down Expand Up @@ -210,21 +210,23 @@ public bool HasResponseStarted
get { return _requestProcessingStatus == RequestProcessingStatus.ResponseStarted; }
}

protected FrameRequestHeaders FrameRequestHeaders => _frameHeaders.RequestHeaders;
protected FrameRequestHeaders FrameRequestHeaders { get; private set; }

protected FrameResponseHeaders FrameResponseHeaders { get; private set; }

public void InitializeHeaders()
{
if (_frameHeaders == null)
if (FrameRequestHeaders == null)
{
_frameHeaders = new Headers(ServerOptions);
RequestHeaders = _frameHeaders.RequestHeaders;
ResponseHeaders = _frameHeaders.ResponseHeaders;
RequestHeaders = FrameRequestHeaders = new FrameRequestHeaders();
Copy link
Member

@halter73 halter73 May 24, 2016

Choose a reason for hiding this comment

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

This looks kind of gross. Can we just change the type of the RequestHeaders and ResponseHeaders properties from IHeaderDictionary to FrameRequestHeaders and FrameResponseHeaders and use those properties everywhere?

Copy link
Contributor Author

@khellang khellang May 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure it becomes much better. You'd get into issues with the explicit implementations of IDictionary<string, StringValues> in FrameHeaders and casting in Frame.FeatureCollection. Example:

IHeaderDictionary IHttpRequestFeature.Headers
{
    get
    {
        return RequestHeaders;
    }

    set
    {
        // This won't work if RequestHeaders is FrameRequestHeaders. Cast?
        RequestHeaders = value;
    }
}

Copy link
Member

@halter73 halter73 May 24, 2016

Choose a reason for hiding this comment

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

Settable features. What a pain! It makes sense now though.

Is RequestHeaders even reset between requests? It doesn't look like it, and I think it needs to be. This seems to be a preexisting issue, but we might as well fix it now that we're looking at it.

It looks like ResponseHeaders is reset in ProduceEnd which works I guess.

@Tratcher Why is IHttpResponseFeature.Headers settable to begin with? It's not like Kestrel would send those headers to begin with.

Copy link
Member

@Tratcher Tratcher May 24, 2016

Choose a reason for hiding this comment

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

@halter73 largely because everything else is. in theory you could wrap the headers collection and apply filters/transforms. No, I would not expect Kestrel to send headers from a new dictionary.

Copy link
Contributor Author

@khellang khellang May 25, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@halter73 Uuuh. Isn't this resetting RequestHeaders? FrameRequestHeaders and RequestHeaders are the same, remember? 😉 Which in turn is the same as IHttpRequestFeature.Headers... 😖

Copy link
Member

@halter73 halter73 May 25, 2016

Choose a reason for hiding this comment

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

@khellang I was referring specifically to the issue you filed in #879 which is that the RequestHeaders reference should be reset to FrameRequestHeaders like the ResponseHeaders reference is reset to FrameResponseHeaders in ProduceEnd. Not that Reset() needs to be called on the RequestHeaders object itself.

}

_frameHeaders.Initialize(DateHeaderValueManager);
if (FrameResponseHeaders == null)
{
ResponseHeaders = FrameResponseHeaders = new FrameResponseHeaders();
}
}


public void InitializeStreams(MessageBody messageBody)
{
if (_frameStreams == null)
Expand Down Expand Up @@ -259,7 +261,8 @@ public void StopStreams()

public void Reset()
{
_frameHeaders?.Reset();
FrameRequestHeaders?.Reset();
FrameResponseHeaders?.Reset();

_onStarting = null;
_onCompleted = null;
Expand Down Expand Up @@ -598,7 +601,7 @@ protected Task TryProduceInvalidRequestResponse()
{
if (_requestProcessingStatus == RequestProcessingStatus.RequestStarted && _requestRejected)
{
if (_frameHeaders == null)
if (FrameRequestHeaders == null || FrameResponseHeaders == null)
{
InitializeHeaders();
}
Expand Down Expand Up @@ -634,7 +637,7 @@ protected Task ProduceEnd()

ReasonPhrase = null;

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

Expand All @@ -643,7 +646,7 @@ protected Task ProduceEnd()

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

ResponseHeaders = responseHeaders;
Expand Down Expand Up @@ -698,7 +701,7 @@ private void CreateResponseHeader(
byte[] statusBytes,
bool appCompleted)
{
var responseHeaders = _frameHeaders.ResponseHeaders;
var responseHeaders = FrameResponseHeaders;
responseHeaders.SetReadOnly();

var hasConnection = responseHeaders.HasConnection;
Expand Down Expand Up @@ -759,6 +762,17 @@ private void CreateResponseHeader(
responseHeaders.SetRawConnection("keep-alive", _bytesConnectionKeepAlive);
}

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

if (!responseHeaders.HasDate)
{
var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues();
responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes);
}

end.CopyFrom(_bytesHttpVersion11);
end.CopyFrom(statusBytes);
responseHeaders.CopyTo(ref end);
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.AspNetCore.Server.Kestrel/Http/FrameResponseHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public partial class FrameResponseHeaders : FrameHeaders

public bool HasContentLength => HeaderContentLength.Count != 0;

public bool HasServer => HeaderServer.Count != 0;

public bool HasDate => HeaderDate.Count != 0;

public Enumerator GetEnumerator()
{
Expand Down
40 changes: 0 additions & 40 deletions src/Microsoft.AspNetCore.Server.Kestrel/Infrastructure/Headers.cs
View file
Open in desktop

This file was deleted.

7 changes: 3 additions & 4 deletions test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public async Task TestBadRequestLines(string request)
{
using (var server = new TestServer(context => { return Task.FromResult(0); }))
{
using (var connection = new TestConnection(server.Port))
using (var connection = server.CreateConnection())
{
await connection.SendEnd(request);
await ReceiveBadRequestResponse(connection);
Expand All @@ -87,7 +87,7 @@ public async Task ServerClosesConnectionAsSoonAsBadRequestLineIsDetected(string
{
using (var server = new TestServer(context => { return Task.FromResult(0); }))
{
using (var connection = new TestConnection(server.Port))
using (var connection = server.CreateConnection())
{
await connection.Send(request);
await ReceiveBadRequestResponse(connection);
Expand All @@ -103,10 +103,9 @@ await connection.Receive(
await connection.Receive(
"Connection: close",
"");
await connection.ReceiveStartsWith("Date: ");
await connection.ReceiveEnd(
$"Date: {connection.Server.Context.DateHeaderValue}",
"Content-Length: 0",
"Server: Kestrel",
"",
Copy link
Member

@halter73 halter73 May 24, 2016

Choose a reason for hiding this comment

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

Are there any remaining tests the verify that the "Server: Kestrel" header is sent by default?

"");
}
Expand Down
Loading