From ef2d60f886f60cdf6530345396460816c5794def Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Mon, 9 Oct 2017 18:45:37 -0700 Subject: [PATCH 1/2] HTTP/2: implement 100-continue. --- .../Internal/Http/Http1MessageBody.cs | 12 +----- src/Kestrel.Core/Internal/Http/MessageBody.cs | 28 +++++++++++-- .../Internal/Http2/Http2FrameWriter.cs | 12 +++++- .../Http2ConnectionTests.cs | 41 +++++++++++++++++++ 4 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 4f623108d..7e71cd301 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -14,7 +14,6 @@ public abstract class Http1MessageBody : MessageBody { private readonly Http1Connection _context; - private bool _send100Continue = true; private volatile bool _canceled; private Task _pumpTask; @@ -140,7 +139,7 @@ protected override async Task OnConsumeAsync() ReadResult result; do { - result = await _context.RequestBodyPipe.Reader.ReadAsync(); + result = await ReadRequestBodyPipeAsync(); _context.RequestBodyPipe.Reader.Advance(result.Buffer.End); } while (!result.IsCompleted); } @@ -150,15 +149,6 @@ protected override async Task OnConsumeAsync() } } - private void TryProduceContinue() - { - if (_send100Continue) - { - _context.HttpResponseControl.ProduceContinue(); - _send100Continue = false; - } - } - protected void Copy(ReadableBuffer readableBuffer, WritableBuffer writableBuffer) { _context.TimeoutControl.BytesRead(readableBuffer.Length); diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index 8ae912435..c1ad3390b 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -6,7 +6,6 @@ using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http @@ -18,6 +17,8 @@ public abstract class MessageBody private readonly HttpProtocol _context; + private bool _send100Continue = true; + protected MessageBody(HttpProtocol context) { _context = context; @@ -41,7 +42,7 @@ protected MessageBody(HttpProtocol context) while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(); + var result = await ReadRequestBodyPipeAsync(); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; @@ -74,7 +75,7 @@ protected MessageBody(HttpProtocol context) while (true) { - var result = await _context.RequestBodyPipe.Reader.ReadAsync(); + var result = await ReadRequestBodyPipeAsync(); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; @@ -111,6 +112,27 @@ public virtual Task ConsumeAsync() public abstract Task StopAsync(); + protected void TryProduceContinue() + { + if (_send100Continue) + { + _context.HttpResponseControl.ProduceContinue(); + _send100Continue = false; + } + } + + protected ReadableBufferAwaitable ReadRequestBodyPipeAsync() + { + var awaitable = _context.RequestBodyPipe.Reader.ReadAsync(); + + if (!awaitable.IsCompleted) + { + TryProduceContinue(); + } + + return awaitable; + } + private void TryInit() { if (!_context.HasStartedConsumingRequestBody) diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs index fb2468d0c..a34bf0ef5 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs @@ -14,6 +14,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 { public class Http2FrameWriter : IHttp2FrameWriter { + // Literal Header Field without Indexing - Indexed Name (Index 8 - :status) + private static readonly byte[] _continueBytes = new byte[] { 0x08, 0x03, (byte)'1', (byte)'0', (byte)'0' }; + private readonly Http2Frame _outgoingFrame = new Http2Frame(); private readonly object _writeLock = new object(); private readonly HPackEncoder _hpackEncoder = new HPackEncoder(); @@ -50,7 +53,14 @@ public Task FlushAsync(CancellationToken cancellationToken) public Task Write100ContinueAsync(int streamId) { - return Task.CompletedTask; + lock (_writeLock) + { + _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS, streamId); + _outgoingFrame.Length = _continueBytes.Length; + _continueBytes.CopyTo(_outgoingFrame.HeadersPayload); + + return WriteAsync(_outgoingFrame.Raw); + } } public void WriteResponseHeaders(int streamId, int statusCode, IHeaderDictionary headers) diff --git a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs index 39db80b4a..e4a875da5 100644 --- a/test/Kestrel.Core.Tests/Http2ConnectionTests.cs +++ b/test/Kestrel.Core.Tests/Http2ConnectionTests.cs @@ -33,6 +33,15 @@ public class Http2ConnectionTests : IDisposable, IHttpHeadersHandler new KeyValuePair(":scheme", "http"), }; + private static readonly IEnumerable> _expectContinueRequestHeaders = new[] + { + new KeyValuePair(":method", "POST"), + new KeyValuePair(":path", "/"), + new KeyValuePair(":authority", "127.0.0.1"), + new KeyValuePair(":scheme", "https"), + new KeyValuePair("expect", "100-continue"), + }; + private static readonly IEnumerable> _browserRequestHeaders = new[] { new KeyValuePair(":method", "GET"), @@ -755,6 +764,38 @@ await ExpectAsync(Http2FrameType.DATA, await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false); } + [Fact] + public async Task HEADERS_Received_ContainsExpect100Continue_100ContinueSent() + { + await InitializeConnectionAsync(_echoApplication); + + await StartStreamAsync(1, _expectContinueRequestHeaders, false); + + var frame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 5, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await SendDataAsync(1, _helloBytes, endStream: true); + + await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 5, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + Assert.Equal(new byte[] { 0x08, 0x03, (byte)'1', (byte)'0', (byte)'0' }, frame.HeadersPayload.ToArray()); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + } + [Fact] public async Task HEADERS_Received_StreamIdZero_ConnectionError() { From 72640d58f87dd009be170120539ccba7691389b9 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 18 Oct 2017 15:34:48 -0700 Subject: [PATCH 2/2] Don't check _send100Continue on every read --- .../Internal/Http/Http1MessageBody.cs | 2 +- src/Kestrel.Core/Internal/Http/MessageBody.cs | 16 ++-------------- .../Internal/Http2/Http2MessageBody.cs | 9 +++++++++ src/Kestrel.Core/Internal/Http2/Http2Stream.cs | 2 ++ 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs index 7e71cd301..9b50aba2e 100644 --- a/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/Http1MessageBody.cs @@ -139,7 +139,7 @@ protected override async Task OnConsumeAsync() ReadResult result; do { - result = await ReadRequestBodyPipeAsync(); + result = await _context.RequestBodyPipe.Reader.ReadAsync(); _context.RequestBodyPipe.Reader.Advance(result.Buffer.End); } while (!result.IsCompleted); } diff --git a/src/Kestrel.Core/Internal/Http/MessageBody.cs b/src/Kestrel.Core/Internal/Http/MessageBody.cs index c1ad3390b..2fd8c12e6 100644 --- a/src/Kestrel.Core/Internal/Http/MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http/MessageBody.cs @@ -42,7 +42,7 @@ protected MessageBody(HttpProtocol context) while (true) { - var result = await ReadRequestBodyPipeAsync(); + var result = await _context.RequestBodyPipe.Reader.ReadAsync(); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; @@ -75,7 +75,7 @@ protected MessageBody(HttpProtocol context) while (true) { - var result = await ReadRequestBodyPipeAsync(); + var result = await _context.RequestBodyPipe.Reader.ReadAsync(); var readableBuffer = result.Buffer; var consumed = readableBuffer.End; @@ -121,18 +121,6 @@ protected void TryProduceContinue() } } - protected ReadableBufferAwaitable ReadRequestBodyPipeAsync() - { - var awaitable = _context.RequestBodyPipe.Reader.ReadAsync(); - - if (!awaitable.IsCompleted) - { - TryProduceContinue(); - } - - return awaitable; - } - private void TryInit() { if (!_context.HasStartedConsumingRequestBody) diff --git a/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs b/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs index 71a308a8f..b6ad3af16 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2MessageBody.cs @@ -16,6 +16,15 @@ protected Http2MessageBody(Http2Stream context) _context = context; } + protected override void OnReadStarted() + { + // Produce 100-continue if no request body data for the stream has arrived yet. + if (!_context.RequestBodyStarted) + { + TryProduceContinue(); + } + } + protected override Task OnConsumeAsync() => Task.CompletedTask; public override Task StopAsync() diff --git a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs index b8f9db04e..c9832ef03 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2Stream.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2Stream.cs @@ -24,6 +24,7 @@ public Http2Stream(Http2StreamContext context) public int StreamId => _context.StreamId; + public bool RequestBodyStarted { get; private set; } public bool EndStreamReceived { get; private set; } protected IHttp2StreamLifetimeHandler StreamLifetimeHandler => _context.StreamLifetimeHandler; @@ -84,6 +85,7 @@ public async Task OnDataAsync(ArraySegment data, bool endStream) writableBuffer.Commit(); } + RequestBodyStarted = true; await writableBuffer.FlushAsync(); }