From cb6059c143e595dd47b47892aa002adfcf3b9018 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 1 Mar 2017 13:12:03 -0800 Subject: [PATCH] Make the IHttpParser per frame and add a reset (#1415) * Make the IHttpParser per frame and add a reset - Made the IHttpParser a per frame object so state can be stored across method calls and parses. - Added HttpParserFactory to ServiceContext --- .../Internal/Http/Frame.cs | 5 +++- .../Internal/Http/IHttpParser.cs | 2 ++ .../Internal/Http/KestrelHttpParser.cs | 5 ++++ .../Internal/ServiceContext.cs | 2 +- .../KestrelServer.cs | 2 +- .../RequestParsing.cs | 4 +-- .../FrameResponseHeadersTests.cs | 30 ++++++++++++++++--- .../FrameTests.cs | 4 +-- .../ListenerPrimaryTests.cs | 6 ++-- .../TestInput.cs | 4 +-- test/shared/TestServiceContext.cs | 2 +- 11 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index 26e9dddee6..c87c72c468 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -86,7 +86,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http ServerOptions = context.ListenerContext.ServiceContext.ServerOptions; _pathBase = context.ListenerContext.ListenOptions.PathBase; - _parser = context.ListenerContext.ServiceContext.HttpParser; + _parser = context.ListenerContext.ServiceContext.HttpParserFactory(this); FrameControl = this; _keepAliveMilliseconds = (long)ServerOptions.Limits.KeepAliveTimeout.TotalMilliseconds; @@ -379,6 +379,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http _requestHeadersParsed = 0; _responseBytesWritten = 0; + + // When testing parser can be null + _parser.Reset(); } /// diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpParser.cs index e1d68293cd..ee888bea1f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/IHttpParser.cs @@ -10,5 +10,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http bool ParseRequestLine(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) where T : IHttpRequestLineHandler; bool ParseHeaders(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler; + + void Reset(); } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 8f334a5a20..662fd8aabe 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -525,6 +525,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestLineError) : string.Empty); } + public void Reset() + { + + } + private enum HeaderState { Name, diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/ServiceContext.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/ServiceContext.cs index 531ff25fe7..6233378f40 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/ServiceContext.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/ServiceContext.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal public IThreadPool ThreadPool { get; set; } - public IHttpParser HttpParser { get; set; } + public Func HttpParserFactory { get; set; } public Func FrameFactory { get; set; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs index 194d03b0dd..1d05a0dd2b 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs @@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel }, AppLifetime = _applicationLifetime, Log = trace, - HttpParser = new KestrelHttpParser(trace), + HttpParserFactory = frame => new KestrelHttpParser(frame.ConnectionContext.ListenerContext.ServiceContext.Log), ThreadPool = threadPool, DateHeaderValueManager = dateHeaderValueManager, ServerOptions = Options diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 0b42b9761e..9d91d8e681 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -162,7 +162,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } Pipe.Reader.Advance(consumed, examined); } - while(true); + while (true); } private void ThrowInvalidStartLine() @@ -179,7 +179,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public void Setup() { var connectionContext = new MockConnection(new KestrelServerOptions()); - connectionContext.ListenerContext.ServiceContext.HttpParser = (IHttpParser) Activator.CreateInstance(ParserType, connectionContext.ListenerContext.ServiceContext.Log); + connectionContext.ListenerContext.ServiceContext.HttpParserFactory = frame => (IHttpParser)Activator.CreateInstance(ParserType, frame.ConnectionContext.ListenerContext.ServiceContext.Log); Frame = new Frame(application: null, context: connectionContext); PipelineFactory = new PipeFactory(); diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs index b3dee3d0c7..7b908fd642 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameResponseHeadersTests.cs @@ -4,16 +4,14 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.IO.Pipelines; using System.Net; -using System.Text; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.Extensions.Primitives; using Xunit; -using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Server.KestrelTests { @@ -27,7 +25,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var serviceContext = new ServiceContext { DateHeaderValueManager = new DateHeaderValueManager(), - ServerOptions = serverOptions + ServerOptions = serverOptions, + HttpParserFactory = f => new NoopHttpParser(), }; var listenerContext = new ListenerContext(serviceContext) { @@ -268,5 +267,28 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "42,000", "42.000", }; + + private class NoopHttpParser : IHttpParser + { + public bool ParseHeaders(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler + { + consumed = buffer.Start; + examined = buffer.End; + consumedBytes = 0; + return false; + } + + public bool ParseRequestLine(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) where T : IHttpRequestLineHandler + { + consumed = buffer.Start; + examined = buffer.End; + return false; + } + + public void Reset() + { + + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 853cb2cefd..f4b4339479 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), - HttpParser = new KestrelHttpParser(trace), + HttpParserFactory = frame => new KestrelHttpParser(trace), Log = trace }; var listenerContext = new ListenerContext(_serviceContext) @@ -484,7 +484,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(requestLineBytes); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() =>_frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); + var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal("Request line too long.", exception.Message); diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/ListenerPrimaryTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/ListenerPrimaryTests.cs index 797520a1c7..486b9ffda5 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/ListenerPrimaryTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/ListenerPrimaryTests.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests DateHeaderValueManager = serviceContextPrimary.DateHeaderValueManager, ServerOptions = serviceContextPrimary.ServerOptions, ThreadPool = serviceContextPrimary.ThreadPool, - HttpParser = new KestrelHttpParser(serviceContextPrimary.Log), + HttpParserFactory = frame => new KestrelHttpParser(serviceContextPrimary.Log), FrameFactory = context => { return new Frame(new TestApplication(c => @@ -122,7 +122,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests DateHeaderValueManager = serviceContextPrimary.DateHeaderValueManager, ServerOptions = serviceContextPrimary.ServerOptions, ThreadPool = serviceContextPrimary.ThreadPool, - HttpParser = new KestrelHttpParser(serviceContextPrimary.Log), + HttpParserFactory = frame => new KestrelHttpParser(serviceContextPrimary.Log), FrameFactory = context => { return new Frame(new TestApplication(c => @@ -245,7 +245,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests DateHeaderValueManager = serviceContextPrimary.DateHeaderValueManager, ServerOptions = serviceContextPrimary.ServerOptions, ThreadPool = serviceContextPrimary.ThreadPool, - HttpParser = new KestrelHttpParser(serviceContextPrimary.Log), + HttpParserFactory = frame => new KestrelHttpParser(serviceContextPrimary.Log), FrameFactory = context => { return new Frame(new TestApplication(c => diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/TestInput.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/TestInput.cs index fae8ad499f..19f1f81890 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/TestInput.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/TestInput.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), - HttpParser = new KestrelHttpParser(trace), + HttpParserFactory = frame => new KestrelHttpParser(trace), }; var listenerContext = new ListenerContext(serviceContext) { @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _memoryPool = new MemoryPool(); _pipelineFactory = new PipeFactory(); - FrameContext.Input = _pipelineFactory.Create();; + FrameContext.Input = _pipelineFactory.Create(); ; } public Frame FrameContext { get; set; } diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index 356e1e3beb..ad78537efb 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Testing ThreadPool = new LoggingThreadPool(Log); DateHeaderValueManager = new DateHeaderValueManager(systemClock: new MockSystemClock()); DateHeaderValue = DateHeaderValueManager.GetDateHeaderValues().String; - HttpParser = new KestrelHttpParser(Log); + HttpParserFactory = frame => new KestrelHttpParser(Log); ServerOptions = new KestrelServerOptions { AddServerHeader = false,