From 81b757afcc15c02fc89f35f47cc96a148b09648f Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Fri, 28 Jun 2019 23:51:52 +0200 Subject: [PATCH] Pass ReadOnlySequence via in (#11052) --- src/Http/WebUtilities/src/FormPipeReader.cs | 2 +- .../src/Internal/Http/BufferExtensions.cs | 2 +- .../Http/Http1ChunkedEncodingMessageBody.cs | 14 ++-- .../Core/src/Internal/Http/Http1Connection.cs | 45 +++++++---- .../src/Internal/Http2/HPack/HPackDecoder.cs | 2 +- .../src/Internal/Http2/Http2Connection.cs | 16 ++-- .../src/Internal/Http2/Http2FrameReader.cs | 6 +- .../src/Internal/Http2/Http2FrameWriter.cs | 74 ++++++++++++++----- .../Core/src/Internal/Http2/Http2Stream.cs | 2 +- .../src/Internal/Networking/UvWriteReq.cs | 4 +- .../src/Internal/SocketSender.cs | 4 +- 11 files changed, 109 insertions(+), 62 deletions(-) diff --git a/src/Http/WebUtilities/src/FormPipeReader.cs b/src/Http/WebUtilities/src/FormPipeReader.cs index 3d7c99f083..c8d8497807 100644 --- a/src/Http/WebUtilities/src/FormPipeReader.cs +++ b/src/Http/WebUtilities/src/FormPipeReader.cs @@ -284,7 +284,7 @@ namespace Microsoft.AspNetCore.WebUtilities throw new InvalidDataException($"Form value length limit {ValueLengthLimit} exceeded."); } - private string GetDecodedStringFromReadOnlySequence(ReadOnlySequence ros) + private string GetDecodedStringFromReadOnlySequence(in ReadOnlySequence ros) { if (ros.IsSingleSegment) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/BufferExtensions.cs b/src/Servers/Kestrel/Core/src/Internal/Http/BufferExtensions.cs index f24886152b..72d19d94fb 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/BufferExtensions.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/BufferExtensions.cs @@ -17,7 +17,7 @@ namespace System.Buffers private static byte[] _numericBytesScratch; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static ReadOnlySpan ToSpan(this ReadOnlySequence buffer) + public static ReadOnlySpan ToSpan(in this ReadOnlySequence buffer) { if (buffer.IsSingleSegment) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index fabf8950a4..984e38348c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -220,7 +220,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http _requestBodyPipe.Reset(); } - private void Copy(ReadOnlySequence readableBuffer, PipeWriter writableBuffer) + private void Copy(in ReadOnlySequence readableBuffer, PipeWriter writableBuffer) { if (readableBuffer.IsSingleSegment) { @@ -321,10 +321,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http return _mode == Mode.Complete; } - private void ParseChunkedPrefix(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private void ParseChunkedPrefix(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; - examined = buffer.Start; var reader = new SequenceReader(buffer); if (!reader.TryRead(out var ch1) || !reader.TryRead(out var ch2)) @@ -383,7 +382,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { // Chunk-extensions not currently parsed // Just drain the data - consumed = buffer.Start; examined = buffer.Start; do @@ -432,7 +430,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } while (_mode == Mode.Extension); } - private void ReadChunkedData(ReadOnlySequence buffer, PipeWriter writableBuffer, out SequencePosition consumed, out SequencePosition examined) + private void ReadChunkedData(in ReadOnlySequence buffer, PipeWriter writableBuffer, out SequencePosition consumed, out SequencePosition examined) { var actual = Math.Min(buffer.Length, _inputLength); consumed = buffer.GetPosition(actual); @@ -449,10 +447,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } - private void ParseChunkedSuffix(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private void ParseChunkedSuffix(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; - examined = buffer.Start; if (buffer.Length < 2) { @@ -478,10 +475,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } } - private void ParseChunkedTrailer(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private void ParseChunkedTrailer(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; - examined = buffer.Start; if (buffer.Length < 2) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 0766be46c3..165be2add9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -131,7 +131,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http SendTimeoutResponse(); } - public void ParseRequest(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + public void ParseRequest(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; examined = buffer.End; @@ -151,10 +151,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http case RequestProcessingStatus.ParsingRequestLine: if (TakeStartLine(buffer, out consumed, out examined)) { - buffer = buffer.Slice(consumed, buffer.End); - - _requestProcessingStatus = RequestProcessingStatus.ParsingHeaders; - goto case RequestProcessingStatus.ParsingHeaders; + TrimAndParseHeaders(buffer, ref consumed, out examined); + return; } else { @@ -167,24 +165,42 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } break; } + + void TrimAndParseHeaders(in ReadOnlySequence buffer, ref SequencePosition consumed, out SequencePosition examined) + { + var trimmedBuffer = buffer.Slice(consumed, buffer.End); + _requestProcessingStatus = RequestProcessingStatus.ParsingHeaders; + + if (TakeMessageHeaders(trimmedBuffer, trailers: false, out consumed, out examined)) + { + _requestProcessingStatus = RequestProcessingStatus.AppStarted; + } + } } - public bool TakeStartLine(ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + public bool TakeStartLine(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { - var overLength = false; + // Make sure the buffer is limited if (buffer.Length >= ServerOptions.Limits.MaxRequestLineSize) { - buffer = buffer.Slice(buffer.Start, ServerOptions.Limits.MaxRequestLineSize); - overLength = true; + // Input oversize, cap amount checked + return TrimAndTakeStartLine(buffer, out consumed, out examined); } - var result = _parser.ParseRequestLine(new Http1ParsingHandler(this), buffer, out consumed, out examined); - if (!result && overLength) + return _parser.ParseRequestLine(new Http1ParsingHandler(this), buffer, out consumed, out examined); + + bool TrimAndTakeStartLine(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { - BadHttpRequestException.Throw(RequestRejectionReason.RequestLineTooLong); - } + var trimmedBuffer = buffer.Slice(buffer.Start, ServerOptions.Limits.MaxRequestLineSize); - return result; + if (!_parser.ParseRequestLine(new Http1ParsingHandler(this), trimmedBuffer, out consumed, out examined)) + { + // We read the maximum allowed but didn't complete the start line. + BadHttpRequestException.Throw(RequestRejectionReason.RequestLineTooLong); + } + + return true; + } } public bool TakeMessageHeaders(in ReadOnlySequence buffer, bool trailers, out SequencePosition consumed, out SequencePosition examined) @@ -192,6 +208,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // Make sure the buffer is limited if (buffer.Length > _remainingRequestHeadersBytesAllowed) { + // Input oversize, cap amount checked return TrimAndTakeMessageHeaders(buffer, trailers, out consumed, out examined); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/HPack/HPackDecoder.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/HPack/HPackDecoder.cs index 4f644db63d..543ce8afc1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/HPack/HPackDecoder.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/HPack/HPackDecoder.cs @@ -108,7 +108,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.HPack _headerValueOctets = new byte[maxRequestHeaderFieldSize]; } - public void Decode(ReadOnlySequence data, bool endHeaders, IHttpHeadersHandler handler) + public void Decode(in ReadOnlySequence data, bool endHeaders, IHttpHeadersHandler handler) { foreach (var segment in data) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index fd5728b8e2..be4fece62b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -379,7 +379,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return true; } - private Task ProcessFrameAsync(IHttpApplication application, ReadOnlySequence payload) + private Task ProcessFrameAsync(IHttpApplication application, in ReadOnlySequence payload) { // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1.1 // Streams initiated by a client MUST use odd-numbered stream identifiers; ... @@ -417,7 +417,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - private Task ProcessDataFrameAsync(ReadOnlySequence payload) + private Task ProcessDataFrameAsync(in ReadOnlySequence payload) { if (_currentHeadersStream != null) { @@ -473,7 +473,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamClosed(_incomingFrame.Type, _incomingFrame.StreamId), Http2ErrorCode.STREAM_CLOSED); } - private Task ProcessHeadersFrameAsync(IHttpApplication application, ReadOnlySequence payload) + private Task ProcessHeadersFrameAsync(IHttpApplication application, in ReadOnlySequence payload) { if (_currentHeadersStream != null) { @@ -640,7 +640,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - private Task ProcessSettingsFrameAsync(ReadOnlySequence payload) + private Task ProcessSettingsFrameAsync(in ReadOnlySequence payload) { if (_currentHeadersStream != null) { @@ -711,7 +711,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - private Task ProcessPingFrameAsync(ReadOnlySequence payload) + private Task ProcessPingFrameAsync(in ReadOnlySequence payload) { if (_currentHeadersStream != null) { @@ -818,7 +818,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - private Task ProcessContinuationFrameAsync(ReadOnlySequence payload) + private Task ProcessContinuationFrameAsync(in ReadOnlySequence payload) { if (_currentHeadersStream == null) { @@ -857,7 +857,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - private Task DecodeHeadersAsync(bool endHeaders, ReadOnlySequence payload) + private Task DecodeHeadersAsync(bool endHeaders, in ReadOnlySequence payload) { try { @@ -879,7 +879,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - private Task DecodeTrailersAsync(bool endHeaders, ReadOnlySequence payload) + private Task DecodeTrailersAsync(bool endHeaders, in ReadOnlySequence payload) { _hpackDecoder.Decode(payload, endHeaders, handler: this); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameReader.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameReader.cs index 4b6f67bf46..8437ad334a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameReader.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameReader.cs @@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 public const int SettingSize = 6; // 2 bytes for the id, 4 bytes for the value. - public static bool ReadFrame(ReadOnlySequence readableBuffer, Http2Frame frame, uint maxFrameSize, out ReadOnlySequence framePayload) + public static bool ReadFrame(in ReadOnlySequence readableBuffer, Http2Frame frame, uint maxFrameSize, out ReadOnlySequence framePayload) { framePayload = ReadOnlySequence.Empty; @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return true; } - private static int ReadExtendedFields(Http2Frame frame, ReadOnlySequence readableBuffer) + private static int ReadExtendedFields(Http2Frame frame, in ReadOnlySequence readableBuffer) { // Copy in any extra fields for the given frame type var extendedHeaderLength = GetPayloadFieldsLength(frame); @@ -217,7 +217,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public static IList ReadSettings(ReadOnlySequence payload) + public static IList ReadSettings(in ReadOnlySequence payload) { var data = payload.ToSpan(); Debug.Assert(data.Length % SettingSize == 0, "Invalid settings payload length"); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 5934083371..acdecca2ad 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -235,7 +235,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, bool endStream) + public ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, in ReadOnlySequence data, bool endStream) { // The Length property of a ReadOnlySequence can be expensive, so we cache the value. var dataLength = data.Length; @@ -270,28 +270,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 | Padding (*) ... +---------------------------------------------------------------+ */ - private void WriteDataUnsynchronized(int streamId, ReadOnlySequence data, long dataLength, bool endStream) + private void WriteDataUnsynchronized(int streamId, in ReadOnlySequence data, long dataLength, bool endStream) { + Debug.Assert(dataLength == data.Length); + // Note padding is not implemented _outgoingFrame.PrepareData(streamId); - var dataPayloadLength = (int)_maxFrameSize; // Minus padding - - while (data.Length > dataPayloadLength) + if (dataLength > _maxFrameSize) // Minus padding { - var currentData = data.Slice(0, dataPayloadLength); - _outgoingFrame.PayloadLength = dataPayloadLength; // Plus padding - - WriteHeaderUnsynchronized(); - - foreach (var buffer in currentData) - { - _outputWriter.Write(buffer.Span); - } - - // Plus padding - - data = data.Slice(dataPayloadLength); + TrimAndWriteDataUnsynchronized(in data, dataLength, endStream); + return; } if (endStream) @@ -299,7 +288,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 _outgoingFrame.DataFlags |= Http2DataFrameFlags.END_STREAM; } - _outgoingFrame.PayloadLength = (int)data.Length; // Plus padding + _outgoingFrame.PayloadLength = (int)dataLength; // Plus padding WriteHeaderUnsynchronized(); @@ -309,6 +298,51 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } // Plus padding + return; + + void TrimAndWriteDataUnsynchronized(in ReadOnlySequence data, long dataLength, bool endStream) + { + Debug.Assert(dataLength == data.Length); + + var dataPayloadLength = (int)_maxFrameSize; // Minus padding + + Debug.Assert(dataLength > dataPayloadLength); + + var remainingData = data; + do + { + var currentData = remainingData.Slice(0, dataPayloadLength); + _outgoingFrame.PayloadLength = dataPayloadLength; // Plus padding + + WriteHeaderUnsynchronized(); + + foreach (var buffer in currentData) + { + _outputWriter.Write(buffer.Span); + } + + // Plus padding + dataLength -= dataPayloadLength; + remainingData = remainingData.Slice(dataPayloadLength); + + } while (dataLength > dataPayloadLength); + + if (endStream) + { + _outgoingFrame.DataFlags |= Http2DataFrameFlags.END_STREAM; + } + + _outgoingFrame.PayloadLength = (int)dataLength; // Plus padding + + WriteHeaderUnsynchronized(); + + foreach (var buffer in remainingData) + { + _outputWriter.Write(buffer.Span); + } + + // Plus padding + } } private async ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, long dataLength, bool endStream) @@ -498,7 +532,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 | | +---------------------------------------------------------------+ */ - public ValueTask WritePingAsync(Http2PingFrameFlags flags, ReadOnlySequence payload) + public ValueTask WritePingAsync(Http2PingFrameFlags flags, in ReadOnlySequence payload) { lock (_writeLock) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs index d2d1cb3bb9..174cccb47f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Stream.cs @@ -320,7 +320,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public Task OnDataAsync(Http2Frame dataFrame, ReadOnlySequence payload) + public Task OnDataAsync(Http2Frame dataFrame, in ReadOnlySequence payload) { // Since padding isn't buffered, immediately count padding bytes as read for flow control purposes. if (dataFrame.DataHasPadding) diff --git a/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/UvWriteReq.cs b/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/UvWriteReq.cs index 5d71426c80..18687d7d85 100644 --- a/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/UvWriteReq.cs +++ b/src/Servers/Kestrel/Transport.Libuv/src/Internal/Networking/UvWriteReq.cs @@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin _bufs = handle + requestSize; } - public LibuvAwaitable WriteAsync(UvStreamHandle handle, ReadOnlySequence buffer) + public LibuvAwaitable WriteAsync(UvStreamHandle handle, in ReadOnlySequence buffer) { Write(handle, buffer, LibuvAwaitable.Callback, _awaitable); return _awaitable; @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal.Networkin private unsafe void Write( UvStreamHandle handle, - ReadOnlySequence buffer, + in ReadOnlySequence buffer, Action callback, object state) { diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketSender.cs b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketSender.cs index 235c2c3ec4..de8ca1cc16 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketSender.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketSender.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal { } - public SocketAwaitableEventArgs SendAsync(ReadOnlySequence buffers) + public SocketAwaitableEventArgs SendAsync(in ReadOnlySequence buffers) { if (buffers.IsSingleSegment) { @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.Internal return _awaitableEventArgs; } - private List> GetBufferList(ReadOnlySequence buffer) + private List> GetBufferList(in ReadOnlySequence buffer) { Debug.Assert(!buffer.IsEmpty); Debug.Assert(!buffer.IsSingleSegment);