From 3f4e2323f4f577d782e30481be7b6f2081d480ad Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Fri, 20 May 2016 12:12:28 -0700 Subject: [PATCH] Throw when setting Frame.StatusCode or Frame.ReasonPhrase after response has already started (#805). --- .../Http/Frame.cs | 78 +++++++++++++------ .../FrameFacts.cs | 32 -------- .../FrameResponseHeadersTests.cs | 42 ++++++++++ .../FrameTests.cs | 65 ++++++++++++++++ .../TestHelpers/MockSocketOuptut.cs | 32 ++++++++ 5 files changed, 194 insertions(+), 55 deletions(-) delete mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/FrameFacts.cs create mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockSocketOuptut.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs index 16c431445a..2716a94e52 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Http/Frame.cs @@ -121,8 +121,42 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http public IHeaderDictionary RequestHeaders { get; set; } public Stream RequestBody { get; set; } - public int StatusCode { get; set; } - public string ReasonPhrase { get; set; } + private int _statusCode; + public int StatusCode + { + get + { + return _statusCode; + } + set + { + if (HasResponseStarted) + { + throw new InvalidOperationException("Status code cannot be set, response has already started."); + } + + _statusCode = value; + } + } + + private string _reasonPhrase; + public string ReasonPhrase + { + get + { + return _reasonPhrase; + } + set + { + if (HasResponseStarted) + { + throw new InvalidOperationException("Reason phrase cannot be set, response had already started."); + } + + _reasonPhrase = value; + } + } + public IHeaderDictionary ResponseHeaders { get; set; } public Stream ResponseBody { get; set; } @@ -579,6 +613,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http { if (_requestRejected || _applicationException != null) { + if (HasResponseStarted) + { + // We can no longer change the response, so we simply close the connection. + _requestProcessingStopping = true; + return TaskUtilities.CompletedTask; + } + if (_requestRejected) { // 400 Bad Request @@ -591,30 +632,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http StatusCode = 500; } - if (HasResponseStarted) + ReasonPhrase = null; + + var responseHeaders = _frameHeaders.ResponseHeaders; + responseHeaders.Reset(); + var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues(); + + responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes); + responseHeaders.SetRawContentLength("0", _bytesContentLengthZero); + + if (ServerOptions.AddServerHeader) { - // We can no longer respond with a 500, so we simply close the connection. - _requestProcessingStopping = true; - return TaskUtilities.CompletedTask; + responseHeaders.SetRawServer(Constants.ServerName, Headers.BytesServer); } - else - { - ReasonPhrase = null; - var responseHeaders = _frameHeaders.ResponseHeaders; - responseHeaders.Reset(); - var dateHeaderValues = DateHeaderValueManager.GetDateHeaderValues(); - - responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes); - responseHeaders.SetRawContentLength("0", _bytesContentLengthZero); - - if (ServerOptions.AddServerHeader) - { - responseHeaders.SetRawServer(Constants.ServerName, Headers.BytesServer); - } - - ResponseHeaders = responseHeaders; - } + ResponseHeaders = responseHeaders; } if (!HasResponseStarted) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameFacts.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameFacts.cs deleted file mode 100644 index deefd5511d..0000000000 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameFacts.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http.Features; -using Microsoft.AspNetCore.Server.Kestrel; -using Microsoft.AspNetCore.Server.Kestrel.Http; -using Xunit; - -namespace Microsoft.AspNetCore.Server.KestrelTests -{ - public class FrameFacts - { - [Fact] - public void ResetResetsScheme() - { - // Arrange - var connectionContext = new ConnectionContext() - { - DateHeaderValueManager = new DateHeaderValueManager(), - ServerAddress = ServerAddress.FromUrl("http://localhost:5000") - }; - var frame = new Frame(application: null, context: connectionContext); - frame.Scheme = "https"; - - // Act - frame.Reset(); - - // Assert - Assert.Equal("http", ((IFeatureCollection)frame).Get().Scheme); - } - } -} diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs index 41f3fd3f78..ce6098a5b2 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs @@ -125,5 +125,47 @@ namespace Microsoft.AspNetCore.Server.KestrelTests ((IDictionary)responseHeaders).Add(key, value); }); } + + [Fact] + public void ThrowsWhenAddingHeaderAfterReadOnlyIsSet() + { + var headers = new FrameResponseHeaders(); + headers.SetReadOnly(); + + Assert.Throws(() => ((IDictionary)headers).Add("my-header", new[] { "value" })); + } + + [Fact] + public void ThrowsWhenChangingHeaderAfterReadOnlyIsSet() + { + var headers = new FrameResponseHeaders(); + var dictionary = (IDictionary)headers; + dictionary.Add("my-header", new[] { "value" }); + headers.SetReadOnly(); + + Assert.Throws(() => dictionary["my-header"] = "other-value"); + } + + [Fact] + public void ThrowsWhenRemovingHeaderAfterReadOnlyIsSet() + { + var headers = new FrameResponseHeaders(); + var dictionary = (IDictionary)headers; + dictionary.Add("my-header", new[] { "value" }); + headers.SetReadOnly(); + + Assert.Throws(() => dictionary.Remove("my-header")); + } + + [Fact] + public void ThrowsWhenClearingHeadersAfterReadOnlyIsSet() + { + var headers = new FrameResponseHeaders(); + var dictionary = (IDictionary)headers; + dictionary.Add("my-header", new[] { "value" }); + headers.SetReadOnly(); + + Assert.Throws(() => dictionary.Clear()); + } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 8d6133a5f3..809dde54ab 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -4,9 +4,11 @@ using System; using System.Linq; using System.Text; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Http; using Microsoft.AspNetCore.Server.Kestrel.Infrastructure; +using Microsoft.AspNetCore.Server.KestrelTests.TestHelpers; using Xunit; namespace Microsoft.AspNetCore.Server.KestrelTests @@ -50,5 +52,68 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.True(scan.IsEnd); } } + + [Fact] + public void ResetResetsScheme() + { + // Arrange + var connectionContext = new ConnectionContext() + { + DateHeaderValueManager = new DateHeaderValueManager(), + ServerAddress = ServerAddress.FromUrl("http://localhost:5000") + }; + var frame = new Frame(application: null, context: connectionContext); + frame.Scheme = "https"; + + // Act + frame.Reset(); + + // Assert + Assert.Equal("http", ((IFeatureCollection)frame).Get().Scheme); + } + + [Fact] + public void ThrowsWhenStatusCodeIsSetAfterResponseStarted() + { + // Arrange + var connectionContext = new ConnectionContext() + { + DateHeaderValueManager = new DateHeaderValueManager(), + ServerAddress = ServerAddress.FromUrl("http://localhost:5000"), + ServerOptions = new KestrelServerOptions(), + SocketOutput = new MockSocketOuptut() + }; + var frame = new Frame(application: null, context: connectionContext); + frame.InitializeHeaders(); + + // Act + frame.Write(new ArraySegment(new byte[1])); + + // Assert + Assert.True(frame.HasResponseStarted); + Assert.Throws(() => ((IHttpResponseFeature)frame).StatusCode = 404); + } + + [Fact] + public void ThrowsWhenReasonPhraseIsSetAfterResponseStarted() + { + // Arrange + var connectionContext = new ConnectionContext() + { + DateHeaderValueManager = new DateHeaderValueManager(), + ServerAddress = ServerAddress.FromUrl("http://localhost:5000"), + ServerOptions = new KestrelServerOptions(), + SocketOutput = new MockSocketOuptut() + }; + var frame = new Frame(application: null, context: connectionContext); + frame.InitializeHeaders(); + + // Act + frame.Write(new ArraySegment(new byte[1])); + + // Assert + Assert.True(frame.HasResponseStarted); + Assert.Throws(() => ((IHttpResponseFeature)frame).ReasonPhrase = "Reason phrase"); + } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockSocketOuptut.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockSocketOuptut.cs new file mode 100644 index 0000000000..051074c7d8 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/TestHelpers/MockSocketOuptut.cs @@ -0,0 +1,32 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Http; +using Microsoft.AspNetCore.Server.Kestrel.Infrastructure; + +namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers +{ + public class MockSocketOuptut : ISocketOutput + { + public void ProducingComplete(MemoryPoolIterator end) + { + } + + public MemoryPoolIterator ProducingStart() + { + return new MemoryPoolIterator(); + } + + public void Write(ArraySegment buffer, bool chunk = false) + { + } + + public Task WriteAsync(ArraySegment buffer, bool chunk = false, CancellationToken cancellationToken = default(CancellationToken)) + { + return Task.FromResult(0); + } + } +}