diff --git a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs index 7052bf3a41..acc57a23d6 100644 --- a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs @@ -22,8 +22,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance // Standard completed task private static readonly Func _syncTaskFunc = (obj) => Task.CompletedTask; // Non-standard completed task - private static readonly Task _psuedoAsyncTask = Task.FromResult(27); - private static readonly Func _psuedoAsyncTaskFunc = (obj) => _psuedoAsyncTask; + private static readonly Task _pseudoAsyncTask = Task.FromResult(27); + private static readonly Func _pseudoAsyncTaskFunc = (obj) => _pseudoAsyncTask; private TestHttp1Connection _http1Connection; private DuplexPipe.DuplexPipePair _pair; @@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance _http1Connection.OnStarting(_syncTaskFunc, null); break; case Startup.Async: - _http1Connection.OnStarting(_psuedoAsyncTaskFunc, null); + _http1Connection.OnStarting(_pseudoAsyncTaskFunc, null); break; } } diff --git a/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs b/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs index 9841be5826..0fc02aa7d9 100644 --- a/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs +++ b/benchmarks/Kestrel.Performance/Mocks/MockTrace.cs @@ -43,8 +43,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public void RequestBodyDone(string connectionId, string traceIdentifier) { } public void RequestBodyNotEntirelyRead(string connectionId, string traceIdentifier) { } public void RequestBodyDrainTimedOut(string connectionId, string traceIdentifier) { } - public void RequestBodyMininumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) { } - public void ResponseMininumDataRateNotSatisfied(string connectionId, string traceIdentifier) { } + public void RequestBodyMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) { } + public void ResponseMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier) { } public void ApplicationAbortedConnection(string connectionId, string traceIdentifier) { } public void Http2ConnectionError(string connectionId, Http2ConnectionErrorException ex) { } public void Http2StreamError(string connectionId, Http2StreamErrorException ex) { } diff --git a/benchmarks/Kestrel.Performance/PipeThroughputBenchmark.cs b/benchmarks/Kestrel.Performance/PipeThroughputBenchmark.cs index b647567715..eb0cedad24 100644 --- a/benchmarks/Kestrel.Performance/PipeThroughputBenchmark.cs +++ b/benchmarks/Kestrel.Performance/PipeThroughputBenchmark.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance { public class PipeThroughputBenchmark { - private const int _writeLenght = 57; + private const int _writeLength = 57; private const int InnerLoopCount = 512; private Pipe _pipe; @@ -32,15 +32,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance { for (int i = 0; i < InnerLoopCount; i++) { - _pipe.Writer.GetMemory(_writeLenght); - _pipe.Writer.Advance(_writeLenght); + _pipe.Writer.GetMemory(_writeLength); + _pipe.Writer.Advance(_writeLength); await _pipe.Writer.FlushAsync(); } }); var reading = Task.Run(async () => { - long remaining = InnerLoopCount * _writeLenght; + long remaining = InnerLoopCount * _writeLength; while (remaining != 0) { var result = await _pipe.Reader.ReadAsync(); @@ -57,8 +57,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance { for (int i = 0; i < InnerLoopCount; i++) { - _pipe.Writer.GetMemory(_writeLenght); - _pipe.Writer.Advance(_writeLenght); + _pipe.Writer.GetMemory(_writeLength); + _pipe.Writer.Advance(_writeLength); _pipe.Writer.FlushAsync().GetAwaiter().GetResult(); var result = _pipe.Reader.ReadAsync().GetAwaiter().GetResult(); _pipe.Reader.AdvanceTo(result.Buffer.End, result.Buffer.End); diff --git a/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/DiagnosticMemoryPool.cs b/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/DiagnosticMemoryPool.cs index 3d2c0d72c7..d4d58e0241 100644 --- a/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/DiagnosticMemoryPool.cs +++ b/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/DiagnosticMemoryPool.cs @@ -24,7 +24,7 @@ namespace System.Buffers private readonly List _blockAccessExceptions; - private readonly TaskCompletionSource _allBlocksRetuned; + private readonly TaskCompletionSource _allBlocksReturned; private int _totalBlocks; @@ -40,7 +40,7 @@ namespace System.Buffers _rentTracking = rentTracking; _blocks = new HashSet(); _syncObj = new object(); - _allBlocksRetuned = new TaskCompletionSource(); + _allBlocksReturned = new TaskCompletionSource(); _blockAccessExceptions = new List(); } @@ -138,11 +138,11 @@ namespace System.Buffers { if (_blockAccessExceptions.Any()) { - _allBlocksRetuned.SetException(CreateAccessExceptions()); + _allBlocksReturned.SetException(CreateAccessExceptions()); } else { - _allBlocksRetuned.SetResult(null); + _allBlocksReturned.SetResult(null); } } @@ -153,8 +153,8 @@ namespace System.Buffers public async Task WhenAllBlocksReturnedAsync(TimeSpan timeout) { - var task = await Task.WhenAny(_allBlocksRetuned.Task, Task.Delay(timeout)); - if (task != _allBlocksRetuned.Task) + var task = await Task.WhenAny(_allBlocksReturned.Task, Task.Delay(timeout)); + if (task != _allBlocksReturned.Task) { MemoryPoolThrowHelper.ThrowInvalidOperationException_BlocksWereNotReturnedInTime(_totalBlocks - _blocks.Count, _totalBlocks, _blocks.ToArray()); } diff --git a/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/MemoryPoolSlab.cs b/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/MemoryPoolSlab.cs index d4e5d5ab9a..99dfe082e5 100644 --- a/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/MemoryPoolSlab.cs +++ b/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/MemoryPoolSlab.cs @@ -7,7 +7,7 @@ namespace System.Buffers { /// /// Slab tracking object used by the byte buffer memory pool. A slab is a large allocation which is divided into smaller blocks. The - /// individual blocks are then treated as independant array segments. + /// individual blocks are then treated as independent array segments. /// internal class MemoryPoolSlab : IDisposable { diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index 5a4e1d68bb..c2655344b8 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -356,7 +356,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http else if (_requestTargetForm != HttpRequestTarget.OriginForm) { // Tail call - ValidateNonOrginHostHeader(hostText); + ValidateNonOriginHostHeader(hostText); } else if (!HttpUtilities.IsHostHeaderValid(hostText)) { @@ -364,7 +364,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } - private void ValidateNonOrginHostHeader(string hostText) + private void ValidateNonOriginHostHeader(string hostText) { if (_requestTargetForm == HttpRequestTarget.AuthorityForm) { diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 602df5100a..c0461f9eb8 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -155,6 +155,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } } + catch (OperationCanceledException) + { + // TryRead can throw OperationCanceledException https://github.com/dotnet/corefx/issues/32029 + // beacuse of buggy logic, this works around that for now + } catch (BadHttpRequestException ex) { // At this point, the response has already been written, so this won't result in a 4XX response; @@ -567,8 +572,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var extensionCursor = extensionCursorPosition.Value; var charsToByteCRExclusive = buffer.Slice(0, extensionCursor).Length; - var sufixBuffer = buffer.Slice(extensionCursor); - if (sufixBuffer.Length < 2) + var suffixBuffer = buffer.Slice(extensionCursor); + if (suffixBuffer.Length < 2) { consumed = extensionCursor; examined = buffer.End; @@ -576,16 +581,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return; } - sufixBuffer = sufixBuffer.Slice(0, 2); - var sufixSpan = sufixBuffer.ToSpan(); + suffixBuffer = suffixBuffer.Slice(0, 2); + var sufixSpan = suffixBuffer.ToSpan(); if (sufixSpan[1] == '\n') { // We consumed the \r\n at the end of the extension, so switch modes. _mode = _inputLength > 0 ? Mode.Data : Mode.Trailer; - consumed = sufixBuffer.End; - examined = sufixBuffer.End; + consumed = suffixBuffer.End; + examined = suffixBuffer.End; AddAndCheckConsumedBytes(charsToByteCRExclusive + 2); } else diff --git a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs index 1337b6b38b..5acff95b16 100644 --- a/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http/Http1OutputProducer.cs @@ -73,23 +73,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return WriteAsync(Constants.EmptyData, cancellationToken); } - public void Write(Func callback, T state) - { - lock (_contextLock) - { - if (_completed) - { - return; - } - - var buffer = _pipeWriter; - var bytesCommitted = callback(buffer, state); - _unflushedBytes += bytesCommitted; - _totalBytesCommitted += bytesCommitted; - } - } - - public Task WriteAsync(Func callback, T state) + public Task WriteAsync(Func callback, T state, CancellationToken cancellationToken) { lock (_contextLock) { @@ -104,7 +88,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _totalBytesCommitted += bytesCommitted; } - return FlushAsync(); + return FlushAsync(cancellationToken); } public void WriteResponseHeaders(int statusCode, string reasonPhrase, HttpResponseHeaders responseHeaders) diff --git a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs index b537e44e5f..4a1a474b08 100644 --- a/src/Kestrel.Core/Internal/Http/HttpHeaders.cs +++ b/src/Kestrel.Core/Internal/Http/HttpHeaders.cs @@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } if (string.IsNullOrEmpty(key)) { - ThrowInvalidEmtpyHeaderName(); + ThrowInvalidEmptyHeaderName(); } if (value.Count == 0) { @@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } if (string.IsNullOrEmpty(key)) { - ThrowInvalidEmtpyHeaderName(); + ThrowInvalidEmptyHeaderName(); } if (value.Count > 0 && !AddValueFast(key, value)) @@ -457,7 +457,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http throw new InvalidOperationException(CoreStrings.FormatInvalidAsciiOrControlChar(string.Format("0x{0:X4}", (ushort)ch))); } - private static void ThrowInvalidEmtpyHeaderName() + private static void ThrowInvalidEmptyHeaderName() { throw new InvalidOperationException(CoreStrings.InvalidEmptyHeaderName); } diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index 0c27877c4a..2e80c87bae 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -915,7 +915,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private Task WriteChunkedAsync(ReadOnlyMemory data, CancellationToken cancellationToken) { - return Output.WriteAsync(_writeChunk, data); + return Output.WriteAsync(_writeChunk, data, cancellationToken); } private static long WriteChunk(PipeWriter writableBuffer, ReadOnlyMemory buffer) diff --git a/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs b/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs index 6dbdaca7f4..25f3af7012 100644 --- a/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http/IHttpOutputProducer.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public interface IHttpOutputProducer : IDisposable { void Abort(ConnectionAbortedException abortReason); - Task WriteAsync(Func callback, T state); + Task WriteAsync(Func callback, T state, CancellationToken cancellationToken); Task FlushAsync(CancellationToken cancellationToken); Task Write100ContinueAsync(); void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders); diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index ed0bab633b..2ef88d5f33 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -43,7 +43,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(); + var result = await _context.RequestBodyPipe.Reader.ReadAsync(cancellationToken); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; var actual = 0; @@ -83,7 +83,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(); + var result = await _context.RequestBodyPipe.Reader.ReadAsync(cancellationToken); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; var bytesRead = 0; @@ -101,7 +101,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http bytesRead += memory.Length; #if NETCOREAPP2_1 - await destination.WriteAsync(memory); + await destination.WriteAsync(memory, cancellationToken); #elif NETSTANDARD2_0 var array = memory.GetArray(); await destination.WriteAsync(array.Array, array.Offset, array.Count, cancellationToken); diff --git a/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs b/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs index e56c43b23c..c379fa257e 100644 --- a/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs +++ b/src/Kestrel.Core/Internal/Http/PipelineExtensions.cs @@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http const int Shift16Shift24 = (1 << 16) | (1 << 24); const int Shift8Identity = (1 << 8) | (1); - // Encode as bytes upto the first non-ASCII byte and return count encoded + // Encode as bytes up to the first non-ASCII byte and return count encoded int i = 0; // Use Intrinsic switch if (IntPtr.Size == 8) // 64 bit diff --git a/src/Kestrel.Core/Internal/Http2/Bitshifter.cs b/src/Kestrel.Core/Internal/Http2/Bitshifter.cs index 7c3915203f..6dea868291 100644 --- a/src/Kestrel.Core/Internal/Http2/Bitshifter.cs +++ b/src/Kestrel.Core/Internal/Http2/Bitshifter.cs @@ -8,7 +8,7 @@ using System.Runtime.CompilerServices; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { - // Mimics BinaryPrimities with oddly sized units + // Mimics BinaryPrimitives with oddly sized units internal class Bitshifter { [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/Kestrel.Core/Internal/Http2/FlowControl/StreamInputFlowControl.cs b/src/Kestrel.Core/Internal/Http2/FlowControl/StreamInputFlowControl.cs index e85b2bbe2e..4ee1d0238c 100644 --- a/src/Kestrel.Core/Internal/Http2/FlowControl/StreamInputFlowControl.cs +++ b/src/Kestrel.Core/Internal/Http2/FlowControl/StreamInputFlowControl.cs @@ -29,9 +29,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl public void Advance(int bytes) { - var connectionSucess = _connectionLevelFlowControl.TryAdvance(bytes); + var connectionSuccess = _connectionLevelFlowControl.TryAdvance(bytes); - Debug.Assert(connectionSucess, "Connection-level input flow control should never be aborted."); + Debug.Assert(connectionSuccess, "Connection-level input flow control should never be aborted."); if (!_streamLevelFlowControl.TryAdvance(bytes)) { @@ -78,9 +78,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.FlowControl private void UpdateConnectionWindow(int bytes) { - var connectionSucess = _connectionLevelFlowControl.TryUpdateWindow(bytes, out var connectionWindowUpdateSize); + var connectionSuccess = _connectionLevelFlowControl.TryUpdateWindow(bytes, out var connectionWindowUpdateSize); - Debug.Assert(connectionSucess, "Connection-level input flow control should never be aborted."); + Debug.Assert(connectionSuccess, "Connection-level input flow control should never be aborted."); if (connectionWindowUpdateSize > 0) { diff --git a/src/Kestrel.Core/Internal/Http2/HPack/HeaderField.cs b/src/Kestrel.Core/Internal/Http2/HPack/HeaderField.cs index 73eb4d726e..fd224f4e52 100644 --- a/src/Kestrel.Core/Internal/Http2/HPack/HeaderField.cs +++ b/src/Kestrel.Core/Internal/Http2/HPack/HeaderField.cs @@ -25,6 +25,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack public int Length => GetLength(Name.Length, Value.Length); - public static int GetLength(int nameLength, int valueLenth) => nameLength + valueLenth + 32; + public static int GetLength(int nameLength, int valueLength) => nameLength + valueLength + 32; } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs index c33674d645..5b051ff224 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Connection.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Connection.cs @@ -75,6 +75,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private RequestHeaderParsingState _requestHeaderParsingState; private PseudoHeaderFields _parsedPseudoHeaderFields; private Http2HeadersFrameFlags _headerFlags; + private int _totalParsedHeaderSize; private bool _isMethodConnect; private readonly object _stateLock = new object(); private int _highestOpenedStreamId; @@ -92,6 +93,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _serverSettings.HeaderTableSize = (uint)context.ServiceContext.ServerOptions.Limits.Http2.HeaderTableSize; _hpackDecoder = new HPackDecoder((int)_serverSettings.HeaderTableSize); _incomingFrame = new Http2Frame(_serverSettings.MaxFrameSize); + _serverSettings.MaxHeaderListSize = (uint)context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize; } public string ConnectionId => _context.ConnectionId; @@ -888,6 +890,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _parsedPseudoHeaderFields = PseudoHeaderFields.None; _headerFlags = Http2HeadersFrameFlags.NONE; _isMethodConnect = false; + _totalParsedHeaderSize = 0; } private void ThrowIfIncomingFrameSentToIdleStream() @@ -934,7 +937,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 Input.CancelPendingRead(); } - if (_state != Http2ConnectionState.Open) { // Complete the task waiting on all streams to finish @@ -944,15 +946,57 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } + // We can't throw a Http2StreamErrorException here, it interrupts the header decompression state and may corrupt subsequent header frames on other streams. + // For now these either need to be connection errors or BadRequests. If we want to downgrade any of them to stream errors later then we need to + // rework the flow so that the remaining headers are drained and the decompression state is maintained. public void OnHeader(Span name, Span value) { + // https://tools.ietf.org/html/rfc7540#section-6.5.2 + // "The value is based on the uncompressed size of header fields, including the length of the name and value in octets plus an overhead of 32 octets for each header field."; + _totalParsedHeaderSize += HeaderField.RfcOverhead + name.Length + value.Length; + if (_totalParsedHeaderSize > _context.ServiceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize) + { + throw new Http2ConnectionErrorException(CoreStrings.BadRequest_HeadersExceedMaxTotalSize, Http2ErrorCode.PROTOCOL_ERROR); + } + ValidateHeader(name, value); - _currentHeadersStream.OnHeader(name, value); + try + { + // Drop trailers for now. Adding them to the request headers is not thread safe. + // https://github.com/aspnet/KestrelHttpServer/issues/2051 + if (_requestHeaderParsingState != RequestHeaderParsingState.Trailers) + { + // Throws BadReqeust for header count limit breaches. + // Throws InvalidOperation for bad encoding. + _currentHeadersStream.OnHeader(name, value); + } + } + catch (BadHttpRequestException bre) + { + throw new Http2ConnectionErrorException(bre.Message, Http2ErrorCode.PROTOCOL_ERROR); + } + catch (InvalidOperationException) + { + throw new Http2ConnectionErrorException(CoreStrings.BadRequest_MalformedRequestInvalidHeaders, Http2ErrorCode.PROTOCOL_ERROR); + } } private void ValidateHeader(Span name, Span value) { // http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.1 + /* + Intermediaries that process HTTP requests or responses (i.e., any + intermediary not acting as a tunnel) MUST NOT forward a malformed + request or response. Malformed requests or responses that are + detected MUST be treated as a stream error (Section 5.4.2) of type + PROTOCOL_ERROR. + + For malformed requests, a server MAY send an HTTP response prior to + closing or resetting the stream. Clients MUST NOT accept a malformed + response. Note that these requirements are intended to protect + against several types of common attacks against HTTP; they are + deliberately strict because being permissive can expose + implementations to these vulnerabilities.*/ if (IsPseudoHeaderField(name, out var headerField)) { if (_requestHeaderParsingState == RequestHeaderParsingState.Headers) @@ -960,7 +1004,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 // All pseudo-header fields MUST appear in the header block before regular header fields. // Any request or response that contains a pseudo-header field that appears in a header // block after a regular header field MUST be treated as malformed (Section 8.1.2.6). - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorPseudoHeaderFieldAfterRegularHeaders, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorPseudoHeaderFieldAfterRegularHeaders, Http2ErrorCode.PROTOCOL_ERROR); } if (_requestHeaderParsingState == RequestHeaderParsingState.Trailers) @@ -975,21 +1019,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { // Endpoints MUST treat a request or response that contains undefined or invalid pseudo-header // fields as malformed (Section 8.1.2.6). - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorUnknownPseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorUnknownPseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); } if (headerField == PseudoHeaderFields.Status) { // Pseudo-header fields defined for requests MUST NOT appear in responses; pseudo-header fields // defined for responses MUST NOT appear in requests. - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorResponsePseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorResponsePseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); } if ((_parsedPseudoHeaderFields & headerField) == headerField) { // http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2.3 // All HTTP/2 requests MUST include exactly one valid value for the :method, :scheme, and :path pseudo-header fields - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorDuplicatePseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorDuplicatePseudoHeaderField, Http2ErrorCode.PROTOCOL_ERROR); } if (headerField == PseudoHeaderFields.Method) @@ -1006,7 +1050,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 if (IsConnectionSpecificHeaderField(name, value)) { - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorConnectionSpecificHeaderField, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorConnectionSpecificHeaderField, Http2ErrorCode.PROTOCOL_ERROR); } // http://httpwg.org/specs/rfc7540.html#rfc.section.8.1.2 @@ -1021,7 +1065,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } else { - throw new Http2StreamErrorException(_currentHeadersStream.StreamId, CoreStrings.Http2ErrorHeaderNameUppercase, Http2ErrorCode.PROTOCOL_ERROR); + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorHeaderNameUppercase, Http2ErrorCode.PROTOCOL_ERROR); } } } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs b/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs index 3039ec1b76..ceddf8089d 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Frame.Settings.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 */ public partial class Http2Frame { - private const int SettingSize = 6; // 2 bytes for the id, 4 bytes for the value. + internal const int SettingSize = 6; // 2 bytes for the id, 4 bytes for the value. public Http2SettingsFrameFlags SettingsFlags { diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index 1a9f88b4c7..405b339436 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -77,7 +77,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 Dispose(); } - public Task WriteAsync(Func callback, T state) + public Task WriteAsync(Func callback, T state, CancellationToken cancellationToken) { throw new NotImplementedException(); } diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index 82935a343b..64e32d560e 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -160,6 +160,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return true; } + // Approximate MaxRequestLineSize by totaling the required pseudo header field lengths. + var requestLineLength = _methodText.Length + Scheme.Length + hostText.Length + path.Length; + if (requestLineLength > ServiceContext.ServerOptions.Limits.MaxRequestLineSize) + { + ResetAndAbort(new ConnectionAbortedException(CoreStrings.BadRequest_RequestLineTooLong), Http2ErrorCode.PROTOCOL_ERROR); + return false; + } + var queryIndex = path.IndexOf('?'); QueryString = queryIndex == -1 ? string.Empty : path.Substring(queryIndex); diff --git a/src/Kestrel.Core/Internal/HttpConnection.cs b/src/Kestrel.Core/Internal/HttpConnection.cs index d860d36ba7..9fc64acb68 100644 --- a/src/Kestrel.Core/Internal/HttpConnection.cs +++ b/src/Kestrel.Core/Internal/HttpConnection.cs @@ -465,7 +465,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal if (rate < minRequestBodyDataRate.BytesPerSecond && !Debugger.IsAttached) { - Log.RequestBodyMininumDataRateNotSatisfied(_context.ConnectionId, _http1Connection.TraceIdentifier, minRequestBodyDataRate.BytesPerSecond); + Log.RequestBodyMinimumDataRateNotSatisfied(_context.ConnectionId, _http1Connection.TraceIdentifier, minRequestBodyDataRate.BytesPerSecond); RequestTimedOut = true; _http1Connection.SendTimeoutResponse(); } @@ -492,7 +492,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal if (_writeTimingWrites > 0 && timestamp > _writeTimingTimeoutTimestamp && !Debugger.IsAttached) { RequestTimedOut = true; - Log.ResponseMininumDataRateNotSatisfied(_http1Connection.ConnectionIdFeature, _http1Connection.TraceIdentifier); + Log.ResponseMinimumDataRateNotSatisfied(_http1Connection.ConnectionIdFeature, _http1Connection.TraceIdentifier); Abort(new ConnectionAbortedException(CoreStrings.ConnectionTimedBecauseResponseMininumDataRateNotSatisfied)); } } diff --git a/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs b/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs index 9d28cd3804..19ecc86ee2 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs @@ -49,9 +49,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure void RequestBodyDrainTimedOut(string connectionId, string traceIdentifier); - void RequestBodyMininumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate); + void RequestBodyMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate); - void ResponseMininumDataRateNotSatisfied(string connectionId, string traceIdentifier); + void ResponseMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier); void ApplicationAbortedConnection(string connectionId, string traceIdentifier); diff --git a/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs b/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs index 3c269b0ac9..f735370d3e 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs @@ -64,10 +64,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal LoggerMessage.Define(LogLevel.Debug, new EventId(26, nameof(RequestBodyDone)), @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": done reading request body."); private static readonly Action _requestBodyMinimumDataRateNotSatisfied = - LoggerMessage.Define(LogLevel.Information, new EventId(27, nameof(RequestBodyMininumDataRateNotSatisfied)), @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": the request timed out because it was not sent by the client at a minimum of {Rate} bytes/second."); + LoggerMessage.Define(LogLevel.Information, new EventId(27, nameof(RequestBodyMinimumDataRateNotSatisfied)), @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": the request timed out because it was not sent by the client at a minimum of {Rate} bytes/second."); private static readonly Action _responseMinimumDataRateNotSatisfied = - LoggerMessage.Define(LogLevel.Information, new EventId(28, nameof(ResponseMininumDataRateNotSatisfied)), @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": the connection was closed because the response was not read by the client at the specified minimum data rate."); + LoggerMessage.Define(LogLevel.Information, new EventId(28, nameof(ResponseMinimumDataRateNotSatisfied)), @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": the connection was closed because the response was not read by the client at the specified minimum data rate."); private static readonly Action _http2ConnectionError = LoggerMessage.Define(LogLevel.Information, new EventId(29, nameof(Http2ConnectionError)), @"Connection id ""{ConnectionId}"": HTTP/2 connection error."); @@ -199,7 +199,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _requestBodyDone(_logger, connectionId, traceIdentifier, null); } - public virtual void RequestBodyMininumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) + public virtual void RequestBodyMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) { _requestBodyMinimumDataRateNotSatisfied(_logger, connectionId, traceIdentifier, rate, null); } @@ -214,7 +214,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal _requestBodyDrainTimedOut(_logger, connectionId, traceIdentifier, null); } - public virtual void ResponseMininumDataRateNotSatisfied(string connectionId, string traceIdentifier) + public virtual void ResponseMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier) { _responseMinimumDataRateNotSatisfied(_logger, connectionId, traceIdentifier, null); } diff --git a/src/Kestrel.Core/KestrelServerLimits.cs b/src/Kestrel.Core/KestrelServerLimits.cs index e7ddc479df..10db0fe662 100644 --- a/src/Kestrel.Core/KestrelServerLimits.cs +++ b/src/Kestrel.Core/KestrelServerLimits.cs @@ -88,6 +88,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core /// Defaults to 8,192 bytes (8 KB). /// /// + /// For HTTP/2 this measures the total size of the required pseudo headers + /// :method, :scheme, :authority, and :path. /// public int MaxRequestLineSize { diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index 1280e0c204..663f7bfbd5 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal (handle, status, state) => ReadCallback(handle, status, state); private static readonly Func _allocCallback = - (handle, suggestedsize, state) => AllocCallback(handle, suggestedsize, state); + (handle, suggestedSize, state) => AllocCallback(handle, suggestedSize, state); private readonly UvStreamHandle _socket; private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource(); diff --git a/src/Kestrel.Transport.Libuv/Internal/Listener.cs b/src/Kestrel.Transport.Libuv/Internal/Listener.cs index 5a89ebe545..e2c392d02d 100644 --- a/src/Kestrel.Transport.Libuv/Internal/Listener.cs +++ b/src/Kestrel.Transport.Libuv/Internal/Listener.cs @@ -187,7 +187,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal protected virtual void DispatchConnection(UvStreamHandle socket) { // REVIEW: This task should be tracked by the server for graceful shutdown - // Today it's handled specifically for http but not for aribitrary middleware + // Today it's handled specifically for http but not for arbitrary middleware _ = HandleConnectionAsync(socket); } diff --git a/src/Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs b/src/Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs index 2ef1ec2a12..3547047add 100644 --- a/src/Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs +++ b/src/Kestrel.Transport.Libuv/Internal/ListenerSecondary.cs @@ -153,7 +153,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal DispatchPipe.Accept(acceptSocket); // REVIEW: This task should be tracked by the server for graceful shutdown - // Today it's handled specifically for http but not for aribitrary middleware + // Today it's handled specifically for http but not for arbitrary middleware _ = HandleConnectionAsync(acceptSocket); } catch (UvException ex) when (LibuvConstants.IsConnectionReset(ex.StatusCode)) diff --git a/src/Kestrel.Transport.Libuv/Internal/Networking/SockAddr.cs b/src/Kestrel.Transport.Libuv/Internal/Networking/SockAddr.cs index 5386749623..96fa624bcd 100644 --- a/src/Kestrel.Transport.Libuv/Internal/Networking/SockAddr.cs +++ b/src/Kestrel.Transport.Libuv/Internal/Networking/SockAddr.cs @@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin int family = (int)_field0; if (PlatformApis.IsDarwin) { - // see explaination in example 4 + // see explanation in example 4 family = family >> 8; } family = family & 0xFF; diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs index 8d0f1e418b..2ade6c44a6 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionTests.cs @@ -82,7 +82,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Debugger = mockDebugger.Object; var bytesPerSecond = 100; var mockLogger = new Mock(); - mockLogger.Setup(l => l.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny())).Throws(new InvalidOperationException("Should not log")); + mockLogger.Setup(l => l.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny())).Throws(new InvalidOperationException("Should not log")); TickBodyWithMinimumDataRate(mockLogger.Object, bytesPerSecond); @@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Timed out Assert.True(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } private void TickBodyWithMinimumDataRate(IKestrelTrace logger, int bytesPerSecond) @@ -155,7 +155,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); // Tick after grace period w/ low data rate now += TimeSpan.FromSeconds(2); @@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Timed out Assert.True(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } [Fact] @@ -202,7 +202,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); // Data rate: 150 bytes/second now += TimeSpan.FromSeconds(1); @@ -212,7 +212,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); // Data rate: 120 bytes/second now += TimeSpan.FromSeconds(1); @@ -222,7 +222,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); // Data rate: 100 bytes/second now += TimeSpan.FromSeconds(1); @@ -232,7 +232,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Never); // Data rate: ~85 bytes/second now += TimeSpan.FromSeconds(1); @@ -242,7 +242,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Timed out Assert.True(_httpConnection.RequestTimedOut); mockLogger.Verify(logger => - logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); + logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), bytesPerSecond), Times.Once); } [Fact] @@ -285,7 +285,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); // Resume at 6.5s @@ -299,7 +299,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); // Tick at 10s, expected counted time is 7s, expected data rate drops below 100 bytes/second @@ -309,7 +309,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Timed out Assert.True(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } @@ -341,7 +341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); // Pause at 2.25s @@ -360,7 +360,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Not timed out Assert.False(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); // Tick at 4s, expected counted time is 4s, expected data rate drops below 100 bytes/second @@ -370,7 +370,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests // Timed out Assert.True(_httpConnection.RequestTimedOut); mockLogger.Verify( - logger => logger.RequestBodyMininumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), + logger => logger.RequestBodyMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } @@ -539,7 +539,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public async Task WriteTimingAbortsConnectionWhenRepeadtedSmallWritesDoNotCompleteWithMinimumDataRate() + public async Task WriteTimingAbortsConnectionWhenRepeatedSmallWritesDoNotCompleteWithMinimumDataRate() { var systemClock = new MockSystemClock(); var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); diff --git a/test/Kestrel.Core.Tests/HttpParserTests.cs b/test/Kestrel.Core.Tests/HttpParserTests.cs index f40c25669b..825ee085de 100644 --- a/test/Kestrel.Core.Tests/HttpParserTests.cs +++ b/test/Kestrel.Core.Tests/HttpParserTests.cs @@ -494,7 +494,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } } - // Doesn't put empty blocks inbetween every byte + // Doesn't put empty blocks in between every byte internal class BytePerSegmentTestSequenceFactory : ReadOnlySequenceFactory { public static ReadOnlySequenceFactory Instance { get; } = new HttpParserTests.BytePerSegmentTestSequenceFactory(); diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index 870e89c47b..f52d8622e1 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -562,7 +562,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await body.StopAsync(); - // Add some more data. Checking for cancelation and exiting the loop + // Add some more data. Checking for cancellation and exiting the loop // should take priority over reading this data. input.Add("b"); diff --git a/test/Kestrel.Core.Tests/OutputProducerTests.cs b/test/Kestrel.Core.Tests/OutputProducerTests.cs index 7f3d566ff5..22102de0be 100644 --- a/test/Kestrel.Core.Tests/OutputProducerTests.cs +++ b/test/Kestrel.Core.Tests/OutputProducerTests.cs @@ -5,6 +5,7 @@ using System; using System.Buffers; using System.IO.Pipelines; using System.Threading; +using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -31,7 +32,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public void WritesNoopAfterConnectionCloses() + public async Task WritesNoopAfterConnectionCloses() { var pipeOptions = new PipeOptions ( @@ -48,12 +49,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var called = false; - socketOutput.Write((buffer, state) => + await socketOutput.WriteAsync((buffer, state) => { called = true; return 0; }, - 0); + 0, + default); Assert.False(called); } diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 0d599b75ca..390085f9f2 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -51,10 +51,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair(HeaderNames.Path, "/"), new KeyValuePair(HeaderNames.Scheme, "http"), new KeyValuePair(HeaderNames.Authority, "localhost:80"), - new KeyValuePair("a", _largeHeaderValue), - new KeyValuePair("b", _largeHeaderValue), - new KeyValuePair("c", _largeHeaderValue), - new KeyValuePair("d", _largeHeaderValue) + new KeyValuePair("a", _4kHeaderValue), + new KeyValuePair("b", _4kHeaderValue), + new KeyValuePair("c", _4kHeaderValue), + new KeyValuePair("d", _4kHeaderValue) }; private static readonly IEnumerable> _twoContinuationsRequestHeaders = new[] @@ -63,14 +63,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair(HeaderNames.Path, "/"), new KeyValuePair(HeaderNames.Scheme, "http"), new KeyValuePair(HeaderNames.Authority, "localhost:80"), - new KeyValuePair("a", _largeHeaderValue), - new KeyValuePair("b", _largeHeaderValue), - new KeyValuePair("c", _largeHeaderValue), - new KeyValuePair("d", _largeHeaderValue), - new KeyValuePair("e", _largeHeaderValue), - new KeyValuePair("f", _largeHeaderValue), - new KeyValuePair("g", _largeHeaderValue), - new KeyValuePair("h", _largeHeaderValue) + new KeyValuePair("a", _4kHeaderValue), + new KeyValuePair("b", _4kHeaderValue), + new KeyValuePair("c", _4kHeaderValue), + new KeyValuePair("d", _4kHeaderValue), + new KeyValuePair("e", _4kHeaderValue), + new KeyValuePair("f", _4kHeaderValue), + new KeyValuePair("g", _4kHeaderValue), }; private static readonly byte[] _helloBytes = Encoding.ASCII.GetBytes("hello"); @@ -102,7 +101,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _connectionContext.ServiceContext.ServerOptions.Limits.Http2.MaxFrameSize = length; _connection = new Http2Connection(_connectionContext); - await InitializeConnectionAsync(_echoApplication, expectedSettingsLegnth: 12); + await InitializeConnectionAsync(_echoApplication, expectedSettingsCount: 3); await StartStreamAsync(1, _browserRequestHeaders, endStream: false); await SendDataAsync(1, new byte[length].AsSpan(), endStream: true); @@ -1067,7 +1066,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Theory] [InlineData(true)] [InlineData(false)] - public async Task HEADERS_Received_WithTrailers_Decoded(bool sendData) + public async Task HEADERS_Received_WithTrailers_Discarded(bool sendData) { await InitializeConnectionAsync(_readTrailersApplication); @@ -1105,7 +1104,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2DataFrameFlags.END_STREAM, withStreamId: 1); - VerifyDecodedRequestHeaders(_browserRequestHeaders.Concat(_requestTrailers)); + VerifyDecodedRequestHeaders(_browserRequestHeaders); + + // Make sure the trailers are missing. https://github.com/aspnet/KestrelHttpServer/issues/2630 + foreach (var header in _requestTrailers) + { + Assert.False(_receivedHeaders.ContainsKey(header.Key)); + } await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); } @@ -1454,27 +1459,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Theory] [MemberData(nameof(UpperCaseHeaderNameData))] - public async Task HEADERS_Received_HeaderNameContainsUpperCaseCharacter_StreamError(byte[] headerBlock) + public async Task HEADERS_Received_HeaderNameContainsUpperCaseCharacter_ConnectionError(byte[] headerBlock) { await InitializeConnectionAsync(_noopApplication); await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS, headerBlock); - await WaitForStreamErrorAsync( - expectedStreamId: 1, - expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, - expectedErrorMessage: CoreStrings.Http2ErrorHeaderNameUppercase); - - // Verify that the stream ID can't be re-used - await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS, _browserRequestHeaders); await WaitForConnectionErrorAsync( ignoreNonGoAwayFrames: false, expectedLastStreamId: 1, - expectedErrorCode: Http2ErrorCode.STREAM_CLOSED, - expectedErrorMessage: CoreStrings.FormatHttp2ErrorStreamClosed(Http2FrameType.HEADERS, streamId: 1)); + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.Http2ErrorHeaderNameUppercase); } [Fact] - public Task HEADERS_Received_HeaderBlockContainsUnknownPseudoHeaderField_StreamError() + public Task HEADERS_Received_HeaderBlockContainsUnknownPseudoHeaderField_ConnectionError() { var headers = new[] { @@ -1484,11 +1482,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair(":unknown", "0"), }; - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, expectedErrorMessage: CoreStrings.Http2ErrorUnknownPseudoHeaderField); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, expectedErrorMessage: CoreStrings.Http2ErrorUnknownPseudoHeaderField); } [Fact] - public Task HEADERS_Received_HeaderBlockContainsResponsePseudoHeaderField_StreamError() + public Task HEADERS_Received_HeaderBlockContainsResponsePseudoHeaderField_ConnectionError() { var headers = new[] { @@ -1498,21 +1496,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair(HeaderNames.Status, "200"), }; - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, expectedErrorMessage: CoreStrings.Http2ErrorResponsePseudoHeaderField); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, expectedErrorMessage: CoreStrings.Http2ErrorResponsePseudoHeaderField); } [Theory] [MemberData(nameof(DuplicatePseudoHeaderFieldData))] - public Task HEADERS_Received_HeaderBlockContainsDuplicatePseudoHeaderField_StreamError(IEnumerable> headers) + public Task HEADERS_Received_HeaderBlockContainsDuplicatePseudoHeaderField_ConnectionError(IEnumerable> headers) { - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, expectedErrorMessage: CoreStrings.Http2ErrorDuplicatePseudoHeaderField); - } - - [Theory] - [MemberData(nameof(MissingPseudoHeaderFieldData))] - public Task HEADERS_Received_HeaderBlockDoesNotContainMandatoryPseudoHeaderField_StreamError(IEnumerable> headers) - { - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, expectedErrorMessage: CoreStrings.Http2ErrorMissingMandatoryPseudoHeaderFields); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, expectedErrorMessage: CoreStrings.Http2ErrorDuplicatePseudoHeaderField); } [Theory] @@ -1537,20 +1528,33 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Theory] [MemberData(nameof(PseudoHeaderFieldAfterRegularHeadersData))] - public Task HEADERS_Received_HeaderBlockContainsPseudoHeaderFieldAfterRegularHeaders_StreamError(IEnumerable> headers) + public Task HEADERS_Received_HeaderBlockContainsPseudoHeaderFieldAfterRegularHeaders_ConnectionError(IEnumerable> headers) { - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, expectedErrorMessage: CoreStrings.Http2ErrorPseudoHeaderFieldAfterRegularHeaders); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, expectedErrorMessage: CoreStrings.Http2ErrorPseudoHeaderFieldAfterRegularHeaders); } - private async Task HEADERS_Received_InvalidHeaderFields_StreamError(IEnumerable> headers, string expectedErrorMessage) + private async Task HEADERS_Received_InvalidHeaderFields_ConnectionError(IEnumerable> headers, string expectedErrorMessage) + { + await InitializeConnectionAsync(_noopApplication); + await StartStreamAsync(1, headers, endStream: true); + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 1, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: expectedErrorMessage); + } + + [Theory] + [MemberData(nameof(MissingPseudoHeaderFieldData))] + public async Task HEADERS_Received_HeaderBlockDoesNotContainMandatoryPseudoHeaderField_StreamError(IEnumerable> headers) { await InitializeConnectionAsync(_noopApplication); await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS, headers); await WaitForStreamErrorAsync( - expectedStreamId: 1, - expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, - expectedErrorMessage: expectedErrorMessage); + expectedStreamId: 1, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.Http2ErrorMissingMandatoryPseudoHeaderFields); // Verify that the stream ID can't be re-used await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS, _browserRequestHeaders); @@ -1562,7 +1566,62 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public Task HEADERS_Received_HeaderBlockContainsConnectionHeader_StreamError() + public Task HEADERS_Received_HeaderBlockOverLimit_ConnectionError() + { + // > 32kb + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair("a", _4kHeaderValue), + new KeyValuePair("b", _4kHeaderValue), + new KeyValuePair("c", _4kHeaderValue), + new KeyValuePair("d", _4kHeaderValue), + new KeyValuePair("e", _4kHeaderValue), + new KeyValuePair("f", _4kHeaderValue), + new KeyValuePair("g", _4kHeaderValue), + new KeyValuePair("h", _4kHeaderValue), + }; + + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.BadRequest_HeadersExceedMaxTotalSize); + } + + [Fact] + public Task HEADERS_Received_TooManyHeaders_ConnectionError() + { + // > MaxRequestHeaderCount (100) + var headers = new List>(); + headers.AddRange(new [] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }); + for (var i = 0; i < 100; i++) + { + headers.Add(new KeyValuePair(i.ToString(), i.ToString())); + } + + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.BadRequest_TooManyHeaders); + } + + [Fact] + public Task HEADERS_Received_InvalidCharacters_ConnectionError() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair("Custom", "val\0ue"), + }; + + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.BadRequest_MalformedRequestInvalidHeaders); + } + + [Fact] + public Task HEADERS_Received_HeaderBlockContainsConnectionHeader_ConnectionError() { var headers = new[] { @@ -1572,11 +1631,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair("connection", "keep-alive") }; - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, CoreStrings.Http2ErrorConnectionSpecificHeaderField); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.Http2ErrorConnectionSpecificHeaderField); } [Fact] - public Task HEADERS_Received_HeaderBlockContainsTEHeader_ValueIsNotTrailers_StreamError() + public Task HEADERS_Received_HeaderBlockContainsTEHeader_ValueIsNotTrailers_ConnectionError() { var headers = new[] { @@ -1586,7 +1645,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests new KeyValuePair("te", "trailers, deflate") }; - return HEADERS_Received_InvalidHeaderFields_StreamError(headers, CoreStrings.Http2ErrorConnectionSpecificHeaderField); + return HEADERS_Received_InvalidHeaderFields_ConnectionError(headers, CoreStrings.Http2ErrorConnectionSpecificHeaderField); } [Fact] @@ -2005,18 +2064,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await SendSettingsAsync(); var frame = await ExpectAsync(Http2FrameType.SETTINGS, - withLength: 6, + withLength: Http2Frame.SettingSize * 2, withFlags: 0, withStreamId: 0); // Only non protocol defaults are sent - Assert.Equal(1, frame.SettingsCount); + Assert.Equal(2, frame.SettingsCount); var settings = frame.GetSettings(); - Assert.Equal(1, settings.Count); + Assert.Equal(2, settings.Count); + var setting = settings[0]; Assert.Equal(Http2SettingsParameter.SETTINGS_MAX_CONCURRENT_STREAMS, setting.Parameter); Assert.Equal(100u, setting.Value); + setting = settings[1]; + Assert.Equal(Http2SettingsParameter.SETTINGS_MAX_HEADER_LIST_SIZE, setting.Parameter); + Assert.Equal(32 * 1024u, setting.Value); + await ExpectAsync(Http2FrameType.SETTINGS, withLength: 0, withFlags: (byte)Http2SettingsFrameFlags.ACK, @@ -2029,6 +2093,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public async Task SETTINGS_Custom_Sent() { _connection.ServerSettings.MaxConcurrentStreams = 1; + _connection.ServerSettings.MaxHeaderListSize = 4 * 1024; _connectionTask = _connection.ProcessRequestsAsync(new DummyApplication(_noopApplication)); @@ -2036,18 +2101,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await SendSettingsAsync(); var frame = await ExpectAsync(Http2FrameType.SETTINGS, - withLength: 6, + withLength: 6 * 2, withFlags: 0, withStreamId: 0); // Only non protocol defaults are sent - Assert.Equal(1, frame.SettingsCount); + Assert.Equal(2, frame.SettingsCount); var settings = frame.GetSettings(); - Assert.Equal(1, settings.Count); + Assert.Equal(2, settings.Count); + var setting = settings[0]; Assert.Equal(Http2SettingsParameter.SETTINGS_MAX_CONCURRENT_STREAMS, setting.Parameter); Assert.Equal(1u, setting.Value); + setting = settings[1]; + Assert.Equal(Http2SettingsParameter.SETTINGS_MAX_HEADER_LIST_SIZE, setting.Parameter); + Assert.Equal(4 * 1024u, setting.Value); + await ExpectAsync(Http2FrameType.SETTINGS, withLength: 0, withFlags: (byte)Http2SettingsFrameFlags.ACK, @@ -2945,7 +3015,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Theory] [InlineData(true)] [InlineData(false)] - public async Task CONTINUATION_Received_WithTrailers_Decoded(bool sendData) + public async Task CONTINUATION_Received_WithTrailers_Discarded(bool sendData) { await InitializeConnectionAsync(_readTrailersApplication); @@ -2994,11 +3064,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests withFlags: (byte)Http2DataFrameFlags.END_STREAM, withStreamId: 1); - VerifyDecodedRequestHeaders(_browserRequestHeaders.Concat(new[] - { - new KeyValuePair("trailer-1", "1"), - new KeyValuePair("trailer-2", "2") - })); + VerifyDecodedRequestHeaders(_browserRequestHeaders); + + // Make sure the trailers are missing. https://github.com/aspnet/KestrelHttpServer/issues/2630 + Assert.False(_receivedHeaders.ContainsKey("trailer-1")); + Assert.False(_receivedHeaders.ContainsKey("trailer-2")); await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); } @@ -3128,14 +3198,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); Assert.Equal("0", _decodedHeaders["content-length"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["a"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["b"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["c"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["d"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["e"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["f"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["g"]); - Assert.Equal(_largeHeaderValue, _decodedHeaders["h"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["a"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["b"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["c"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["d"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["e"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["f"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["g"]); + Assert.Equal(_4kHeaderValue, _decodedHeaders["h"]); } [Fact] diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 6a727466d1..7df7750f81 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -569,6 +569,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); } + [Fact] + public async Task HEADERS_Received_MaxRequestLineSize_Reset() + { + // Default 8kb limit + // This test has to work around the HPack parser limit for incoming field sizes over 4kb. That's going to be a problem for people with long urls. + // https://github.com/aspnet/KestrelHttpServer/issues/2872 + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET" + new string('a', 1024 * 3)), + new KeyValuePair(HeaderNames.Path, "/Hello/How/Are/You/" + new string('a', 1024 * 3)), + new KeyValuePair(HeaderNames.Scheme, "http"), + new KeyValuePair(HeaderNames.Authority, "localhost" + new string('a', 1024 * 3) + ":80"), + }; + await InitializeConnectionAsync(_noopApplication); + + await StartStreamAsync(1, headers, endStream: true); + + await WaitForStreamErrorAsync(expectedStreamId: 1, Http2ErrorCode.PROTOCOL_ERROR, CoreStrings.BadRequest_RequestLineTooLong); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task ContentLength_Received_SingleDataFrame_Verified() { @@ -623,10 +645,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests 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("a", _4kHeaderValue), + new KeyValuePair("b", _4kHeaderValue), + new KeyValuePair("c", _4kHeaderValue), + new KeyValuePair("d", _4kHeaderValue), new KeyValuePair(HeaderNames.ContentLength, "12"), }; await StartStreamAsync(1, headers, endStream: false); @@ -726,10 +748,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests 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("a", _4kHeaderValue), + new KeyValuePair("b", _4kHeaderValue), + new KeyValuePair("c", _4kHeaderValue), + new KeyValuePair("d", _4kHeaderValue), new KeyValuePair(HeaderNames.ContentLength, "12"), }; await InitializeConnectionAsync(_noopApplication); diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs index 92e4921508..fff7a4bba7 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { public class Http2TestBase : TestApplicationErrorLoggerLoggedTest, IDisposable, IHttpHeadersHandler { - protected static readonly string _largeHeaderValue = new string('a', HPackDecoder.MaxStringOctets); + protected static readonly string _4kHeaderValue = new string('a', HPackDecoder.MaxStringOctets); protected static readonly IEnumerable> _browserRequestHeaders = new[] { @@ -174,7 +174,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { foreach (var name in new[] { "a", "b", "c", "d", "e", "f", "g", "h" }) { - context.Response.Headers[name] = _largeHeaderValue; + context.Response.Headers[name] = _4kHeaderValue; } return Task.CompletedTask; @@ -307,7 +307,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(); } - protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsLegnth = 6) + protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 2) { _connectionTask = _connection.ProcessRequestsAsync(new DummyApplication(application)); @@ -315,7 +315,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests await SendSettingsAsync(); await ExpectAsync(Http2FrameType.SETTINGS, - withLength: expectedSettingsLegnth, + withLength: expectedSettingsCount * 6, withFlags: 0, withStreamId: 0); diff --git a/test/Kestrel.InMemory.FunctionalTests/HttpProtocolSelectionTests.cs b/test/Kestrel.InMemory.FunctionalTests/HttpProtocolSelectionTests.cs index 4ad2c374dc..7b2028ba69 100644 --- a/test/Kestrel.InMemory.FunctionalTests/HttpProtocolSelectionTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/HttpProtocolSelectionTests.cs @@ -35,11 +35,34 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } [Fact] - public Task Server_Http2Only_Cleartext_Success() + public async Task Server_Http2Only_Cleartext_Success() { // Expect a SETTINGS frame (type 0x4) with default settings - return TestSuccess(HttpProtocols.Http2, Encoding.ASCII.GetString(Http2Connection.ClientPreface), - "\x00\x00\x06\x04\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x64"); + var expected = new byte[] + { + 0x00, 0x00, 0x0C, // Payload Length (6 * settings count) + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, // SETTINGS frame (type 0x04) + 0x00, 0x03, 0x00, 0x00, 0x00, 0x64, // Connection limit + 0x00, 0x06, 0x00, 0x00, 0x80, 0x00 // Header size limit + }; + var testContext = new TestServiceContext(LoggerFactory); + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + { + Protocols = HttpProtocols.Http2 + }; + + using (var server = new TestServer(context => Task.CompletedTask, testContext, listenOptions)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send(Encoding.ASCII.GetString(Http2Connection.ClientPreface)); + // Can't use Receive when expecting binary data + var actual = new byte[expected.Length]; + var read = await connection.Stream.ReadAsync(actual, 0, actual.Length); + Assert.Equal(expected.Length, read); + Assert.Equal(expected, actual); + } + } } private async Task TestSuccess(HttpProtocols serverProtocols, string request, string expectedResponse) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs index 0edb95b273..6760c4b0b5 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs @@ -7,6 +7,7 @@ using System.IO; using System.IO.Pipelines; using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -55,6 +56,73 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } } + [Fact] + public async Task RequestBodyReadAsyncCanBeCancelled() + { + var helloTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var cts = new CancellationTokenSource(); + + using (var server = new TestServer(async context => + { + var buffer = new byte[1024]; + try + { + + int read = await context.Request.Body.ReadAsync(buffer, 0, buffer.Length, cts.Token); + + Assert.Equal("Hello ", Encoding.UTF8.GetString(buffer, 0, read)); + + helloTcs.TrySetResult(null); + } + catch (Exception ex) + { + // This shouldn't fail + helloTcs.TrySetException(ex); + } + + try + { + await context.Request.Body.ReadAsync(buffer, 0, buffer.Length, cts.Token); + + context.Response.ContentLength = 12; + await context.Response.WriteAsync("Read success"); + } + catch (OperationCanceledException) + { + context.Response.ContentLength = 14; + await context.Response.WriteAsync("Read cancelled"); + } + + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "POST / HTTP/1.1", + "Host:", + "Connection: keep-alive", + "Content-Length: 11", + "", + ""); + + await connection.Send("Hello "); + + await helloTcs.Task; + + // Cancel the body after hello is read + cts.Cancel(); + + await connection.Send("World"); + + await connection.Receive($"HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 14", + "", + "Read cancelled"); + } + } + } + [Fact] public async Task CanUpgradeRequestWithConnectionKeepAliveUpgradeHeader() { diff --git a/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs b/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs index e0e63fd0d5..26c1433b16 100644 --- a/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/ResponseTests.cs @@ -7,6 +7,7 @@ using System.IO; using System.Linq; using System.Net; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; @@ -99,6 +100,61 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } } + [Fact] + public async Task ResponseBodyWriteAsyncCanBeCancelled() + { + var serviceContext = new TestServiceContext(LoggerFactory); + serviceContext.ServerOptions.Limits.MaxResponseBufferSize = 5; + var cts = new CancellationTokenSource(); + var appTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var writeReturnedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + using (var server = new TestServer(async context => + { + try + { + await context.Response.WriteAsync("hello", cts.Token).DefaultTimeout(); + writeReturnedTcs.TrySetResult(null); + + var task = context.Response.WriteAsync("world", cts.Token); + Assert.False(task.IsCompleted); + await task.DefaultTimeout(); + } + catch (Exception ex) + { + appTcs.TrySetException(ex); + } + finally + { + appTcs.TrySetResult(null); + writeReturnedTcs.TrySetCanceled(); + } + }, serviceContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + + await connection.Receive($"HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + "5", + "hello"); + + await writeReturnedTcs.Task.DefaultTimeout(); + + cts.Cancel(); + + await Assert.ThrowsAsync(() => appTcs.Task).DefaultTimeout(); + } + } + } + [Fact] public Task ResponseStatusCodeSetBeforeHttpContextDisposeAppException() { diff --git a/test/Kestrel.Tests/ConfigurationReaderTests.cs b/test/Kestrel.Tests/ConfigurationReaderTests.cs index ecc7f5e587..6d9b58071d 100644 --- a/test/Kestrel.Tests/ConfigurationReaderTests.cs +++ b/test/Kestrel.Tests/ConfigurationReaderTests.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Tests public class ConfigurationReaderTests { [Fact] - public void ReadCertificatesWhenNoCertificatsSection_ReturnsEmptyCollection() + public void ReadCertificatesWhenNoCertificatesSection_ReturnsEmptyCollection() { var config = new ConfigurationBuilder().AddInMemoryCollection().Build(); var reader = new ConfigurationReader(config); diff --git a/test/Kestrel.Transport.BindTests/AddressRegistrationTests.cs b/test/Kestrel.Transport.BindTests/AddressRegistrationTests.cs index 7152851f37..a67d4bcaed 100644 --- a/test/Kestrel.Transport.BindTests/AddressRegistrationTests.cs +++ b/test/Kestrel.Transport.BindTests/AddressRegistrationTests.cs @@ -211,7 +211,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests RunTestWithStaticPort(port => RegisterAddresses_Success($"{addressInput}:{port}", testUrls, port)); [Fact] - public async Task RegisterHttpAddress_UpradedToHttpsByConfigureEndpointDefaults() + public async Task RegisterHttpAddress_UpgradedToHttpsByConfigureEndpointDefaults() { var hostBuilder = TransportSelector.GetWebHostBuilder() .UseKestrel(serverOptions => @@ -676,14 +676,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [Theory] [InlineData("ftp://localhost")] [InlineData("ssh://localhost")] - public void ThrowsForUnsupportedAddressFromHosting(string addr) + public void ThrowsForUnsupportedAddressFromHosting(string address) { TestApplicationErrorLogger.IgnoredExceptions.Add(typeof(InvalidOperationException)); var hostBuilder = TransportSelector.GetWebHostBuilder() .ConfigureServices(AddTestLogging) .UseKestrel() - .UseUrls(addr) + .UseUrls(address) .Configure(ConfigureEchoAddress); using (var host = hostBuilder.Build()) @@ -786,7 +786,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var socket = new Socket(AddressFamily.InterNetworkV6, SocketType.Stream, ProtocolType.Tcp)) { - // Bind first to IPv6Any to ensure both the IPv4 and IPv6 ports are avaiable. + // Bind first to IPv6Any to ensure both the IPv4 and IPv6 ports are available. socket.Bind(new IPEndPoint(IPAddress.IPv6Any, 0)); socket.Listen(0); port = ((IPEndPoint)socket.LocalEndPoint).Port; diff --git a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs index e194331df1..ce253762df 100644 --- a/test/Kestrel.Transport.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/RequestTests.cs @@ -658,7 +658,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await Assert.ThrowsAsync(async () => await readTcs.Task); // The cancellation token for only the last request should be triggered. - var abortedRequestId = await registrationTcs.Task; + var abortedRequestId = await registrationTcs.Task.DefaultTimeout(); Assert.Equal(2, abortedRequestId); Assert.Single(TestSink.Writes.Where(w => w.LoggerName == "Microsoft.AspNetCore.Server.Kestrel" && diff --git a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs b/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs index bc61229b9c..e38ea63639 100644 --- a/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.Transport.FunctionalTests/ResponseTests.cs @@ -470,7 +470,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var mockKestrelTrace = new Mock(); mockKestrelTrace - .Setup(trace => trace.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny())) + .Setup(trace => trace.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny())) .Callback(() => responseRateTimeoutMessageLogged.SetResult(null)); mockKestrelTrace .Setup(trace => trace.ConnectionStop(It.IsAny())) @@ -560,7 +560,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var mockKestrelTrace = new Mock(); mockKestrelTrace - .Setup(trace => trace.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny())) + .Setup(trace => trace.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny())) .Callback(() => responseRateTimeoutMessageLogged.SetResult(null)); mockKestrelTrace .Setup(trace => trace.ConnectionStop(It.IsAny())) @@ -645,7 +645,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var mockKestrelTrace = new Mock(); mockKestrelTrace - .Setup(trace => trace.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny())) + .Setup(trace => trace.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny())) .Callback(() => responseRateTimeoutMessageLogged.SetResult(null)); mockKestrelTrace .Setup(trace => trace.ConnectionStop(It.IsAny())) @@ -779,7 +779,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond); await appFuncCompleted.Task.DefaultTimeout(); - mockKestrelTrace.Verify(t => t.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); Assert.False(requestAborted); } @@ -854,7 +854,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var targetBytesPerSecond = responseSize / 4; await AssertStreamCompleted(connection.Stream, minTotalOutputSize, targetBytesPerSecond); - mockKestrelTrace.Verify(t => t.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); + mockKestrelTrace.Verify(t => t.ResponseMinimumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); Assert.False(requestAborted); } diff --git a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs index 466f68ac40..48c2cee506 100644 --- a/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs +++ b/test/Kestrel.Transport.Libuv.Tests/LibuvOutputConsumerTests.cs @@ -303,12 +303,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests Assert.NotEmpty(completeQueue); // Add more bytes to the write-behind buffer to prevent the next write from - outputProducer.Write((writableBuffer, state) => + _ = outputProducer.WriteAsync((writableBuffer, state) => { writableBuffer.Write(state); return state.Count; }, - halfWriteBehindBuffer); + halfWriteBehindBuffer, + default); // Act var writeTask2 = outputProducer.WriteDataAsync(halfWriteBehindBuffer); diff --git a/test/shared/CompositeKestrelTrace.cs b/test/shared/CompositeKestrelTrace.cs index ecd82d1a99..8c9be6c1bc 100644 --- a/test/shared/CompositeKestrelTrace.cs +++ b/test/shared/CompositeKestrelTrace.cs @@ -153,16 +153,16 @@ namespace Microsoft.AspNetCore.Testing _trace2.RequestBodyDrainTimedOut(connectionId, traceIdentifier); } - public void RequestBodyMininumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) + public void RequestBodyMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier, double rate) { - _trace1.RequestBodyMininumDataRateNotSatisfied(connectionId, traceIdentifier, rate); - _trace2.RequestBodyMininumDataRateNotSatisfied(connectionId, traceIdentifier, rate); + _trace1.RequestBodyMinimumDataRateNotSatisfied(connectionId, traceIdentifier, rate); + _trace2.RequestBodyMinimumDataRateNotSatisfied(connectionId, traceIdentifier, rate); } - public void ResponseMininumDataRateNotSatisfied(string connectionId, string traceIdentifier) + public void ResponseMinimumDataRateNotSatisfied(string connectionId, string traceIdentifier) { - _trace1.ResponseMininumDataRateNotSatisfied(connectionId, traceIdentifier); - _trace2.ResponseMininumDataRateNotSatisfied(connectionId, traceIdentifier); + _trace1.ResponseMinimumDataRateNotSatisfied(connectionId, traceIdentifier); + _trace2.ResponseMinimumDataRateNotSatisfied(connectionId, traceIdentifier); } public void ApplicationAbortedConnection(string connectionId, string traceIdentifier) diff --git a/test/shared/TransportTestHelpers/IPv6ScopeIdPresentConditionAttribute.cs b/test/shared/TransportTestHelpers/IPv6ScopeIdPresentConditionAttribute.cs index f1705b9c7b..a1bab723cf 100644 --- a/test/shared/TransportTestHelpers/IPv6ScopeIdPresentConditionAttribute.cs +++ b/test/shared/TransportTestHelpers/IPv6ScopeIdPresentConditionAttribute.cs @@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests return NetworkInterface.GetAllNetworkInterfaces() .Where(iface => iface.OperationalStatus == OperationalStatus.Up) .SelectMany(iface => iface.GetIPProperties().UnicastAddresses) - .Any(addrInfo => addrInfo.Address.AddressFamily == AddressFamily.InterNetworkV6 && addrInfo.Address.ScopeId != 0); + .Any(addressInfo => addressInfo.Address.AddressFamily == AddressFamily.InterNetworkV6 && addressInfo.Address.ScopeId != 0); } catch (SocketException) { diff --git a/tools/CodeGenerator/KnownHeaders.cs b/tools/CodeGenerator/KnownHeaders.cs index 9d84bab347..5e16debf1d 100644 --- a/tools/CodeGenerator/KnownHeaders.cs +++ b/tools/CodeGenerator/KnownHeaders.cs @@ -269,7 +269,7 @@ namespace CodeGenerator PrimaryHeader = responsePrimaryHeaders.Contains("Content-Length") }}) .ToArray(); - // 63 for reponseHeaders as it steals one bit for Content-Length in CopyTo(ref MemoryPoolIterator output) + // 63 for responseHeaders as it steals one bit for Content-Length in CopyTo(ref MemoryPoolIterator output) Debug.Assert(responseHeaders.Length <= 63); Debug.Assert(responseHeaders.Max(x => x.Index) <= 62);