diff --git a/src/Kestrel.Core/CoreStrings.resx b/src/Kestrel.Core/CoreStrings.resx index 0cef31d07b..477d64597a 100644 --- a/src/Kestrel.Core/CoreStrings.resx +++ b/src/Kestrel.Core/CoreStrings.resx @@ -560,4 +560,10 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l An error occured after the response headers were sent, a reset is being sent. + + Less data received than specified in the Content-Length header. + + + More data received than specified in the Content-Length header. + \ No newline at end of file diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index f5d84bec57..5832379e26 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -73,6 +73,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private Http2Stream _currentHeadersStream; private RequestHeaderParsingState _requestHeaderParsingState; private PseudoHeaderFields _parsedPseudoHeaderFields; + private Http2HeadersFrameFlags _headerFlags; private bool _isMethodConnect; private readonly object _stateLock = new object(); private int _highestOpenedStreamId; @@ -517,12 +518,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 TimeoutControl = this, }); - if ((_incomingFrame.HeadersFlags & Http2HeadersFrameFlags.END_STREAM) == Http2HeadersFrameFlags.END_STREAM) - { - _currentHeadersStream.OnEndStreamReceived(); - } - _currentHeadersStream.Reset(); + _headerFlags = _incomingFrame.HeadersFlags; var endHeaders = (_incomingFrame.HeadersFlags & Http2HeadersFrameFlags.END_HEADERS) == Http2HeadersFrameFlags.END_HEADERS; await DecodeHeadersAsync(application, endHeaders, _incomingFrame.HeadersPayload); @@ -825,6 +822,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorMissingMandatoryPseudoHeaderFields, Http2ErrorCode.PROTOCOL_ERROR); } + // This must be initialized before we offload the request or else we may start processing request body frames without it. + _currentHeadersStream.InputRemaining = _currentHeadersStream.RequestHeaders.ContentLength; + + // This must wait until we've received all of the headers so we can verify the content-length. + if ((_headerFlags & Http2HeadersFrameFlags.END_STREAM) == Http2HeadersFrameFlags.END_STREAM) + { + _currentHeadersStream.OnEndStreamReceived(); + } + _streams[_incomingFrame.StreamId] = _currentHeadersStream; // Must not allow app code to block the connection handling loop. ThreadPool.UnsafeQueueUserWorkItem(state => @@ -840,6 +846,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _currentHeadersStream = null; _requestHeaderParsingState = RequestHeaderParsingState.Ready; _parsedPseudoHeaderFields = PseudoHeaderFields.None; + _headerFlags = Http2HeadersFrameFlags.NONE; _isMethodConnect = false; } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 6319de96b9..464075cbd2 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -48,8 +48,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public int StreamId => _context.StreamId; + public long? InputRemaining { get; internal set; } + public bool RequestBodyStarted { get; private set; } public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived; + private bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted; public override bool IsUpgradableRequest => false; @@ -263,8 +266,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public Task OnDataAsync(Http2Frame dataFrame) { - // TODO: content-length accounting - // Since padding isn't buffered, immediately count padding bytes as read for flow control purposes. if (dataFrame.DataHasPadding) { @@ -288,6 +289,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _inputFlowControl.Advance(payload.Count); + if (IsAborted) + { + // Ignore data frames for aborted streams, but only after counting them for purposes of connection level flow control. + return Task.CompletedTask; + } + + // This check happens after flow control so that when we throw and abort, the byte count is returned to the connection + // level accounting. + if (InputRemaining.HasValue) + { + // https://tools.ietf.org/html/rfc7540#section-8.1.2.6 + if (payload.Count > InputRemaining.Value) + { + throw new Http2StreamErrorException(StreamId, CoreStrings.Http2StreamErrorMoreDataThanLength, Http2ErrorCode.PROTOCOL_ERROR); + } + + InputRemaining -= payload.Count; + } + RequestBodyPipe.Writer.Write(payload); var flushTask = RequestBodyPipe.Writer.FlushAsync(); @@ -306,6 +326,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public void OnEndStreamReceived() { + if (InputRemaining.HasValue) + { + // https://tools.ietf.org/html/rfc7540#section-8.1.2.6 + if (InputRemaining.Value != 0) + { + throw new Http2StreamErrorException(StreamId, CoreStrings.Http2StreamErrorLessDataThanLength, Http2ErrorCode.PROTOCOL_ERROR); + } + } + TryApplyCompletionFlag(StreamCompletionFlags.EndStreamReceived); RequestBodyPipe.Writer.Complete(); diff --git a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs index fddd203d4a..c5de786f55 100644 --- a/src/Kestrel.Core/Properties/CoreStrings.Designer.cs +++ b/src/Kestrel.Core/Properties/CoreStrings.Designer.cs @@ -2072,6 +2072,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core internal static string FormatHttp2StreamErrorAfterHeaders() => GetString("Http2StreamErrorAfterHeaders"); + /// + /// Less data received than specified in the Content-Length header. + /// + internal static string Http2StreamErrorLessDataThanLength + { + get => GetString("Http2StreamErrorLessDataThanLength"); + } + + /// + /// Less data received than specified in the Content-Length header. + /// + internal static string FormatHttp2StreamErrorLessDataThanLength() + => GetString("Http2StreamErrorLessDataThanLength"); + + /// + /// More data received than specified in the Content-Length header. + /// + internal static string Http2StreamErrorMoreDataThanLength + { + get => GetString("Http2StreamErrorMoreDataThanLength"); + } + + /// + /// More data received than specified in the Content-Length header. + /// + internal static string FormatHttp2StreamErrorMoreDataThanLength() + => GetString("Http2StreamErrorMoreDataThanLength"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/test/Kestrel.Core.Tests/Http2StreamTests.cs b/test/Kestrel.Core.Tests/Http2StreamTests.cs index 47c78b0a83..4e2a09c449 100644 --- a/test/Kestrel.Core.Tests/Http2StreamTests.cs +++ b/test/Kestrel.Core.Tests/Http2StreamTests.cs @@ -20,7 +20,6 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; -using Microsoft.Extensions.Logging; using Microsoft.Net.Http.Headers; using Xunit; @@ -43,11 +42,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair("upgrade-insecure-requests", "1"), }; - private readonly MemoryPool _memoryPool = KestrelMemoryPool.Create(); - private readonly DuplexPipe.DuplexPipePair _pair; + private MemoryPool _memoryPool = KestrelMemoryPool.Create(); + private DuplexPipe.DuplexPipePair _pair; private readonly TestApplicationErrorLogger _logger; - private readonly Http2ConnectionContext _connectionContext; - private readonly Http2Connection _connection; + private Http2ConnectionContext _connectionContext; + private Http2Connection _connection; private readonly Http2PeerSettings _clientSettings = new Http2PeerSettings(); private readonly HPackEncoder _hpackEncoder = new HPackEncoder(); private readonly HPackDecoder _hpackDecoder; @@ -69,24 +68,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public Http2StreamTests() { - // Always dispatch test code back to the ThreadPool. This prevents deadlocks caused by continuing - // Http2Connection.ProcessRequestsAsync() loop with writer locks acquired. Run product code inline to make - // it easier to verify request frames are processed correctly immediately after sending the them. - var inputPipeOptions = new PipeOptions( - pool: _memoryPool, - readerScheduler: PipeScheduler.Inline, - writerScheduler: PipeScheduler.ThreadPool, - useSynchronizationContext: false - ); - var outputPipeOptions = new PipeOptions( - pool: _memoryPool, - readerScheduler: PipeScheduler.ThreadPool, - writerScheduler: PipeScheduler.Inline, - useSynchronizationContext: false - ); - - _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); - _noopApplication = context => Task.CompletedTask; _echoMethod = context => @@ -179,6 +160,31 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _logger = new TestApplicationErrorLogger(); + InitializeConnectionFields(KestrelMemoryPool.Create()); + } + + private void InitializeConnectionFields(MemoryPool memoryPool) + { + _memoryPool = memoryPool; + + // Always dispatch test code back to the ThreadPool. This prevents deadlocks caused by continuing + // Http2Connection.ProcessRequestsAsync() loop with writer locks acquired. Run product code inline to make + // it easier to verify request frames are processed correctly immediately after sending the them. + var inputPipeOptions = new PipeOptions( + pool: _memoryPool, + readerScheduler: PipeScheduler.Inline, + writerScheduler: PipeScheduler.ThreadPool, + useSynchronizationContext: false + ); + var outputPipeOptions = new PipeOptions( + pool: _memoryPool, + readerScheduler: PipeScheduler.ThreadPool, + writerScheduler: PipeScheduler.Inline, + useSynchronizationContext: false + ); + + _pair = DuplexPipe.CreateConnectionPair(inputPipeOptions, outputPipeOptions); + _connectionContext = new Http2ConnectionContext { ConnectionFeatures = new FeatureCollection(), @@ -756,21 +762,24 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public async Task ContentLength_Response_FirstWriteMoreBytesWritten_Throws_Sends500() + public async Task ContentLength_Received_SingleDataFrame_Verified() { var headers = new[] { - new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Method, "POST"), new KeyValuePair(HeaderNames.Path, "/"), new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), }; await InitializeConnectionAsync(async context => { - context.Response.ContentLength = 11; - await context.Response.WriteAsync("hello, world"); // 12 + var buffer = new byte[100]; + var read = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length); + Assert.Equal(12, read); }); - await StartStreamAsync(1, headers, endStream: true); + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[12].AsSpan(), endStream: true); var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 55, @@ -781,47 +790,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2DataFrameFlags.END_STREAM, withStreamId: 1); - Assert.Contains(_logger.Messages, m => m.Exception?.Message.Contains("Response Content-Length mismatch: too many bytes written (12 of 11).") ?? false); - - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); - - _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); - - Assert.Equal(3, _decodedHeaders.Count); - Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); - Assert.Equal("500", _decodedHeaders[HeaderNames.Status]); - Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); - } - - [Fact] - public async Task ContentLength_Response_MoreBytesWritten_ThrowsAndResetsStream() - { - var headers = new[] - { - new KeyValuePair(HeaderNames.Method, "GET"), - new KeyValuePair(HeaderNames.Path, "/"), - new KeyValuePair(HeaderNames.Scheme, "http"), - }; - await InitializeConnectionAsync(async context => - { - context.Response.ContentLength = 11; - await context.Response.WriteAsync("hello,"); - await context.Response.WriteAsync(" world"); - }); - - await StartStreamAsync(1, headers, endStream: true); - - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 56, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 6, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - await WaitForStreamErrorAsync(1, Http2ErrorCode.INTERNAL_ERROR, "Response Content-Length mismatch: too many bytes written (12 of 11)."); - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); @@ -829,25 +797,32 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(3, _decodedHeaders.Count); Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); - Assert.Equal("11", _decodedHeaders[HeaderNames.ContentLength]); + Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); } [Fact] - public async Task ContentLength_Response_NoBytesWritten_Sends500() + public async Task ContentLength_ReceivedInContinuation_SingleDataFrame_Verified() { - var headers = new[] + await InitializeConnectionAsync(async context => { - new KeyValuePair(HeaderNames.Method, "GET"), - new KeyValuePair(HeaderNames.Path, "/"), - new KeyValuePair(HeaderNames.Scheme, "http"), - }; - await InitializeConnectionAsync(context => - { - context.Response.ContentLength = 11; - return Task.CompletedTask; + var buffer = new byte[100]; + var read = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length); + Assert.Equal(12, read); }); - await StartStreamAsync(1, headers, endStream: true); + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair("a", _largeHeaderValue), + new KeyValuePair("b", _largeHeaderValue), + new KeyValuePair("c", _largeHeaderValue), + new KeyValuePair("d", _largeHeaderValue), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[12].AsSpan(), endStream: true); var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 55, @@ -858,46 +833,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2DataFrameFlags.END_STREAM, withStreamId: 1); - Assert.Contains(_logger.Messages, m => m.Exception?.Message.Contains("Response Content-Length mismatch: too few bytes written (0 of 11).") ?? false); - - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); - - _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); - - Assert.Equal(3, _decodedHeaders.Count); - Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); - Assert.Equal("500", _decodedHeaders[HeaderNames.Status]); - Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); - } - - [Fact] - public async Task ContentLength_Response_TooFewBytesWritten_Resets() - { - var headers = new[] - { - new KeyValuePair(HeaderNames.Method, "GET"), - new KeyValuePair(HeaderNames.Path, "/"), - new KeyValuePair(HeaderNames.Scheme, "http"), - }; - await InitializeConnectionAsync(context => - { - context.Response.ContentLength = 11; - return context.Response.WriteAsync("hello,"); - }); - - await StartStreamAsync(1, headers, endStream: true); - - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 56, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 6, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - await WaitForStreamErrorAsync(1, Http2ErrorCode.INTERNAL_ERROR, "Response Content-Length mismatch: too few bytes written (6 of 11)."); - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); @@ -905,24 +840,37 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Equal(3, _decodedHeaders.Count); Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); - Assert.Equal("11", _decodedHeaders[HeaderNames.ContentLength]); + Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); } [Fact] - public async Task ApplicationExeption_BeforeFirstWrite_Sends500() + public async Task ContentLength_Received_MultipleDataFrame_Verified() { var headers = new[] { - new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Method, "POST"), new KeyValuePair(HeaderNames.Path, "/"), new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), }; - await InitializeConnectionAsync(context => + await InitializeConnectionAsync(async context => { - throw new Exception("App Faulted"); + var buffer = new byte[100]; + var read = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length); + var total = read; + while (read > 0) + { + read = await context.Request.Body.ReadAsync(buffer, total, buffer.Length - total); + total += read; + } + Assert.Equal(12, total); }); - await StartStreamAsync(1, headers, endStream: true); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[3].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[8].AsSpan(), endStream: true); var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, withLength: 55, @@ -933,53 +881,186 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2DataFrameFlags.END_STREAM, withStreamId: 1); - Assert.Contains(_logger.Messages, m => (m.Exception?.Message.Contains("App Faulted") ?? false) && m.LogLevel == LogLevel.Error); - await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); Assert.Equal(3, _decodedHeaders.Count); Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); - Assert.Equal("500", _decodedHeaders[HeaderNames.Status]); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); } [Fact] - public async Task ApplicationExeption_AfterFirstWrite_Resets() + public async Task ContentLength_Received_NoDataFrames_Reset() { var headers = new[] { - new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Method, "POST"), new KeyValuePair(HeaderNames.Path, "/"), new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), }; - await InitializeConnectionAsync(async context => - { - await context.Response.WriteAsync("hello,"); - throw new Exception("App Faulted"); - }); + await InitializeConnectionAsync(_noopApplication); await StartStreamAsync(1, headers, endStream: true); - var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 37, - withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, - withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 6, - withFlags: (byte)Http2DataFrameFlags.NONE, - withStreamId: 1); - - await WaitForStreamErrorAsync(1, Http2ErrorCode.INTERNAL_ERROR, "App Faulted"); + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorLessDataThanLength); await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } - _hpackDecoder.Decode(headersFrame.HeadersPayload, endHeaders: false, handler: this); + [Fact] + public async Task ContentLength_ReceivedInContinuation_NoDataFrames_Reset() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair("a", _largeHeaderValue), + new KeyValuePair("b", _largeHeaderValue), + new KeyValuePair("c", _largeHeaderValue), + new KeyValuePair("d", _largeHeaderValue), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await InitializeConnectionAsync(_noopApplication); - Assert.Equal(2, _decodedHeaders.Count); - Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); - Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + await StartStreamAsync(1, headers, endStream: true); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorLessDataThanLength); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + + [Fact] + public async Task ContentLength_Received_SingleDataFrameOverSize_Reset() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await InitializeConnectionAsync(async context => + { + await Assert.ThrowsAsync(async () => + { + var buffer = new byte[100]; + while (await context.Request.Body.ReadAsync(buffer, 0, buffer.Length) > 0) { } + }); + }); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[13].AsSpan(), endStream: true); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorMoreDataThanLength); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + + [Fact] + public async Task ContentLength_Received_SingleDataFrameUnderSize_Reset() + { + // I hate doing this, but it avoids exceptions from MemoryPool.Dipose() in debug mode. The problem is since + // the stream's ProcessRequestsAsync loop is never awaited by the connection, it's not really possible to + // observe when all the blocks are returned. This can be removed after we implement graceful shutdown. + Dispose(); + InitializeConnectionFields(new DiagnosticMemoryPool(KestrelMemoryPool.CreateSlabMemoryPool(), allowLateReturn: true)); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await InitializeConnectionAsync(async context => + { + await Assert.ThrowsAsync(async () => + { + var buffer = new byte[100]; + while (await context.Request.Body.ReadAsync(buffer, 0, buffer.Length) > 0) { } + }); + }); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[11].AsSpan(), endStream: true); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorLessDataThanLength); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + + [Fact] + public async Task ContentLength_Received_MultipleDataFramesOverSize_Reset() + { + // I hate doing this, but it avoids exceptions from MemoryPool.Dipose() in debug mode. The problem is since + // the stream's ProcessRequestsAsync loop is never awaited by the connection, it's not really possible to + // observe when all the blocks are returned. This can be removed after we implement graceful shutdown. + Dispose(); + InitializeConnectionFields(new DiagnosticMemoryPool(KestrelMemoryPool.CreateSlabMemoryPool(), allowLateReturn: true)); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await InitializeConnectionAsync(async context => + { + await Assert.ThrowsAsync(async () => + { + var buffer = new byte[100]; + while (await context.Request.Body.ReadAsync(buffer, 0, buffer.Length) > 0) { } + }); + }); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[2].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[10].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[2].AsSpan(), endStream: true); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorMoreDataThanLength); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + + [Fact] + public async Task ContentLength_Received_MultipleDataFramesUnderSize_Reset() + { + // I hate doing this, but it avoids exceptions from MemoryPool.Dipose() in debug mode. The problem is since + // the stream's ProcessRequestsAsync loop is never awaited by the connection, it's not really possible to + // observe when all the blocks are returned. This can be removed after we implement graceful shutdown. + Dispose(); + InitializeConnectionFields(new DiagnosticMemoryPool(KestrelMemoryPool.CreateSlabMemoryPool(), allowLateReturn: true)); + + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "POST"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.ContentLength, "12"), + }; + await InitializeConnectionAsync(async context => + { + await Assert.ThrowsAsync(async () => + { + var buffer = new byte[100]; + while (await context.Request.Body.ReadAsync(buffer, 0, buffer.Length) > 0) { } + }); + }); + + await StartStreamAsync(1, headers, endStream: false); + await SendDataAsync(1, new byte[1].AsSpan(), endStream: false); + await SendDataAsync(1, new byte[2].AsSpan(), endStream: true); + + await WaitForStreamErrorAsync(1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.Http2StreamErrorLessDataThanLength); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } [Fact] @@ -1410,8 +1491,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests if (expectedErrorMessage != null) { - var message = Assert.Single(_logger.Messages, m => m.Exception is ConnectionAbortedException); - Assert.Contains(expectedErrorMessage, message.Exception.Message); + Assert.Contains(_logger.Messages, m => m.Exception?.Message.Contains(expectedErrorMessage) ?? false); } } } diff --git a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs index 239d051200..b5a196c5f2 100644 --- a/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs @@ -3,6 +3,7 @@ #if NETCOREAPP2_2 +using System.IO; using System.Linq; using System.Net; using System.Threading.Tasks; @@ -56,7 +57,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 get { var dataset = new TheoryData(); - var toSkip = new[] { "hpack/4.2/1", "http2/5.1/8", "http2/8.1.2.6/1", "http2/8.1.2.6/2" }; + var toSkip = new[] { "hpack/4.2/1", "http2/5.1/8" }; foreach (var testcase in H2SpecCommands.EnumerateTestCases()) { @@ -123,9 +124,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2 private void ConfigureHelloWorld(IApplicationBuilder app) { - app.Run(context => + app.Run(async context => { - return context.Request.Body.CopyToAsync(context.Response.Body); + // Read the whole request body to check for errors. + await context.Request.Body.CopyToAsync(Stream.Null); + await context.Response.WriteAsync("Hello World"); }); } } diff --git a/test/shared/KestrelTestLoggerProvider.cs b/test/shared/KestrelTestLoggerProvider.cs index 48455557fb..69984f2770 100644 --- a/test/shared/KestrelTestLoggerProvider.cs +++ b/test/shared/KestrelTestLoggerProvider.cs @@ -27,7 +27,6 @@ namespace Microsoft.AspNetCore.Testing public void Dispose() { - throw new NotImplementedException(); } } }