From d3924b0149eeda4defa94ed2dc8af738a65810e1 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Wed, 1 Mar 2017 18:14:13 -0800 Subject: [PATCH 01/28] Change korebuild branch and fix argument forwarding in bootstrapper --- build.ps1 | 4 ++-- build.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build.ps1 b/build.ps1 index f780c43a82..5bf0e2c113 100644 --- a/build.ps1 +++ b/build.ps1 @@ -33,7 +33,7 @@ cd $PSScriptRoot $repoFolder = $PSScriptRoot $env:REPO_FOLDER = $repoFolder -$koreBuildZip="https://github.com/aspnet/KoreBuild/archive/feature/msbuild.zip" +$koreBuildZip="https://github.com/aspnet/KoreBuild/archive/dev.zip" if ($env:KOREBUILD_ZIP) { $koreBuildZip=$env:KOREBUILD_ZIP @@ -64,4 +64,4 @@ if (!(Test-Path $buildFolder)) { } } -&"$buildFile" $args \ No newline at end of file +&"$buildFile" @args diff --git a/build.sh b/build.sh index ff79789196..b0bcadb579 100755 --- a/build.sh +++ b/build.sh @@ -2,7 +2,7 @@ repoFolder="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" cd $repoFolder -koreBuildZip="https://github.com/aspnet/KoreBuild/archive/feature/msbuild.zip" +koreBuildZip="https://github.com/aspnet/KoreBuild/archive/dev.zip" if [ ! -z $KOREBUILD_ZIP ]; then koreBuildZip=$KOREBUILD_ZIP fi @@ -43,4 +43,4 @@ if test ! -d $buildFolder; then fi fi -$buildFile -r $repoFolder "$@" \ No newline at end of file +$buildFile -r $repoFolder "$@" From f2a00da8110b1679c19640a4ed7043b07c19f8ea Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 1 Mar 2017 18:18:34 -0800 Subject: [PATCH 02/28] Loop over bytes inside states of parser state machine (#1417) --- .../Internal/Http/KestrelHttpParser.cs | 225 ++++++++++-------- 1 file changed, 123 insertions(+), 102 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 662fd8aabe..2d1e51a013 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -72,36 +72,32 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var versionStart = -1; HttpVersion httpVersion = HttpVersion.Unknown; - HttpMethod method = HttpMethod.Custom; + HttpMethod method; Span customMethod; - var state = StartLineState.KnownMethod; + int i = 0; + var length = span.Length; + var done = false; - int i; fixed (byte* data = &span.DangerousGetPinnableReference()) { - var length = span.Length; - for (i = 0; i < length; i++) + switch (StartLineState.KnownMethod) { - var ch = data[i]; + case StartLineState.KnownMethod: + if (span.GetKnownMethod(out method, out var methodLength)) + { + // Update the index, current char, state and jump directly + // to the next state + i += methodLength + 1; - switch (state) - { - case StartLineState.KnownMethod: - if (span.GetKnownMethod(out method, out var methodLength)) - { - // Update the index, current char, state and jump directly - // to the next state - i += methodLength + 1; - ch = data[i]; - state = StartLineState.Path; + goto case StartLineState.Path; + } + goto case StartLineState.UnknownMethod; - goto case StartLineState.Path; - } + case StartLineState.UnknownMethod: + for (; i < length; i++) + { + var ch = data[i]; - state = StartLineState.UnknownMethod; - goto case StartLineState.UnknownMethod; - - case StartLineState.UnknownMethod: if (ch == ByteSpace) { customMethod = span.Slice(0, i); @@ -110,16 +106,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { RejectRequestLine(span); } + // Consume space + i++; - state = StartLineState.Path; + goto case StartLineState.Path; } - else if (!IsValidTokenChar((char)ch)) + + if (!IsValidTokenChar((char)ch)) { RejectRequestLine(span); } + } - break; - case StartLineState.Path: + break; + case StartLineState.Path: + for (; i < length; i++) + { + var ch = data[i]; if (ch == ByteSpace) { pathEnd = i; @@ -133,7 +136,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // No query string found queryStart = queryEnd = i; - state = StartLineState.KnownVersion; + // Consume space + i++; + + goto case StartLineState.KnownVersion; } else if (ch == ByteQuestionMark) { @@ -146,7 +152,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } queryStart = i; - state = StartLineState.QueryString; + goto case StartLineState.QueryString; } else if (ch == BytePercentage) { @@ -160,35 +166,42 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { pathStart = i; } - break; - - case StartLineState.QueryString: + } + break; + case StartLineState.QueryString: + for (; i < length; i++) + { + var ch = data[i]; if (ch == ByteSpace) { queryEnd = i; - state = StartLineState.KnownVersion; + // Consume space + i++; + + goto case StartLineState.KnownVersion; } - break; - case StartLineState.KnownVersion: - // REVIEW: We don't *need* to slice here but it makes the API - // nicer, slicing should be free :) - if (span.Slice(i).GetKnownVersion(out httpVersion, out var versionLenght)) - { - // Update the index, current char, state and jump directly - // to the next state - i += versionLenght + 1; - ch = data[i]; - state = StartLineState.NewLine; + } + break; + case StartLineState.KnownVersion: + // REVIEW: We don't *need* to slice here but it makes the API + // nicer, slicing should be free :) + if (span.Slice(i).GetKnownVersion(out httpVersion, out var versionLenght)) + { + // Update the index, current char, state and jump directly + // to the next state + i += versionLenght + 1; + goto case StartLineState.NewLine; + } - goto case StartLineState.NewLine; - } + versionStart = i; - versionStart = i; - state = StartLineState.UnknownVersion; - goto case StartLineState.UnknownVersion; + goto case StartLineState.UnknownVersion; - case StartLineState.UnknownVersion: + case StartLineState.UnknownVersion: + for (; i < length; i++) + { + var ch = data[i]; if (ch == ByteCR) { var versionSpan = span.Slice(versionStart, i - versionStart); @@ -199,25 +212,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } else { - RejectRequest(RequestRejectionReason.UnrecognizedHTTPVersion, versionSpan.GetAsciiStringEscaped(32)); + RejectRequest(RequestRejectionReason.UnrecognizedHTTPVersion, + versionSpan.GetAsciiStringEscaped(32)); } } - break; - case StartLineState.NewLine: - if (ch != ByteLF) - { - RejectRequestLine(span); - } + } + break; + case StartLineState.NewLine: + if (data[i] != ByteLF) + { + RejectRequestLine(span); + } + i++; - state = StartLineState.Complete; - break; - case StartLineState.Complete: - break; - } + goto case StartLineState.Complete; + case StartLineState.Complete: + done = true; + break; } } - if (state != StartLineState.Complete) + if (!done) { RejectRequestLine(span); } @@ -311,7 +326,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http headerBuffer.CopyTo(span); } - var state = HeaderState.Name; var nameStart = 0; var nameEnd = -1; var valueStart = -1; @@ -320,33 +334,43 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var previouslyWhitespace = false; var headerLineLength = span.Length; + int i = 0; + var length = span.Length; + bool done = false; fixed (byte* data = &span.DangerousGetPinnableReference()) { - for (var i = 0; i < headerLineLength; i++) + switch (HeaderState.Name) { - var ch = data[i]; - - switch (state) - { - case HeaderState.Name: + case HeaderState.Name: + for (; i < length; i++) + { + var ch = data[i]; if (ch == ByteColon) { if (nameHasWhitespace) { RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); } - - state = HeaderState.Whitespace; nameEnd = i; + + // Consume space + i++; + + goto case HeaderState.Whitespace; } if (ch == ByteSpace || ch == ByteTab) { nameHasWhitespace = true; } - break; - case HeaderState.Whitespace: + } + RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); + + break; + case HeaderState.Whitespace: + for (; i < length; i++) { + var ch = data[i]; var whitespace = ch == ByteTab || ch == ByteSpace || ch == ByteCR; if (!whitespace) @@ -354,18 +378,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Mark the first non whitespace char as the start of the // header value and change the state to expect to the header value valueStart = i; - state = HeaderState.ExpectValue; + + goto case HeaderState.ExpectValue; } // If we see a CR then jump to the next state directly else if (ch == ByteCR) { - state = HeaderState.ExpectValue; goto case HeaderState.ExpectValue; } } - break; - case HeaderState.ExpectValue: + + RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + + break; + case HeaderState.ExpectValue: + for (; i < length; i++) { + var ch = data[i]; var whitespace = ch == ByteTab || ch == ByteSpace; if (whitespace) @@ -392,7 +421,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http valueStart = valueEnd; } - state = HeaderState.ExpectNewLine; + // Consume space + i++; + + goto case HeaderState.ExpectNewLine; } else { @@ -402,32 +434,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http previouslyWhitespace = whitespace; } - break; - case HeaderState.ExpectNewLine: - if (ch != ByteLF) - { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); - } - - state = HeaderState.Complete; - break; - default: - break; - } + RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + break; + case HeaderState.ExpectNewLine: + if (data[i] != ByteLF) + { + RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + } + goto case HeaderState.Complete; + case HeaderState.Complete: + done = true; + break; } } - if (state == HeaderState.Name) - { - RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); - } - - if (state == HeaderState.ExpectValue || state == HeaderState.Whitespace) - { - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); - } - - if (state != HeaderState.Complete) + if (!done) { return false; } @@ -527,7 +548,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http public void Reset() { - + } private enum HeaderState From ca31627a5e4c01378bdf0cfb495316803666c7e4 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 1 Mar 2017 22:33:37 -0800 Subject: [PATCH 03/28] Parser clean up (#1419) - Remove stackalloc - Remove extra Move in ParseRequestLine --- .../Internal/Http/KestrelHttpParser.cs | 25 +++---------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 2d1e51a013..18477d28dd 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -32,32 +32,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http examined = buffer.End; var start = buffer.Start; - ReadCursor end; - if (ReadCursorOperations.Seek(start, buffer.End, out end, ByteLF) == -1) + if (ReadCursorOperations.Seek(start, buffer.End, out var end, ByteLF) == -1) { return false; } - const int stackAllocLimit = 512; - // Move 1 byte past the \n end = buffer.Move(end, 1); var startLineBuffer = buffer.Slice(start, end); Span span; - if (startLineBuffer.IsSingleSpan) { // No copies, directly use the one and only span span = startLineBuffer.First.Span; } - else if (startLineBuffer.Length < stackAllocLimit) - { - // Multiple buffers and < stackAllocLimit, copy into a stack buffer - byte* stackBuffer = stackalloc byte[startLineBuffer.Length]; - span = new Span(stackBuffer, startLineBuffer.Length); - startLineBuffer.CopyTo(span); - } else { // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case @@ -242,7 +231,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var query = span.Slice(queryStart, queryEnd - queryStart); handler.OnStartLine(method, httpVersion, targetBuffer, pathBuffer, query, customMethod); - consumed = buffer.Move(start, i); + + consumed = end; examined = consumed; return true; } @@ -297,8 +287,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return false; } - const int stackAllocLimit = 512; - if (lineEnd != bufferEnd) { lineEnd = buffer.Move(lineEnd, 1); @@ -312,13 +300,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // No copies, directly use the one and only span span = headerBuffer.First.Span; } - else if (headerBuffer.Length < stackAllocLimit) - { - // Multiple buffers and < stackAllocLimit, copy into a stack buffer - byte* stackBuffer = stackalloc byte[headerBuffer.Length]; - span = new Span(stackBuffer, headerBuffer.Length); - headerBuffer.CopyTo(span); - } else { // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case From 40ee51846cb59d04c194642c3770e18cf5f07e87 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 2 Mar 2017 14:56:05 +0000 Subject: [PATCH 04/28] Add allocations column (#1422) --- .../configs/CoreConfig.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/configs/CoreConfig.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/configs/CoreConfig.cs index 1621a1ddc2..01f4c02318 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/configs/CoreConfig.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/configs/CoreConfig.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Engines; using BenchmarkDotNet.Jobs; using BenchmarkDotNet.Validators; @@ -13,6 +14,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public CoreConfig() { Add(JitOptimizationsValidator.FailOnError); + Add(MemoryDiagnoser.Default); Add(new RpsColumn()); Add(Job.Default From aaea173cbaa72b1c373349f6058fd6083e7e0ad7 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Wed, 1 Mar 2017 18:25:45 -0800 Subject: [PATCH 05/28] Update AppVeyor and Travis settings --- .travis.yml | 2 +- appveyor.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 33d8f34d20..4d388988b4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,5 +32,5 @@ branches: before_install: - if test "$TRAVIS_OS_NAME" == "osx"; then brew update; brew install openssl; ln -s /usr/local/opt/openssl/lib/libcrypto.1.0.0.dylib /usr/local/lib/; ln -s /usr/local/opt/openssl/lib/libssl.1.0.0.dylib /usr/local/lib/; fi script: - - ./build.sh --quiet verify + - ./build.sh - if test "$TRAVIS_OS_NAME" != "osx"; then bash test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/SystemdActivation/docker.sh; fi diff --git a/appveyor.yml b/appveyor.yml index 61e1ba06b7..d622af030a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,7 +8,7 @@ branches: - feature/dev-si - /^(.*\/)?ci-.*$/ build_script: - - build.cmd --quiet verify + - ps: .\build.ps1 clone_depth: 1 test: off deploy: off From 8929b405277e42060e85ec52d6839add4ad6197b Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 2 Mar 2017 12:17:39 -0800 Subject: [PATCH 06/28] Single span optimizations (#1421) - Added a fast path for single span in the start line parsing - Added a fast path for single span header parsing - Changed the out header loop to be pointer based (instead of slicing) --- .../Internal/Http/KestrelHttpParser.cs | 424 ++++++++++++------ .../RequestParsing.cs | 4 +- 2 files changed, 284 insertions(+), 144 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 18477d28dd..a92c330b05 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Buffers; using System.IO.Pipelines; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.Extensions.Logging; @@ -31,27 +32,46 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http consumed = buffer.Start; examined = buffer.End; - var start = buffer.Start; - if (ReadCursorOperations.Seek(start, buffer.End, out var end, ByteLF) == -1) - { - return false; - } - - // Move 1 byte past the \n - end = buffer.Move(end, 1); - var startLineBuffer = buffer.Slice(start, end); - + ReadCursor end; Span span; - if (startLineBuffer.IsSingleSpan) + + // If the buffer is a single span then use it to find the LF + if (buffer.IsSingleSpan) { - // No copies, directly use the one and only span - span = startLineBuffer.First.Span; + var startLineSpan = buffer.First.Span; + var lineIndex = startLineSpan.IndexOfVectorized(ByteLF); + + if (lineIndex == -1) + { + return false; + } + + end = buffer.Move(consumed, lineIndex + 1); + span = startLineSpan.Slice(0, lineIndex + 1); } else { - // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case - span = new Span(new byte[startLineBuffer.Length]); - startLineBuffer.CopyTo(span); + var start = buffer.Start; + if (ReadCursorOperations.Seek(start, buffer.End, out end, ByteLF) == -1) + { + return false; + } + + // Move 1 byte past the \n + end = buffer.Move(end, 1); + var startLineBuffer = buffer.Slice(start, end); + + if (startLineBuffer.IsSingleSpan) + { + // No copies, directly use the one and only span + span = startLineBuffer.First.Span; + } + else + { + // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case + span = new Span(new byte[startLineBuffer.Length]); + startLineBuffer.CopyTo(span); + } } var pathStart = -1; @@ -243,6 +263,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http examined = buffer.End; consumedBytes = 0; + if (buffer.IsSingleSpan) + { + var result = TakeMessageHeadersSingleSpan(handler, buffer.First.Span, out consumedBytes); + consumed = buffer.Move(consumed, consumedBytes); + + if (result) + { + examined = consumed; + } + + return result; + } + var bufferEnd = buffer.End; var reader = new ReadableBufferReader(buffer); @@ -307,135 +340,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http headerBuffer.CopyTo(span); } - var nameStart = 0; - var nameEnd = -1; - var valueStart = -1; - var valueEnd = -1; - var nameHasWhitespace = false; - var previouslyWhitespace = false; - var headerLineLength = span.Length; - - int i = 0; - var length = span.Length; - bool done = false; - fixed (byte* data = &span.DangerousGetPinnableReference()) - { - switch (HeaderState.Name) - { - case HeaderState.Name: - for (; i < length; i++) - { - var ch = data[i]; - if (ch == ByteColon) - { - if (nameHasWhitespace) - { - RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); - } - nameEnd = i; - - // Consume space - i++; - - goto case HeaderState.Whitespace; - } - - if (ch == ByteSpace || ch == ByteTab) - { - nameHasWhitespace = true; - } - } - RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); - - break; - case HeaderState.Whitespace: - for (; i < length; i++) - { - var ch = data[i]; - var whitespace = ch == ByteTab || ch == ByteSpace || ch == ByteCR; - - if (!whitespace) - { - // Mark the first non whitespace char as the start of the - // header value and change the state to expect to the header value - valueStart = i; - - goto case HeaderState.ExpectValue; - } - // If we see a CR then jump to the next state directly - else if (ch == ByteCR) - { - goto case HeaderState.ExpectValue; - } - } - - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); - - break; - case HeaderState.ExpectValue: - for (; i < length; i++) - { - var ch = data[i]; - var whitespace = ch == ByteTab || ch == ByteSpace; - - if (whitespace) - { - if (!previouslyWhitespace) - { - // If we see a whitespace char then maybe it's end of the - // header value - valueEnd = i; - } - } - else if (ch == ByteCR) - { - // If we see a CR and we haven't ever seen whitespace then - // this is the end of the header value - if (valueEnd == -1) - { - valueEnd = i; - } - - // We never saw a non whitespace character before the CR - if (valueStart == -1) - { - valueStart = valueEnd; - } - - // Consume space - i++; - - goto case HeaderState.ExpectNewLine; - } - else - { - // If we find a non whitespace char that isn't CR then reset the end index - valueEnd = -1; - } - - previouslyWhitespace = whitespace; - } - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); - break; - case HeaderState.ExpectNewLine: - if (data[i] != ByteLF) - { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); - } - goto case HeaderState.Complete; - case HeaderState.Complete: - done = true; - break; - } - } - - if (!done) + if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) { return false; } // Skip the reader forward past the header line - reader.Skip(headerLineLength); + reader.Skip(span.Length); // Before accepting the header line, we need to see at least one character // > so we can make sure there's no space or tab @@ -473,13 +384,244 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); - consumedBytes += headerLineLength; + consumedBytes += span.Length; handler.OnHeader(nameBuffer, valueBuffer); + consumed = reader.Cursor; } } + private unsafe bool TakeMessageHeadersSingleSpan(T handler, Span headersSpan, out int consumedBytes) where T : IHttpHeadersHandler + { + consumedBytes = 0; + + var remaining = headersSpan.Length; + var index = 0; + fixed (byte* data = &headersSpan.DangerousGetPinnableReference()) + { + while (true) + { + if (remaining < 2) + { + return false; + } + + var ch1 = data[index]; + var ch2 = data[index + 1]; + + if (ch1 == ByteCR) + { + // Check for final CRLF. + if (ch2 == ByteLF) + { + consumedBytes += 2; + return true; + } + + // Headers don't end in CRLF line. + RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); + } + else if (ch1 == ByteSpace || ch1 == ByteTab) + { + RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); + } + + var endOfLineIndex = IndexOf(data, index, headersSpan.Length, ByteLF); + + if (endOfLineIndex == -1) + { + return false; + } + + var span = new Span(data + index, (endOfLineIndex - index + 1)); + index += span.Length; + + if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) + { + return false; + } + + if ((endOfLineIndex + 1) >= headersSpan.Length) + { + return false; + } + + // Before accepting the header line, we need to see at least one character + // > so we can make sure there's no space or tab + var next = data[index]; + + if (next == ByteSpace || next == ByteTab) + { + // From https://tools.ietf.org/html/rfc7230#section-3.2.4: + // + // Historically, HTTP header field values could be extended over + // multiple lines by preceding each extra line with at least one space + // or horizontal tab (obs-fold). This specification deprecates such + // line folding except within the message/http media type + // (Section 8.3.1). A sender MUST NOT generate a message that includes + // line folding (i.e., that has any field-value that contains a match to + // the obs-fold rule) unless the message is intended for packaging + // within the message/http media type. + // + // A server that receives an obs-fold in a request message that is not + // within a message/http container MUST either reject the message by + // sending a 400 (Bad Request), preferably with a representation + // explaining that obsolete line folding is unacceptable, or replace + // each received obs-fold with one or more SP octets prior to + // interpreting the field value or forwarding the message downstream. + RejectRequest(RequestRejectionReason.HeaderValueLineFoldingNotSupported); + } + + var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); + var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); + + handler.OnHeader(nameBuffer, valueBuffer); + + consumedBytes += span.Length; + remaining -= span.Length; + } + } + } + + private unsafe int IndexOf(byte* data, int index, int length, byte value) + { + for (int i = index; i < length; i++) + { + if (data[i] == value) + { + return i; + } + } + return -1; + } + + private unsafe bool TakeSingleHeader(Span span, out int nameStart, out int nameEnd, out int valueStart, out int valueEnd) + { + nameStart = 0; + nameEnd = -1; + valueStart = -1; + valueEnd = -1; + var headerLineLength = span.Length; + var nameHasWhitespace = false; + var previouslyWhitespace = false; + + int i = 0; + var done = false; + fixed (byte* data = &span.DangerousGetPinnableReference()) + { + switch (HeaderState.Name) + { + case HeaderState.Name: + for (; i < headerLineLength; i++) + { + var ch = data[i]; + if (ch == ByteColon) + { + if (nameHasWhitespace) + { + RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); + } + nameEnd = i; + + // Consume space + i++; + + goto case HeaderState.Whitespace; + } + + if (ch == ByteSpace || ch == ByteTab) + { + nameHasWhitespace = true; + } + } + RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); + + break; + case HeaderState.Whitespace: + for (; i < headerLineLength; i++) + { + var ch = data[i]; + var whitespace = ch == ByteTab || ch == ByteSpace || ch == ByteCR; + + if (!whitespace) + { + // Mark the first non whitespace char as the start of the + // header value and change the state to expect to the header value + valueStart = i; + + goto case HeaderState.ExpectValue; + } + // If we see a CR then jump to the next state directly + else if (ch == ByteCR) + { + goto case HeaderState.ExpectValue; + } + } + + RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + + break; + case HeaderState.ExpectValue: + for (; i < headerLineLength; i++) + { + var ch = data[i]; + var whitespace = ch == ByteTab || ch == ByteSpace; + + if (whitespace) + { + if (!previouslyWhitespace) + { + // If we see a whitespace char then maybe it's end of the + // header value + valueEnd = i; + } + } + else if (ch == ByteCR) + { + // If we see a CR and we haven't ever seen whitespace then + // this is the end of the header value + if (valueEnd == -1) + { + valueEnd = i; + } + + // We never saw a non whitespace character before the CR + if (valueStart == -1) + { + valueStart = valueEnd; + } + + // Consume space + i++; + + goto case HeaderState.ExpectNewLine; + } + else + { + // If we find a non whitespace char that isn't CR then reset the end index + valueEnd = -1; + } + + previouslyWhitespace = whitespace; + } + RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + break; + case HeaderState.ExpectNewLine: + if (data[i] != ByteLF) + { + RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + } + goto case HeaderState.Complete; + case HeaderState.Complete: + done = true; + break; + } + } + + return done; + } + private static bool IsValidTokenChar(char c) { // Determines if a character is valid as a 'token' as defined in the diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 9d91d8e681..41c7010c48 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -143,9 +143,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance Frame.Reset(); - ReadCursor consumed; - ReadCursor examined; - if (!Frame.TakeStartLine(readableBuffer, out consumed, out examined)) + if (!Frame.TakeStartLine(readableBuffer, out var consumed, out var examined)) { ThrowInvalidStartLine(); } From 1e0f2b3951c89359da051ee969283d7dd6bba46c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 2 Mar 2017 17:26:41 -0800 Subject: [PATCH 07/28] Fix systemd activation tests (#1429) --- .../SystemdActivation/docker.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/SystemdActivation/docker.sh b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/SystemdActivation/docker.sh index b450ff3e54..7075a37d8c 100755 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/SystemdActivation/docker.sh +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/SystemdActivation/docker.sh @@ -2,13 +2,8 @@ set -e -# Ensure that dotnet is added to the PATH. -# build.sh should always be run before this script to create the .build/ directory and restore packages. scriptDir=$(dirname "${BASH_SOURCE[0]}") -repoDir=$(cd $scriptDir/../../.. && pwd) -source ./.build/KoreBuild.sh -r $repoDir --quiet - -dotnet publish -f netcoreapp1.1 ./samples/SampleApp/ +~/.dotnet/dotnet publish -f netcoreapp1.1 ./samples/SampleApp/ cp -R ./samples/SampleApp/bin/Debug/netcoreapp1.1/publish/ $scriptDir cp -R ~/.dotnet/ $scriptDir From 4d7c6ff69fcf147d83e5b44501f3edb0e29695bc Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 2 Mar 2017 21:40:20 -0800 Subject: [PATCH 08/28] use rtm appveyor --- NuGet.config | 1 - .../Microsoft.AspNetCore.Server.Kestrel.Performance.csproj | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/NuGet.config b/NuGet.config index 69bab2da9a..93f1ac47df 100644 --- a/NuGet.config +++ b/NuGet.config @@ -3,7 +3,6 @@ - diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj index 4e4fabb7a6..746c59cd10 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj @@ -22,7 +22,7 @@ - + From 4533383612fccc324337528f50ce324d7fc10d27 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 2 Mar 2017 23:09:13 -0800 Subject: [PATCH 09/28] React to hosting change - Kestrel binds to ipv4 by default Fixes #1432 --- .../AddressRegistrationTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs index 00bd4a0826..9e011adda4 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs @@ -371,8 +371,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var dataset = new TheoryData>(); // Default host and port - dataset.Add(null, _ => new[] { "http://127.0.0.1:5000/", "http://[::1]:5000/" }); - dataset.Add(string.Empty, _ => new[] { "http://127.0.0.1:5000/", "http://[::1]:5000/" }); + dataset.Add(null, _ => new[] { "http://127.0.0.1:5000/", /*"http://[::1]:5000/"*/ }); + dataset.Add(string.Empty, _ => new[] { "http://127.0.0.1:5000/", /*"http://[::1]:5000/"*/ }); // Static ports var port = GetNextPort(); From 1d685e195ef57c209bffd45cc4dbc88f30fd5deb Mon Sep 17 00:00:00 2001 From: David Fowler Date: Fri, 3 Mar 2017 10:04:44 -0800 Subject: [PATCH 10/28] Parser cleanup and remove line continuation header error (#1431) - Cleaned up some parsing logic (removed locals etc) - Removing line continuation errors cleaned up code duplication a little bit --- .../Internal/Http/KestrelHttpParser.cs | 141 ++++-------------- .../Internal/Http/PipelineExtensions.cs | 2 + .../FrameTests.cs | 2 +- test/shared/HttpParsingData.cs | 2 +- 4 files changed, 31 insertions(+), 116 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index a92c330b05..7ce4097985 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -61,17 +61,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http end = buffer.Move(end, 1); var startLineBuffer = buffer.Slice(start, end); - if (startLineBuffer.IsSingleSpan) - { - // No copies, directly use the one and only span - span = startLineBuffer.First.Span; - } - else - { - // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case - span = new Span(new byte[startLineBuffer.Length]); - startLineBuffer.CopyTo(span); - } + span = startLineBuffer.ToSpan(); } var pathStart = -1; @@ -80,10 +70,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var pathEnd = -1; var versionStart = -1; - HttpVersion httpVersion = HttpVersion.Unknown; + var httpVersion = HttpVersion.Unknown; HttpMethod method; Span customMethod; - int i = 0; + var i = 0; var length = span.Length; var done = false; @@ -320,25 +310,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return false; } - if (lineEnd != bufferEnd) - { - lineEnd = buffer.Move(lineEnd, 1); - } + // Make sure LF is included in lineEnd + lineEnd = buffer.Move(lineEnd, 1); var headerBuffer = buffer.Slice(consumed, lineEnd); - - Span span; - if (headerBuffer.IsSingleSpan) - { - // No copies, directly use the one and only span - span = headerBuffer.First.Span; - } - else - { - // We're not a single span here but we can use pooled arrays to avoid allocations in the rare case - span = new Span(new byte[headerBuffer.Length]); - headerBuffer.CopyTo(span); - } + var span = headerBuffer.ToSpan(); if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) { @@ -347,73 +323,41 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Skip the reader forward past the header line reader.Skip(span.Length); - - // Before accepting the header line, we need to see at least one character - // > so we can make sure there's no space or tab - var next = reader.Peek(); - - // TODO: We don't need to reject the line here, we can use the state machine - // to store the fact that we're reading a header value - if (next == -1) - { - // If we can't see the next char then reject the entire line - return false; - } - - if (next == ByteSpace || next == ByteTab) - { - // From https://tools.ietf.org/html/rfc7230#section-3.2.4: - // - // Historically, HTTP header field values could be extended over - // multiple lines by preceding each extra line with at least one space - // or horizontal tab (obs-fold). This specification deprecates such - // line folding except within the message/http media type - // (Section 8.3.1). A sender MUST NOT generate a message that includes - // line folding (i.e., that has any field-value that contains a match to - // the obs-fold rule) unless the message is intended for packaging - // within the message/http media type. - // - // A server that receives an obs-fold in a request message that is not - // within a message/http container MUST either reject the message by - // sending a 400 (Bad Request), preferably with a representation - // explaining that obsolete line folding is unacceptable, or replace - // each received obs-fold with one or more SP octets prior to - // interpreting the field value or forwarding the message downstream. - RejectRequest(RequestRejectionReason.HeaderValueLineFoldingNotSupported); - } + consumed = reader.Cursor; + consumedBytes += span.Length; var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); - consumedBytes += span.Length; handler.OnHeader(nameBuffer, valueBuffer); - - consumed = reader.Cursor; } } private unsafe bool TakeMessageHeadersSingleSpan(T handler, Span headersSpan, out int consumedBytes) where T : IHttpHeadersHandler { consumedBytes = 0; + var length = headersSpan.Length; - var remaining = headersSpan.Length; - var index = 0; fixed (byte* data = &headersSpan.DangerousGetPinnableReference()) { - while (true) + while (consumedBytes < length) { - if (remaining < 2) - { - return false; - } + var ch1 = data[consumedBytes]; + var ch2 = -1; - var ch1 = data[index]; - var ch2 = data[index + 1]; + if (consumedBytes + 1 < length) + { + ch2 = data[consumedBytes + 1]; + } if (ch1 == ByteCR) { // Check for final CRLF. - if (ch2 == ByteLF) + if (ch2 == -1) + { + return false; + } + else if (ch2 == ByteLF) { consumedBytes += 2; return true; @@ -427,61 +371,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); } - var endOfLineIndex = IndexOf(data, index, headersSpan.Length, ByteLF); + var lineEnd = IndexOf(data, consumedBytes, headersSpan.Length, ByteLF); - if (endOfLineIndex == -1) + if (lineEnd == -1) { return false; } - var span = new Span(data + index, (endOfLineIndex - index + 1)); - index += span.Length; + var span = new Span(data + consumedBytes, (lineEnd - consumedBytes + 1)); if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) { return false; } - if ((endOfLineIndex + 1) >= headersSpan.Length) - { - return false; - } - - // Before accepting the header line, we need to see at least one character - // > so we can make sure there's no space or tab - var next = data[index]; - - if (next == ByteSpace || next == ByteTab) - { - // From https://tools.ietf.org/html/rfc7230#section-3.2.4: - // - // Historically, HTTP header field values could be extended over - // multiple lines by preceding each extra line with at least one space - // or horizontal tab (obs-fold). This specification deprecates such - // line folding except within the message/http media type - // (Section 8.3.1). A sender MUST NOT generate a message that includes - // line folding (i.e., that has any field-value that contains a match to - // the obs-fold rule) unless the message is intended for packaging - // within the message/http media type. - // - // A server that receives an obs-fold in a request message that is not - // within a message/http container MUST either reject the message by - // sending a 400 (Bad Request), preferably with a representation - // explaining that obsolete line folding is unacceptable, or replace - // each received obs-fold with one or more SP octets prior to - // interpreting the field value or forwarding the message downstream. - RejectRequest(RequestRejectionReason.HeaderValueLineFoldingNotSupported); - } + consumedBytes += span.Length; var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); handler.OnHeader(nameBuffer, valueBuffer); - - consumedBytes += span.Length; - remaining -= span.Length; } } + + return false; } private unsafe int IndexOf(byte* data, int index, int length, byte value) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs index c7a92e7639..a37670cfa6 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs @@ -3,6 +3,7 @@ using System; using System.IO.Pipelines; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; @@ -77,6 +78,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return result; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span ToSpan(this ReadableBuffer buffer) { if (buffer.IsSingleSpan) diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index f4b4339479..0f3185e6c8 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -199,7 +199,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); - Assert.Equal("Header value line folding not supported.", exception.Message); + Assert.Equal("Header line must not start with whitespace.", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index b38afb96c5..a5a83c58c6 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -287,7 +287,7 @@ namespace Microsoft.AspNetCore.Testing return new[] { - Tuple.Create(headersWithLineFolding,"Header value line folding not supported."), + Tuple.Create(headersWithLineFolding,"Header line must not start with whitespace."), Tuple.Create(headersWithCRInValue,"Header value must not contain CR characters."), Tuple.Create(headersWithMissingColon,"No ':' character found in header line."), Tuple.Create(headersStartingWithWhitespace, "Header line must not start with whitespace."), From 7cc5c537a42634c13e45705799098da06bc79a6c Mon Sep 17 00:00:00 2001 From: Krzysztof Cwalina Date: Thu, 2 Mar 2017 08:41:44 -0800 Subject: [PATCH 11/28] Added a new benchmark --- .../RequestParsing.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 9d91d8e681..7dffa9c1f4 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -23,6 +23,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private const string plaintextRequest = "GET /plaintext HTTP/1.1\r\nHost: www.example.com\r\n\r\n"; + private const string plaintextTechEmpower = "GET /plaintext HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n" + + "Connection: keep-alive\r\n\r\n"; + private const string liveaspnetRequest = "GET https://live.asp.net/ HTTP/1.1\r\n" + "Host: live.asp.net\r\n" + "Connection: keep-alive\r\n" + @@ -51,7 +56,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private static readonly byte[] _plaintextPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(plaintextRequest, Pipelining))); private static readonly byte[] _plaintextRequest = Encoding.ASCII.GetBytes(plaintextRequest); - + private static readonly byte[] _plaintextTechEmpower = Encoding.ASCII.GetBytes(plaintextTechEmpower); + private static readonly byte[] _liveaspnentPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(liveaspnetRequest, Pipelining))); private static readonly byte[] _liveaspnentRequest = Encoding.ASCII.GetBytes(liveaspnetRequest); @@ -71,6 +77,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } } + [Benchmark(OperationsPerInvoke = InnerLoopCount)] + public void ParsePlaintextTechEmpower() + { + for (var i = 0; i < InnerLoopCount; i++) { + InsertData(_plaintextTechEmpower); + ParseData(); + } + } + [Benchmark(OperationsPerInvoke = InnerLoopCount * Pipelining)] public void ParsePipelinedPlaintext() { From ac60f13312b0776cbffe333e72ce521211845b89 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 3 Mar 2017 14:43:32 -0800 Subject: [PATCH 12/28] Modify RequestProcessingAsync to call single parse method (#1427) * Modify RequestProcessingAsync to call single parse method * Fix bad request logging --- .../BadHttpRequestException.cs | 3 + .../Internal/Http/Frame.cs | 77 ++++++++++-------- .../Internal/Http/FrameOfT.cs | 79 +++++-------------- .../Internal/Http/KestrelHttpParser.cs | 21 ++--- .../Internal/Http/MessageBody.cs | 2 +- .../Internal/Http/RequestProcessingStatus.cs | 14 ++++ .../RequestParsing.cs | 6 +- .../FrameTests.cs | 32 ++++---- 8 files changed, 111 insertions(+), 123 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestProcessingStatus.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index f7109710c6..a19fd6db35 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs @@ -40,6 +40,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel case RequestRejectionReason.HeaderValueLineFoldingNotSupported: ex = new BadHttpRequestException("Header value line folding not supported.", StatusCodes.Status400BadRequest); break; + case RequestRejectionReason.InvalidRequestLine: + ex = new BadHttpRequestException("Invalid request line.", StatusCodes.Status400BadRequest); + break; case RequestRejectionReason.MalformedRequestInvalidHeaders: ex = new BadHttpRequestException("Malformed request: invalid headers.", StatusCodes.Status400BadRequest); break; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index c87c72c468..e059ce638f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; - private RequestProcessingStatus _requestProcessingStatus; + protected RequestProcessingStatus _requestProcessingStatus; protected bool _keepAlive; protected bool _upgrade; private bool _canHaveBody; @@ -982,15 +982,46 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Output.ProducingComplete(end); } + public void ParseRequest(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) + { + consumed = buffer.Start; + examined = buffer.End; + + switch (_requestProcessingStatus) + { + case RequestProcessingStatus.RequestPending: + if (buffer.IsEmpty) + { + break; + } + + ConnectionControl.ResetTimeout(_requestHeadersTimeoutMilliseconds, TimeoutAction.SendTimeoutResponse); + + _requestProcessingStatus = RequestProcessingStatus.ParsingRequestLine; + goto case RequestProcessingStatus.ParsingRequestLine; + case RequestProcessingStatus.ParsingRequestLine: + if (TakeStartLine(buffer, out consumed, out examined)) + { + buffer = buffer.Slice(consumed, buffer.End); + + _requestProcessingStatus = RequestProcessingStatus.ParsingHeaders; + goto case RequestProcessingStatus.ParsingHeaders; + } + else + { + break; + } + case RequestProcessingStatus.ParsingHeaders: + if (TakeMessageHeaders(buffer, out consumed, out examined)) + { + _requestProcessingStatus = RequestProcessingStatus.AppStarted; + } + break; + } + } + public bool TakeStartLine(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) { - if (_requestProcessingStatus == RequestProcessingStatus.RequestPending) - { - ConnectionControl.ResetTimeout(_requestHeadersTimeoutMilliseconds, TimeoutAction.SendTimeoutResponse); - } - - _requestProcessingStatus = RequestProcessingStatus.RequestStarted; - var overLength = false; if (buffer.Length >= ServerOptions.Limits.MaxRequestLineSize) { @@ -1039,7 +1070,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return true; } - public bool TakeMessageHeaders(ReadableBuffer buffer, FrameRequestHeaders requestHeaders, out ReadCursor consumed, out ReadCursor examined) + public bool TakeMessageHeaders(ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) { // Make sure the buffer is limited bool overLength = false; @@ -1085,7 +1116,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (!appCompleted) { // Back out of header creation surface exeception in user code - _requestProcessingStatus = RequestProcessingStatus.RequestStarted; + _requestProcessingStatus = RequestProcessingStatus.AppStarted; throw ex; } else @@ -1141,18 +1172,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http public void RejectRequest(RequestRejectionReason reason) { - RejectRequest(BadHttpRequestException.GetException(reason)); + throw BadHttpRequestException.GetException(reason); } public void RejectRequest(RequestRejectionReason reason, string value) { - RejectRequest(BadHttpRequestException.GetException(reason, value)); - } - - private void RejectRequest(BadHttpRequestException ex) - { - Log.ConnectionBadRequest(ConnectionId, ex); - throw ex; + throw BadHttpRequestException.GetException(reason, value); } public void SetBadRequestState(RequestRejectionReason reason) @@ -1162,6 +1187,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http public void SetBadRequestState(BadHttpRequestException ex) { + Log.ConnectionBadRequest(ConnectionId, ex); + if (!HasResponseStarted) { SetErrorResponseHeaders(ex.StatusCode); @@ -1190,20 +1217,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Log.ApplicationError(ConnectionId, ex); } - public enum RequestLineStatus - { - Empty, - Incomplete, - Done - } - - private enum RequestProcessingStatus - { - RequestPending, - RequestStarted, - ResponseStarted - } - public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod) { // URIs are always encoded/escaped to ASCII https://tools.ietf.org/html/rfc3986#page-11 diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs index 23626c47ac..bf9d21e7ab 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting.Server; @@ -31,16 +30,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http /// public override async Task RequestProcessingAsync() { - var requestLineStatus = default(RequestLineStatus); - try { while (!_requestProcessingStopping) { - // If writer completes with an error Input.ReadAsyncDispatched would throw and - // this would not be reset to empty. But it's required by ECONNRESET check lower in the method. - requestLineStatus = RequestLineStatus.Empty; - ConnectionControl.SetTimeout(_keepAliveMilliseconds, TimeoutAction.CloseConnection); while (!_requestProcessingStopping) @@ -49,76 +42,44 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var examined = result.Buffer.End; var consumed = result.Buffer.End; + InitializeHeaders(); + try { - if (!result.Buffer.IsEmpty) - { - requestLineStatus = TakeStartLine(result.Buffer, out consumed, out examined) - ? RequestLineStatus.Done : RequestLineStatus.Incomplete; - } - else - { - requestLineStatus = RequestLineStatus.Empty; - } + ParseRequest(result.Buffer, out consumed, out examined); } catch (InvalidOperationException) { - throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + switch (_requestProcessingStatus) + { + case RequestProcessingStatus.ParsingRequestLine: + throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + case RequestProcessingStatus.ParsingHeaders: + throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders); + } + throw; } finally { Input.Reader.Advance(consumed, examined); } - if (requestLineStatus == RequestLineStatus.Done) + if (_requestProcessingStatus == RequestProcessingStatus.AppStarted) { break; } if (result.IsCompleted) { - if (requestLineStatus == RequestLineStatus.Empty) + switch (_requestProcessingStatus) { - return; + case RequestProcessingStatus.RequestPending: + return; + case RequestProcessingStatus.ParsingRequestLine: + throw BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine); + case RequestProcessingStatus.ParsingHeaders: + throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders); } - - RejectRequest(RequestRejectionReason.InvalidRequestLine, requestLineStatus.ToString()); - } - } - - InitializeHeaders(); - - while (!_requestProcessingStopping) - { - - var result = await Input.Reader.ReadAsync(); - var examined = result.Buffer.End; - var consumed = result.Buffer.End; - - bool headersDone; - - try - { - headersDone = TakeMessageHeaders(result.Buffer, FrameRequestHeaders, out consumed, - out examined); - } - catch (InvalidOperationException) - { - throw BadHttpRequestException.GetException(RequestRejectionReason.MalformedRequestInvalidHeaders); - } - finally - { - Input.Reader.Advance(consumed, examined); - } - - if (headersDone) - { - break; - } - - if (result.IsCompleted) - { - RejectRequest(RequestRejectionReason.MalformedRequestInvalidHeaders); } } @@ -231,7 +192,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http catch (IOException ex) when (ex.InnerException is UvException) { // Don't log ECONNRESET errors made between requests. Browsers like IE will reset connections regularly. - if (requestLineStatus != RequestLineStatus.Empty || + if (_requestProcessingStatus != RequestProcessingStatus.RequestPending || ((UvException)ex.InnerException).StatusCode != Constants.ECONNRESET) { Log.RequestProcessingError(ConnectionId, ex); diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 7ce4097985..2e9947c9d8 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -289,7 +289,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } else if (ch2 == ByteLF) { - consumed = reader.Cursor; + // REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is + // sliced and doesn't start at the start of a segment. We should probably fix this. + //consumed = reader.Cursor; + consumed = buffer.Move(consumed, 2); examined = consumed; return true; } @@ -323,7 +326,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Skip the reader forward past the header line reader.Skip(span.Length); - consumed = reader.Cursor; + // REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is + // sliced and doesn't start at the start of a segment. We should probably fix this. + //consumed = reader.Cursor; + consumed = buffer.Move(consumed, span.Length); consumedBytes += span.Length; var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); @@ -562,17 +568,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http public void RejectRequest(RequestRejectionReason reason) { - RejectRequest(BadHttpRequestException.GetException(reason)); + throw BadHttpRequestException.GetException(reason); } public void RejectRequest(RequestRejectionReason reason, string value) { - RejectRequest(BadHttpRequestException.GetException(reason, value)); - } - - private void RejectRequest(BadHttpRequestException ex) - { - throw ex; + throw BadHttpRequestException.GetException(reason, value); } private void RejectRequestLine(Span span) @@ -608,4 +609,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http Complete } } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs index ce0edff4b9..8a817b82bb 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs @@ -595,7 +595,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http try { - if (_context.TakeMessageHeaders(buffer, _requestHeaders, out consumed, out examined)) + if (_context.TakeMessageHeaders(buffer, out consumed, out examined)) { break; } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestProcessingStatus.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestProcessingStatus.cs new file mode 100644 index 0000000000..e8ebb73784 --- /dev/null +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestProcessingStatus.cs @@ -0,0 +1,14 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http +{ + public enum RequestProcessingStatus + { + RequestPending, + ParsingRequestLine, + ParsingHeaders, + AppStarted, + ResponseStarted + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 41c7010c48..7080d93e3a 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -6,12 +6,8 @@ using System.IO.Pipelines; using System.Linq; using System.Text; using BenchmarkDotNet.Attributes; -using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; -using MemoryPool = Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure.MemoryPool; -using RequestLineStatus = Microsoft.AspNetCore.Server.Kestrel.Internal.Http.Frame.RequestLineStatus; namespace Microsoft.AspNetCore.Server.Kestrel.Performance { @@ -154,7 +150,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance Frame.InitializeHeaders(); - if (!Frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)Frame.RequestHeaders, out consumed, out examined)) + if (!Frame.TakeMessageHeaders(readableBuffer, out consumed, out examined)) { ThrowInvalidMessageHeaders(); } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 0f3185e6c8..0d4c59b374 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header:value\r\n\r\n")); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(success); @@ -114,7 +114,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(success); @@ -138,7 +138,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(success); @@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(success); @@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); + var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal(expectedExceptionMessage, exception.Message); @@ -190,13 +190,13 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n")); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - Assert.False(_frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); + Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(" ")); readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); + var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal("Header line must not start with whitespace.", exception.Message); @@ -213,7 +213,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine}\r\n")); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); + var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal("Request headers too long.", exception.Message); @@ -229,7 +229,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLines}\r\n")); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined)); + var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal("Request contains too many headers.", exception.Message); @@ -248,7 +248,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(success); @@ -282,7 +282,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine1}\r\n")); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + var takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(takeMessageHeaders); @@ -294,7 +294,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine2}\r\n")); readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.True(takeMessageHeaders); @@ -461,14 +461,14 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } [Fact] - public async Task TakeStartLineStartsRequestHeadersTimeoutOnFirstByteAvailable() + public async Task ParseRequestStartsRequestHeadersTimeoutOnFirstByteAvailable() { var connectionControl = new Mock(); _connectionContext.ConnectionControl = connectionControl.Object; await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("G")); - _frame.TakeStartLine((await _socketInput.Reader.ReadAsync()).Buffer, out _consumed, out _examined); + _frame.ParseRequest((await _socketInput.Reader.ReadAsync()).Buffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); var expectedRequestHeadersTimeout = (long)_serviceContext.ServerOptions.Limits.RequestHeadersTimeout.TotalMilliseconds; @@ -533,7 +533,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeader)); var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out _consumed, out _examined); + _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); _socketInput.Reader.Advance(_consumed, _examined); Assert.Equal(readableBuffer.End, _examined); } @@ -566,7 +566,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests ReadCursor examined; var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - Assert.Equal(false, _frame.TakeMessageHeaders(readableBuffer, (FrameRequestHeaders)_frame.RequestHeaders, out consumed, out examined)); + Assert.Equal(false, _frame.TakeMessageHeaders(readableBuffer, out consumed, out examined)); _socketInput.Reader.Advance(consumed, examined); } From 83edc38e722c924aafad146b5b366035b781757b Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Fri, 3 Mar 2017 15:55:07 -0800 Subject: [PATCH 13/28] Use TechEmpower request as baseline for request parsing benchmarks. --- .../RequestParsing.cs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index fb11720970..350a2a0000 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -17,8 +17,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private const int InnerLoopCount = 512; private const int Pipelining = 16; - private const string plaintextRequest = "GET /plaintext HTTP/1.1\r\nHost: www.example.com\r\n\r\n"; - private const string plaintextTechEmpower = "GET /plaintext HTTP/1.1\r\n" + "Host: localhost\r\n" + "Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n" + @@ -50,8 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance "Pragma: no-cache\r\n" + "Cookie: prov=20629ccd-8b0f-e8ef-2935-cd26609fc0bc; __qca=P0-1591065732-1479167353442; _ga=GA1.2.1298898376.1479167354; _gat=1; sgt=id=9519gfde_3347_4762_8762_df51458c8ec2; acct=t=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric&s=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric\r\n\r\n"; - private static readonly byte[] _plaintextPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(plaintextRequest, Pipelining))); - private static readonly byte[] _plaintextRequest = Encoding.ASCII.GetBytes(plaintextRequest); + private static readonly byte[] _plaintextTechEmpowerPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(plaintextTechEmpower, Pipelining))); private static readonly byte[] _plaintextTechEmpower = Encoding.ASCII.GetBytes(plaintextTechEmpower); private static readonly byte[] _liveaspnentPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(liveaspnetRequest, Pipelining))); @@ -64,16 +61,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance public Type ParserType { get; set; } [Benchmark(Baseline = true, OperationsPerInvoke = InnerLoopCount)] - public void ParsePlaintext() - { - for (var i = 0; i < InnerLoopCount; i++) - { - InsertData(_plaintextRequest); - ParseData(); - } - } - - [Benchmark(OperationsPerInvoke = InnerLoopCount)] public void ParsePlaintextTechEmpower() { for (var i = 0; i < InnerLoopCount; i++) { @@ -83,11 +70,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance } [Benchmark(OperationsPerInvoke = InnerLoopCount * Pipelining)] - public void ParsePipelinedPlaintext() + public void ParsePipelinedPlaintextTechEmpower() { for (var i = 0; i < InnerLoopCount; i++) { - InsertData(_plaintextPipelinedRequests); + InsertData(_plaintextTechEmpowerPipelinedRequests); ParseData(); } } From 20f75605ca776cfb66309c319c491ca2170c5292 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Fri, 3 Mar 2017 22:35:41 -0800 Subject: [PATCH 14/28] Workaround rogue System.IO.Pipelines on nuget.org --- build/dependencies.props | 3 +++ .../Microsoft.AspNetCore.Server.Kestrel.csproj | 7 +++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 9aa824128d..da2ebb0d94 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -3,5 +3,8 @@ 1.6.1 4.3.0 1.9.1 + + 0.1.0-e170301-1 + 0.1.0-e170228-1 diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj b/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj index 5dd7b984c9..35ccda46b3 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Microsoft.AspNetCore.Server.Kestrel.csproj @@ -18,10 +18,9 @@ - - - - + + + From da763b487383f8c56c3ee6e4febace161bfc0a0c Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sat, 4 Mar 2017 11:26:32 -0800 Subject: [PATCH 15/28] Use ascii decoding routine that disallows null chars (#1445) * Use ascii decoding routine that disallows null chars - GetAsciiString() in newer corefxlab builds allows 0 (which is a valid ascii char). To avoid future regressions, GetAsciiStringNonNullCharacters() was added and used in place of GetAsciiString() when interpreting the request line and headers - Make GetAsciiStringNonNullCharacters return empty instead of null --- .../Internal/Http/Frame.cs | 14 ++++++------ .../Internal/Infrastructure/HttpUtilities.cs | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index e059ce638f..463ce0a237 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -1228,7 +1228,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http if (needDecode) { // Read raw target before mutating memory. - rawTarget = target.GetAsciiString() ?? string.Empty; + rawTarget = target.GetAsciiStringNonNullCharacters(); // URI was encoded, unescape and then parse as utf8 int pathLength = UrlEncoder.Decode(path, path); @@ -1237,7 +1237,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http else { // URI wasn't encoded, parse as ASCII - requestUrlPath = path.GetAsciiString() ?? string.Empty; + requestUrlPath = path.GetAsciiStringNonNullCharacters(); if (query.Length == 0) { @@ -1247,21 +1247,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } else { - rawTarget = target.GetAsciiString() ?? string.Empty; + rawTarget = target.GetAsciiStringNonNullCharacters(); } } var normalizedTarget = PathNormalizer.RemoveDotSegments(requestUrlPath); if (method != HttpMethod.Custom) { - Method = HttpUtilities.MethodToString(method) ?? String.Empty; + Method = HttpUtilities.MethodToString(method) ?? string.Empty; } else { - Method = customMethod.GetAsciiString() ?? string.Empty; + Method = customMethod.GetAsciiStringNonNullCharacters(); } - QueryString = query.GetAsciiString() ?? string.Empty; + QueryString = query.GetAsciiStringNonNullCharacters(); RawTarget = rawTarget; HttpVersion = HttpUtilities.VersionToString(version); @@ -1289,7 +1289,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { RejectRequest(RequestRejectionReason.TooManyHeaders); } - var valueString = value.GetAsciiString() ?? string.Empty; + var valueString = value.GetAsciiStringNonNullCharacters(); FrameRequestHeaders.Append(name, valueString); } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs index 719e27eff4..e38ad9fd6e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/HttpUtilities.cs @@ -94,6 +94,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure } } + public unsafe static string GetAsciiStringNonNullCharacters(this Span span) + { + if (span.IsEmpty) + { + return string.Empty; + } + + var asciiString = new string('\0', span.Length); + + fixed (char* output = asciiString) + fixed (byte* buffer = &span.DangerousGetPinnableReference()) + { + // This version if AsciiUtilities returns null if there are any null (0 byte) characters + // in the string + if (!AsciiUtilities.TryGetAsciiString(buffer, output, span.Length)) + { + throw new InvalidOperationException(); + } + } + return asciiString; + } + public static string GetAsciiStringEscaped(this Span span, int maxChars) { var sb = new StringBuilder(); From acf97b610277019e33ece6ce99dfd652d0540bd3 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Sun, 5 Mar 2017 08:54:51 -0800 Subject: [PATCH 16/28] Unpin CoreFxLab package versions --- build/dependencies.props | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index da2ebb0d94..3c441802d9 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -3,8 +3,7 @@ 1.6.1 4.3.0 1.9.1 - - 0.1.0-e170301-1 - 0.1.0-e170228-1 + 0.1.0-* + 0.1.0-* From 0ce111d9f13bc873b609dd0f0e2494a4b4730936 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 6 Mar 2017 03:31:45 -0800 Subject: [PATCH 17/28] Fix write size in benchmark (#1449) --- .../RequestParsing.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 350a2a0000..444e4022b6 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -121,8 +121,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private void InsertData(byte[] bytes) { + var buffer = Pipe.Writer.Alloc(2048); + buffer.Write(bytes); // There should not be any backpressure and task completes immediately - Pipe.Writer.WriteAsync(bytes).GetAwaiter().GetResult(); + buffer.FlushAsync().GetAwaiter().GetResult(); } private void ParseData() From 537b06f025561578ed78de248e0654a745f69df5 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Mon, 6 Mar 2017 09:06:28 -0800 Subject: [PATCH 18/28] Interleave multispan and single span code path (#1442) - Attempt at making the reader better for multispan parsing - Try tighter inner loop - Fix boundary case and clean code up - Update the cursor once instead of after every header - Fix errors with not updating consumed state on incomplete header payload - Filled a test hole, removed a condition that should never happen - Avoid struct copies every iteration when parsing headers --- .../Internal/Http/KestrelHttpParser.cs | 277 +++++++++--------- .../FrameTests.cs | 16 + 2 files changed, 148 insertions(+), 145 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 2e9947c9d8..998aa6f46a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -253,163 +253,150 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http examined = buffer.End; consumedBytes = 0; - if (buffer.IsSingleSpan) - { - var result = TakeMessageHeadersSingleSpan(handler, buffer.First.Span, out consumedBytes); - consumed = buffer.Move(consumed, consumedBytes); - - if (result) - { - examined = consumed; - } - - return result; - } - var bufferEnd = buffer.End; var reader = new ReadableBufferReader(buffer); - while (true) + var start = default(ReadableBufferReader); + var done = false; + + try { - var start = reader; - int ch1 = reader.Take(); - var ch2 = reader.Take(); - - if (ch1 == -1) + while (!reader.End) { - return false; - } + var span = reader.Span; + var remaining = span.Length; - if (ch1 == ByteCR) - { - // Check for final CRLF. - if (ch2 == -1) + fixed (byte* pBuffer = &span.DangerousGetPinnableReference()) { - return false; + while (remaining > 0) + { + var index = reader.Index; + int ch1; + int ch2; + + // Fast path, we're still looking at the same span + if (remaining >= 2) + { + ch1 = pBuffer[index]; + ch2 = pBuffer[index + 1]; + } + else + { + // Store the reader before we look ahead 2 bytes (probably straddling + // spans) + start = reader; + + // Possibly split across spans + ch1 = reader.Take(); + ch2 = reader.Take(); + } + + if (ch1 == ByteCR) + { + // Check for final CRLF. + if (ch2 == -1) + { + // Reset the reader so we don't consume anything + reader = start; + return false; + } + else if (ch2 == ByteLF) + { + // If we got 2 bytes from the span directly so skip ahead 2 so that + // the reader's state matches what we expect + if (index == reader.Index) + { + reader.Skip(2); + } + + done = true; + return true; + } + + // Headers don't end in CRLF line. + RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); + } + else if (ch1 == ByteSpace || ch1 == ByteTab) + { + RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); + } + + // We moved the reader so look ahead 2 bytes so reset both the reader + // and the index + if (index != reader.Index) + { + reader = start; + index = reader.Index; + } + + Span headerSpan; + + var endIndex = IndexOf(pBuffer, index, remaining, ByteLF); + + if (endIndex != -1) + { + headerSpan = new Span(pBuffer + index, (endIndex - index + 1)); + } + else + { + var current = reader.Cursor; + + // Split buffers + if (ReadCursorOperations.Seek(current, bufferEnd, out var lineEnd, ByteLF) == -1) + { + // Not there + return false; + } + + // Make sure LF is included in lineEnd + lineEnd = buffer.Move(lineEnd, 1); + headerSpan = buffer.Slice(current, lineEnd).ToSpan(); + + // We're going to the next span after this since we know we crossed spans here + // so mark the remaining as equal to the headerSpan so that we end up at 0 + // on the next iteration + remaining = headerSpan.Length; + } + + if (!TakeSingleHeader(headerSpan, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) + { + return false; + } + + // Skip the reader forward past the header line + reader.Skip(headerSpan.Length); + + remaining -= headerSpan.Length; + + var nameBuffer = headerSpan.Slice(nameStart, nameEnd - nameStart); + var valueBuffer = headerSpan.Slice(valueStart, valueEnd - valueStart); + + handler.OnHeader(nameBuffer, valueBuffer); + } } - else if (ch2 == ByteLF) - { - // REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is - // sliced and doesn't start at the start of a segment. We should probably fix this. - //consumed = reader.Cursor; - consumed = buffer.Move(consumed, 2); - examined = consumed; - return true; - } - - // Headers don't end in CRLF line. - RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); } - else if (ch1 == ByteSpace || ch1 == ByteTab) + + return false; + } + finally + { + consumed = reader.Cursor; + consumedBytes = reader.ConsumedBytes; + + if (done) { - RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); + examined = consumed; } - - // Reset the reader since we're not at the end of headers - reader = start; - - if (ReadCursorOperations.Seek(consumed, bufferEnd, out var lineEnd, ByteLF) == -1) - { - return false; - } - - // Make sure LF is included in lineEnd - lineEnd = buffer.Move(lineEnd, 1); - - var headerBuffer = buffer.Slice(consumed, lineEnd); - var span = headerBuffer.ToSpan(); - - if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) - { - return false; - } - - // Skip the reader forward past the header line - reader.Skip(span.Length); - // REVIEW: Removed usage of ReadableBufferReader.Cursor, because it's broken when the buffer is - // sliced and doesn't start at the start of a segment. We should probably fix this. - //consumed = reader.Cursor; - consumed = buffer.Move(consumed, span.Length); - consumedBytes += span.Length; - - var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); - var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); - - handler.OnHeader(nameBuffer, valueBuffer); } } - private unsafe bool TakeMessageHeadersSingleSpan(T handler, Span headersSpan, out int consumedBytes) where T : IHttpHeadersHandler + private unsafe int IndexOf(byte* pBuffer, int index, int length, byte value) { - consumedBytes = 0; - var length = headersSpan.Length; - - fixed (byte* data = &headersSpan.DangerousGetPinnableReference()) + for (int i = 0; i < length; i++, index++) { - while (consumedBytes < length) + if (pBuffer[index] == value) { - var ch1 = data[consumedBytes]; - var ch2 = -1; - - if (consumedBytes + 1 < length) - { - ch2 = data[consumedBytes + 1]; - } - - if (ch1 == ByteCR) - { - // Check for final CRLF. - if (ch2 == -1) - { - return false; - } - else if (ch2 == ByteLF) - { - consumedBytes += 2; - return true; - } - - // Headers don't end in CRLF line. - RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); - } - else if (ch1 == ByteSpace || ch1 == ByteTab) - { - RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); - } - - var lineEnd = IndexOf(data, consumedBytes, headersSpan.Length, ByteLF); - - if (lineEnd == -1) - { - return false; - } - - var span = new Span(data + consumedBytes, (lineEnd - consumedBytes + 1)); - - if (!TakeSingleHeader(span, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) - { - return false; - } - - consumedBytes += span.Length; - - var nameBuffer = span.Slice(nameStart, nameEnd - nameStart); - var valueBuffer = span.Slice(valueStart, valueEnd - valueStart); - - handler.OnHeader(nameBuffer, valueBuffer); - } - } - - return false; - } - - private unsafe int IndexOf(byte* data, int index, int length, byte value) - { - for (int i = index; i < length; i++) - { - if (data[i] == value) - { - return i; + return index; } } return -1; @@ -427,14 +414,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http int i = 0; var done = false; - fixed (byte* data = &span.DangerousGetPinnableReference()) + fixed (byte* pHeader = &span.DangerousGetPinnableReference()) { switch (HeaderState.Name) { case HeaderState.Name: for (; i < headerLineLength; i++) { - var ch = data[i]; + var ch = pHeader[i]; if (ch == ByteColon) { if (nameHasWhitespace) @@ -460,7 +447,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http case HeaderState.Whitespace: for (; i < headerLineLength; i++) { - var ch = data[i]; + var ch = pHeader[i]; var whitespace = ch == ByteTab || ch == ByteSpace || ch == ByteCR; if (!whitespace) @@ -484,7 +471,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http case HeaderState.ExpectValue: for (; i < headerLineLength; i++) { - var ch = data[i]; + var ch = pHeader[i]; var whitespace = ch == ByteTab || ch == ByteSpace; if (whitespace) @@ -527,7 +514,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); break; case HeaderState.ExpectNewLine: - if (data[i] != ByteLF) + if (pHeader[i] != ByteLF) { RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 0d4c59b374..f48a614579 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -203,6 +203,22 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } + [Fact] + public async Task TakeMessageHeadersConsumesBytesCorrectlyAtEnd() + { + await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n\r")); + + var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); + _socketInput.Reader.Advance(_consumed, _examined); + + await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("\n")); + + readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + Assert.True(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); + _socketInput.Reader.Advance(_consumed, _examined); + } + [Fact] public async Task TakeMessageHeadersThrowsWhenHeadersExceedTotalSizeLimit() { From 0404bcc58c6de8740b16e393de325bf982eefaa4 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Wed, 1 Mar 2017 17:29:13 -0800 Subject: [PATCH 19/28] Add more microbenchmarks. --- .../FrameParsingOverhead.cs | 135 +++++++++++++++++ .../KestrelHttpParser.cs | 77 ++++++++++ .../Program.cs | 12 +- .../RequestParsing.cs | 142 ++++++------------ .../RequestParsingData.cs | 62 ++++++++ 5 files changed, 334 insertions(+), 94 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs new file mode 100644 index 0000000000..acb39238bc --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/FrameParsingOverhead.cs @@ -0,0 +1,135 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO.Pipelines; +using System.Text; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.AspNetCore.Testing; + +namespace Microsoft.AspNetCore.Server.Kestrel.Performance +{ + [Config(typeof(CoreConfig))] + public class FrameParsingOverhead + { + private const int InnerLoopCount = 512; + + public ReadableBuffer _buffer; + public Frame _frame; + + [Setup] + public void Setup() + { + var connectionContext = new MockConnection(new KestrelServerOptions()); + connectionContext.ListenerContext.ServiceContext.HttpParserFactory = frame => NullParser.Instance; + + _frame = new Frame(application: null, context: connectionContext); + } + + [Benchmark(Baseline = true, OperationsPerInvoke = InnerLoopCount)] + public void FrameOverheadTotal() + { + for (var i = 0; i < InnerLoopCount; i++) + { + ParseRequest(); + } + } + + [Benchmark(OperationsPerInvoke = InnerLoopCount)] + public void FrameOverheadRequestLine() + { + for (var i = 0; i < InnerLoopCount; i++) + { + ParseRequestLine(); + } + } + + [Benchmark(OperationsPerInvoke = InnerLoopCount)] + public void FrameOverheadRequestHeaders() + { + for (var i = 0; i < InnerLoopCount; i++) + { + ParseRequestHeaders(); + } + } + + private void ParseRequest() + { + _frame.Reset(); + + if (!_frame.TakeStartLine(_buffer, out var consumed, out var examined)) + { + RequestParsing.ThrowInvalidRequestLine(); + } + + _frame.InitializeHeaders(); + + if (!_frame.TakeMessageHeaders(_buffer, out consumed, out examined)) + { + RequestParsing.ThrowInvalidRequestHeaders(); + } + } + + private void ParseRequestLine() + { + _frame.Reset(); + + if (!_frame.TakeStartLine(_buffer, out var consumed, out var examined)) + { + RequestParsing.ThrowInvalidRequestLine(); + } + } + + private void ParseRequestHeaders() + { + _frame.Reset(); + _frame.InitializeHeaders(); + + if (!_frame.TakeMessageHeaders(_buffer, out var consumed, out var examined)) + { + RequestParsing.ThrowInvalidRequestHeaders(); + } + } + + private class NullParser : IHttpParser + { + private readonly byte[] _target = Encoding.ASCII.GetBytes("/plaintext"); + private readonly byte[] _hostHeaderName = Encoding.ASCII.GetBytes("Host"); + private readonly byte[] _hostHeaderValue = Encoding.ASCII.GetBytes("www.example.com"); + private readonly byte[] _acceptHeaderName = Encoding.ASCII.GetBytes("Accept"); + private readonly byte[] _acceptHeaderValue = Encoding.ASCII.GetBytes("text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n\r\n"); + private readonly byte[] _connectionHeaderName = Encoding.ASCII.GetBytes("Connection"); + private readonly byte[] _connectionHeaderValue = Encoding.ASCII.GetBytes("keep-alive"); + + public static readonly NullParser Instance = new NullParser(); + + public bool ParseHeaders(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined, out int consumedBytes) where T : IHttpHeadersHandler + { + handler.OnHeader(new Span(_hostHeaderName), new Span(_hostHeaderValue)); + handler.OnHeader(new Span(_acceptHeaderName), new Span(_acceptHeaderValue)); + handler.OnHeader(new Span(_connectionHeaderName), new Span(_connectionHeaderValue)); + + consumedBytes = 0; + consumed = buffer.Start; + examined = buffer.End; + + return true; + } + + public bool ParseRequestLine(T handler, ReadableBuffer buffer, out ReadCursor consumed, out ReadCursor examined) where T : IHttpRequestLineHandler + { + handler.OnStartLine(HttpMethod.Get, HttpVersion.Http11, new Span(_target), new Span(_target), Span.Empty, Span.Empty); + + consumed = buffer.Start; + examined = buffer.End; + + return true; + } + + public void Reset() + { + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs new file mode 100644 index 0000000000..f77346ee8b --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/KestrelHttpParser.cs @@ -0,0 +1,77 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO.Pipelines; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; + +namespace Microsoft.AspNetCore.Server.Kestrel.Performance +{ + [Config(typeof(CoreConfig))] + + public class KestrelHttpParser : IHttpRequestLineHandler, IHttpHeadersHandler + { + private readonly Internal.Http.KestrelHttpParser _parser = new Internal.Http.KestrelHttpParser(log: null); + + private ReadableBuffer _buffer; + + [Benchmark(Baseline = true, OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void PlaintextTechEmpower() + { + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) + { + InsertData(RequestParsingData.PlaintextTechEmpowerRequest); + ParseData(); + } + } + + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void LiveAspNet() + { + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) + { + InsertData(RequestParsingData.LiveaspnetRequest); + ParseData(); + } + } + + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void Unicode() + { + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) + { + InsertData(RequestParsingData.UnicodeRequest); + ParseData(); + } + } + + private void InsertData(byte[] data) + { + _buffer = ReadableBuffer.Create(data); + } + + private void ParseData() + { + if (!_parser.ParseRequestLine(this, _buffer, out var consumed, out var examined)) + { + RequestParsing.ThrowInvalidRequestHeaders(); + } + + _buffer = _buffer.Slice(consumed, _buffer.End); + + if (!_parser.ParseHeaders(this, _buffer, out consumed, out examined, out var consumedBytes)) + { + RequestParsing.ThrowInvalidRequestHeaders(); + } + } + + public void OnStartLine(HttpMethod method, HttpVersion version, Span target, Span path, Span query, Span customMethod) + { + } + + public void OnHeader(Span name, Span value) + { + } + } +} diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Program.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Program.cs index e6bea9e92c..95ca5909db 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Program.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Program.cs @@ -40,10 +40,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance { BenchmarkRunner.Run(); } - if (type.HasFlag(BenchmarkType.KnownStrings)) + if (type.HasFlag(BenchmarkType.KnownStrings)) { BenchmarkRunner.Run(); } + if (type.HasFlag(BenchmarkType.KestrelHttpParser)) + { + BenchmarkRunner.Run(); + } + if (type.HasFlag(BenchmarkType.FrameParsingOverhead)) + { + BenchmarkRunner.Run(); + } } } @@ -54,6 +62,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance Writing = 2, Throughput = 4, KnownStrings = 8, + KestrelHttpParser = 16, + FrameParsingOverhead = 32, // add new ones in powers of two - e.g. 2,4,8,16... All = uint.MaxValue diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 444e4022b6..90bd7a16c5 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -3,8 +3,6 @@ using System; using System.IO.Pipelines; -using System.Linq; -using System.Text; using BenchmarkDotNet.Attributes; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using Microsoft.AspNetCore.Testing; @@ -14,107 +12,82 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance [Config(typeof(CoreConfig))] public class RequestParsing { - private const int InnerLoopCount = 512; - private const int Pipelining = 16; - - private const string plaintextTechEmpower = "GET /plaintext HTTP/1.1\r\n" + - "Host: localhost\r\n" + - "Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n" + - "Connection: keep-alive\r\n\r\n"; - - private const string liveaspnetRequest = "GET https://live.asp.net/ HTTP/1.1\r\n" + - "Host: live.asp.net\r\n" + - "Connection: keep-alive\r\n" + - "Upgrade-Insecure-Requests: 1\r\n" + - "User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36\r\n" + - "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8\r\n" + - "DNT: 1\r\n" + - "Accept-Encoding: gzip, deflate, sdch, br\r\n" + - "Accept-Language: en-US,en;q=0.8\r\n" + - "Cookie: __unam=7a67379-1s65dc575c4-6d778abe-1; omniID=9519gfde_3347_4762_8762_df51458c8ec2\r\n\r\n"; - - private const string unicodeRequest = - "GET http://stackoverflow.com/questions/40148683/why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric HTTP/1.1\r\n" + - "Accept: text/html, application/xhtml+xml, image/jxr, */*\r\n" + - "Accept-Language: en-US,en-GB;q=0.7,en;q=0.3\r\n" + - "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Edge/15.14965\r\n" + - "Accept-Encoding: gzip, deflate\r\n" + - "Host: stackoverflow.com\r\n" + - "Connection: Keep-Alive\r\n" + - "Cache-Control: max-age=0\r\n" + - "Upgrade-Insecure-Requests: 1\r\n" + - "DNT: 1\r\n" + - "Referer: http://stackoverflow.com/?tab=month\r\n" + - "Pragma: no-cache\r\n" + - "Cookie: prov=20629ccd-8b0f-e8ef-2935-cd26609fc0bc; __qca=P0-1591065732-1479167353442; _ga=GA1.2.1298898376.1479167354; _gat=1; sgt=id=9519gfde_3347_4762_8762_df51458c8ec2; acct=t=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric&s=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric\r\n\r\n"; - - private static readonly byte[] _plaintextTechEmpowerPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(plaintextTechEmpower, Pipelining))); - private static readonly byte[] _plaintextTechEmpower = Encoding.ASCII.GetBytes(plaintextTechEmpower); - - private static readonly byte[] _liveaspnentPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(liveaspnetRequest, Pipelining))); - private static readonly byte[] _liveaspnentRequest = Encoding.ASCII.GetBytes(liveaspnetRequest); - - private static readonly byte[] _unicodePipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(unicodeRequest, Pipelining))); - private static readonly byte[] _unicodeRequest = Encoding.ASCII.GetBytes(unicodeRequest); - - [Params(typeof(KestrelHttpParser))] + [Params(typeof(Internal.Http.KestrelHttpParser))] public Type ParserType { get; set; } - [Benchmark(Baseline = true, OperationsPerInvoke = InnerLoopCount)] - public void ParsePlaintextTechEmpower() + public IPipe Pipe { get; set; } + + public Frame Frame { get; set; } + + public PipeFactory PipelineFactory { get; set; } + + [Setup] + public void Setup() { - for (var i = 0; i < InnerLoopCount; i++) { - InsertData(_plaintextTechEmpower); + var connectionContext = new MockConnection(new KestrelServerOptions()); + connectionContext.ListenerContext.ServiceContext.HttpParserFactory = frame => (IHttpParser)Activator.CreateInstance(ParserType, frame.ConnectionContext.ListenerContext.ServiceContext.Log); + + Frame = new Frame(application: null, context: connectionContext); + PipelineFactory = new PipeFactory(); + Pipe = PipelineFactory.Create(); + } + + [Benchmark(Baseline = true, OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void PlaintextTechEmpower() + { + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) + { + InsertData(RequestParsingData.PlaintextTechEmpowerRequest); ParseData(); } } - [Benchmark(OperationsPerInvoke = InnerLoopCount * Pipelining)] - public void ParsePipelinedPlaintextTechEmpower() + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount * RequestParsingData.Pipelining)] + public void PipelinedPlaintextTechEmpower() { - for (var i = 0; i < InnerLoopCount; i++) + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) { - InsertData(_plaintextTechEmpowerPipelinedRequests); + InsertData(RequestParsingData.PlaintextTechEmpowerPipelinedRequests); ParseData(); } } - [Benchmark(OperationsPerInvoke = InnerLoopCount)] - public void ParseLiveAspNet() + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void LiveAspNet() { - for (var i = 0; i < InnerLoopCount; i++) + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) { - InsertData(_liveaspnentRequest); + InsertData(RequestParsingData.LiveaspnetRequest); ParseData(); } } - [Benchmark(OperationsPerInvoke = InnerLoopCount * Pipelining)] - public void ParsePipelinedLiveAspNet() + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount * RequestParsingData.Pipelining)] + public void PipelinedLiveAspNet() { - for (var i = 0; i < InnerLoopCount; i++) + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) { - InsertData(_liveaspnentPipelinedRequests); + InsertData(RequestParsingData.LiveaspnetPipelinedRequests); ParseData(); } } - [Benchmark(OperationsPerInvoke = InnerLoopCount)] - public void ParseUnicode() + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount)] + public void Unicode() { - for (var i = 0; i < InnerLoopCount; i++) + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) { - InsertData(_unicodeRequest); + InsertData(RequestParsingData.UnicodeRequest); ParseData(); } } - [Benchmark(OperationsPerInvoke = InnerLoopCount * Pipelining)] - public void ParseUnicodePipelined() + [Benchmark(OperationsPerInvoke = RequestParsingData.InnerLoopCount * RequestParsingData.Pipelining)] + public void UnicodePipelined() { - for (var i = 0; i < InnerLoopCount; i++) + for (var i = 0; i < RequestParsingData.InnerLoopCount; i++) { - InsertData(_unicodePipelinedRequests); + InsertData(RequestParsingData.UnicodePipelinedRequests); ParseData(); } } @@ -145,7 +118,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance if (!Frame.TakeStartLine(readableBuffer, out var consumed, out var examined)) { - ThrowInvalidStartLine(); + ThrowInvalidRequestLine(); } Pipe.Reader.Advance(consumed, examined); @@ -156,38 +129,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance if (!Frame.TakeMessageHeaders(readableBuffer, out consumed, out examined)) { - ThrowInvalidMessageHeaders(); + ThrowInvalidRequestHeaders(); } Pipe.Reader.Advance(consumed, examined); } while (true); } - private void ThrowInvalidStartLine() + public static void ThrowInvalidRequestLine() { - throw new InvalidOperationException("Invalid StartLine"); + throw new InvalidOperationException("Invalid request line"); } - private void ThrowInvalidMessageHeaders() + public static void ThrowInvalidRequestHeaders() { - throw new InvalidOperationException("Invalid MessageHeaders"); + throw new InvalidOperationException("Invalid request headers"); } - - [Setup] - public void Setup() - { - var connectionContext = new MockConnection(new KestrelServerOptions()); - connectionContext.ListenerContext.ServiceContext.HttpParserFactory = frame => (IHttpParser)Activator.CreateInstance(ParserType, frame.ConnectionContext.ListenerContext.ServiceContext.Log); - - Frame = new Frame(application: null, context: connectionContext); - PipelineFactory = new PipeFactory(); - Pipe = PipelineFactory.Create(); - } - - public IPipe Pipe { get; set; } - - public Frame Frame { get; set; } - - public PipeFactory PipelineFactory { get; set; } } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs new file mode 100644 index 0000000000..bddc1a7a97 --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsingData.cs @@ -0,0 +1,62 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Linq; +using System.Text; +using BenchmarkDotNet.Attributes; + +namespace Microsoft.AspNetCore.Server.Kestrel.Performance +{ + [Config(typeof(CoreConfig))] + public class RequestParsingData + { + public const int InnerLoopCount = 512; + + public const int Pipelining = 16; + + private const string _plaintextTechEmpowerRequest = + "GET /plaintext HTTP/1.1\r\n" + + "Host: localhost\r\n" + + "Accept: text/plain,text/html;q=0.9,application/xhtml+xml;q=0.9,application/xml;q=0.8,*/*;q=0.7\r\n" + + "Connection: keep-alive\r\n" + + "\r\n"; + + private const string _liveaspnetRequest = + "GET https://live.asp.net/ HTTP/1.1\r\n" + + "Host: live.asp.net\r\n" + + "Connection: keep-alive\r\n" + + "Upgrade-Insecure-Requests: 1\r\n" + + "User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36\r\n" + + "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8\r\n" + + "DNT: 1\r\n" + + "Accept-Encoding: gzip, deflate, sdch, br\r\n" + + "Accept-Language: en-US,en;q=0.8\r\n" + + "Cookie: __unam=7a67379-1s65dc575c4-6d778abe-1; omniID=9519gfde_3347_4762_8762_df51458c8ec2\r\n" + + "\r\n"; + + private const string _unicodeRequest = + "GET http://stackoverflow.com/questions/40148683/why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric HTTP/1.1\r\n" + + "Accept: text/html, application/xhtml+xml, image/jxr, */*\r\n" + + "Accept-Language: en-US,en-GB;q=0.7,en;q=0.3\r\n" + + "User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/52.0.2743.116 Safari/537.36 Edge/15.14965\r\n" + + "Accept-Encoding: gzip, deflate\r\n" + + "Host: stackoverflow.com\r\n" + + "Connection: Keep-Alive\r\n" + + "Cache-Control: max-age=0\r\n" + + "Upgrade-Insecure-Requests: 1\r\n" + + "DNT: 1\r\n" + + "Referer: http://stackoverflow.com/?tab=month\r\n" + + "Pragma: no-cache\r\n" + + "Cookie: prov=20629ccd-8b0f-e8ef-2935-cd26609fc0bc; __qca=P0-1591065732-1479167353442; _ga=GA1.2.1298898376.1479167354; _gat=1; sgt=id=9519gfde_3347_4762_8762_df51458c8ec2; acct=t=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric&s=why-is-%e0%a5%a7%e0%a5%a8%e0%a5%a9-numeric\r\n" + + "\r\n"; + + public static readonly byte[] PlaintextTechEmpowerPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(_plaintextTechEmpowerRequest, Pipelining))); + public static readonly byte[] PlaintextTechEmpowerRequest = Encoding.ASCII.GetBytes(_plaintextTechEmpowerRequest); + + public static readonly byte[] LiveaspnetPipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(_liveaspnetRequest, Pipelining))); + public static readonly byte[] LiveaspnetRequest = Encoding.ASCII.GetBytes(_liveaspnetRequest); + + public static readonly byte[] UnicodePipelinedRequests = Encoding.ASCII.GetBytes(string.Concat(Enumerable.Repeat(_unicodeRequest, Pipelining))); + public static readonly byte[] UnicodeRequest = Encoding.ASCII.GetBytes(_unicodeRequest); + } +} From 7d94abd606a787ff9ecb421bdf7c3e486f9e291f Mon Sep 17 00:00:00 2001 From: John Luo Date: Thu, 2 Mar 2017 23:34:52 -0800 Subject: [PATCH 20/28] Enable default server address test --- .../AddressRegistrationTests.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs index 9e011adda4..fd7b950e24 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server.Features; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.DependencyInjection; @@ -129,7 +130,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } - [ConditionalFact(Skip = "Waiting on https://github.com/aspnet/Hosting/issues/917")] + [ConditionalFact] [PortSupportedCondition(5000)] public async Task DefaultsToPort5000() { @@ -147,13 +148,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { host.Start(); - var debugLog = testLogger.Messages.Single(log => log.LogLevel == LogLevel.Debug); - Assert.True(debugLog.Message.Contains("default")); + Assert.Equal(5000, host.GetPort()); + Assert.Single(testLogger.Messages, log => log.LogLevel == LogLevel.Debug && + string.Equals($"No listening endpoints were configured. Binding to {Constants.DefaultIPEndPoint} by default.", + log.Message, StringComparison.Ordinal)); - foreach (var testUrl in new[] { "http://127.0.0.1:5000", "http://localhost:5000" }) + foreach (var testUrl in new[] { "http://127.0.0.1:5000", /* "http://[::1]:5000" */}) { - var response = await HttpClientSlim.GetStringAsync(testUrl); - Assert.Equal(new Uri(testUrl).ToString(), response); + Assert.Equal(new Uri(testUrl).ToString(), await HttpClientSlim.GetStringAsync(testUrl)); } } } From 11c7eb5665b294395db4227dd6f163e7e0fcbed5 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Thu, 2 Mar 2017 16:34:22 -0800 Subject: [PATCH 21/28] Verify all request rejections are logged (#1295). --- .../BadHttpRequestTests.cs | 145 +++++----- .../FrameRequestHeadersTests.cs | 4 +- .../FrameTests.cs | 21 +- test/shared/HttpParsingData.cs | 249 ++++++++---------- 4 files changed, 210 insertions(+), 209 deletions(-) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 0394b7fcd3..80f127c9d1 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -1,11 +1,15 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Internal; +using Microsoft.Extensions.Logging; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests @@ -14,106 +18,102 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { [Theory] [MemberData(nameof(InvalidRequestLineData))] - public async Task TestInvalidRequestLines(string request) + public Task TestInvalidRequestLines(string request, string expectedExceptionMessage) { - using (var server = new TestServer(context => TaskCache.CompletedTask)) - { - using (var connection = server.CreateConnection()) - { - await connection.SendAll(request); - await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + request, + "400 Bad Request", + expectedExceptionMessage); } [Theory] [MemberData(nameof(UnrecognizedHttpVersionData))] - public async Task TestInvalidRequestLinesWithUnrecognizedVersion(string httpVersion) + public Task TestInvalidRequestLinesWithUnrecognizedVersion(string httpVersion) { - using (var server = new TestServer(context => TaskCache.CompletedTask)) - { - using (var connection = server.CreateConnection()) - { - await connection.SendAll($"GET / {httpVersion}\r\n"); - await ReceiveBadRequestResponse(connection, "505 HTTP Version Not Supported", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + $"GET / {httpVersion}\r\n", + "505 HTTP Version Not Supported", + $"Unrecognized HTTP version: {httpVersion}"); } [Theory] [MemberData(nameof(InvalidRequestHeaderData))] - public async Task TestInvalidHeaders(string rawHeaders) + public Task TestInvalidHeaders(string rawHeaders, string expectedExceptionMessage) { - using (var server = new TestServer(context => TaskCache.CompletedTask)) - { - using (var connection = server.CreateConnection()) - { - await connection.SendAll($"GET / HTTP/1.1\r\n{rawHeaders}"); - await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + $"GET / HTTP/1.1\r\n{rawHeaders}", + "400 Bad Request", + expectedExceptionMessage); } - [Fact] - public async Task BadRequestWhenHeaderNameContainsNonASCIICharacters() + [Theory] + [InlineData("Hea\0der: value", "Invalid characters in header name.")] + [InlineData("Header: va\0lue", "Malformed request: invalid headers.")] + [InlineData("Head\x80r: value", "Invalid characters in header name.")] + [InlineData("Header: valu\x80", "Malformed request: invalid headers.")] + public Task BadRequestWhenHeaderNameContainsNonASCIIOrNullCharacters(string header, string expectedExceptionMessage) { - using (var server = new TestServer(context => { return Task.FromResult(0); })) - { - using (var connection = server.CreateConnection()) - { - await connection.SendAll( - "GET / HTTP/1.1", - "H\u00eb\u00e4d\u00ebr: value", - "", - ""); - await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + $"GET / HTTP/1.1\r\n{header}\r\n\r\n", + "400 Bad Request", + expectedExceptionMessage); } [Theory] [InlineData("POST")] [InlineData("PUT")] - public async Task BadRequestIfMethodRequiresLengthButNoContentLengthOrTransferEncodingInRequest(string method) + public Task BadRequestIfMethodRequiresLengthButNoContentLengthOrTransferEncodingInRequest(string method) { - using (var server = new TestServer(context => { return Task.FromResult(0); })) - { - using (var connection = server.CreateConnection()) - { - await connection.Send($"{method} / HTTP/1.1\r\n\r\n"); - await ReceiveBadRequestResponse(connection, "411 Length Required", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + $"{method} / HTTP/1.1\r\n\r\n", + "411 Length Required", + $"{method} request contains no Content-Length or Transfer-Encoding header"); } [Theory] [InlineData("POST")] [InlineData("PUT")] - public async Task BadRequestIfMethodRequiresLengthButNoContentLengthInHttp10Request(string method) + public Task BadRequestIfMethodRequiresLengthButNoContentLengthInHttp10Request(string method) { - using (var server = new TestServer(context => { return Task.FromResult(0); })) - { - using (var connection = server.CreateConnection()) - { - await connection.Send($"{method} / HTTP/1.0\r\n\r\n"); - await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue); - } - } + return TestBadRequest( + $"{method} / HTTP/1.0\r\n\r\n", + "400 Bad Request", + $"{method} request contains no Content-Length header"); } [Theory] [InlineData("NaN")] [InlineData("-1")] - public async Task BadRequestIfContentLengthInvalid(string contentLength) + public Task BadRequestIfContentLengthInvalid(string contentLength) { - using (var server = new TestServer(context => { return Task.FromResult(0); })) + return TestBadRequest( + $"POST / HTTP/1.1\r\nContent-Length: {contentLength}\r\n\r\n", + "400 Bad Request", + $"Invalid content length: {contentLength}"); + } + + private async Task TestBadRequest(string request, string expectedResponseStatusCode, string expectedExceptionMessage) + { + BadHttpRequestException loggedException = null; + var mockKestrelTrace = new Mock(); + mockKestrelTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + mockKestrelTrace + .Setup(trace => trace.ConnectionBadRequest(It.IsAny(), It.IsAny())) + .Callback((connectionId, exception) => loggedException = exception); + + using (var server = new TestServer(context => TaskCache.CompletedTask, new TestServiceContext { Log = mockKestrelTrace.Object })) { using (var connection = server.CreateConnection()) { - await connection.SendAll($"GET / HTTP/1.1\r\nContent-Length: {contentLength}\r\n\r\n"); - await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue); + await connection.SendAll(request); + await ReceiveBadRequestResponse(connection, expectedResponseStatusCode, server.Context.DateHeaderValue); } } + + mockKestrelTrace.Verify(trace => trace.ConnectionBadRequest(It.IsAny(), It.IsAny())); + Assert.Equal(expectedExceptionMessage, loggedException.Message); } private async Task ReceiveBadRequestResponse(TestConnection connection, string expectedResponseStatusCode, string expectedDateHeaderValue) @@ -127,10 +127,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests ""); } - public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData.Select(data => new[] { data[0] }); + public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData + .Select(requestLine => new object[] + { + requestLine, + $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", + }) + .Concat(HttpParsingData.EncodedNullCharInTargetRequestLines.Select(requestLine => new object[] + { + requestLine, + "Invalid request line." + })) + .Concat(HttpParsingData.NullCharInTargetRequestLines.Select(requestLine => new object[] + { + requestLine, + "Invalid request line." + })); public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; - public static IEnumerable InvalidRequestHeaderData => HttpParsingData.InvalidRequestHeaderData.Select(data => new[] { data[0] }); + public static IEnumerable InvalidRequestHeaderData => HttpParsingData.InvalidRequestHeaderData; } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestHeadersTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestHeadersTests.cs index dbf6d857a5..006ac65cd1 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestHeadersTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameRequestHeadersTests.cs @@ -304,14 +304,14 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } [Fact] - public void AppendThrowsWhenHeaderValueContainsNonASCIICharacters() + public void AppendThrowsWhenHeaderNameContainsNonASCIICharacters() { var headers = new FrameRequestHeaders(); const string key = "\u00141d\017c"; var encoding = Encoding.GetEncoding("iso-8859-1"); var exception = Assert.Throws( - () => headers.Append(encoding.GetBytes(key), key)); + () => headers.Append(encoding.GetBytes(key), "value")); Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index f48a614579..ebfd64fea4 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.IO; using System.IO.Pipelines; +using System.Linq; using System.Net; using System.Text; using System.Threading; @@ -759,7 +760,25 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public static IEnumerable ValidRequestLineData => HttpParsingData.ValidRequestLineData; - public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData; + public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData + .Select(requestLine => new object[] + { + requestLine, + typeof(BadHttpRequestException), + $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", + }) + .Concat(HttpParsingData.EncodedNullCharInTargetRequestLines.Select(requestLine => new object[] + { + requestLine, + typeof(InvalidOperationException), + "The path contains null characters." + })) + .Concat(HttpParsingData.NullCharInTargetRequestLines.Select(requestLine => new object[] + { + requestLine, + typeof(InvalidOperationException), + new InvalidOperationException().Message + })); public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index a5a83c58c6..50a5633c9c 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Microsoft.AspNetCore.Server.Kestrel; using Xunit; namespace Microsoft.AspNetCore.Testing @@ -71,150 +70,118 @@ namespace Microsoft.AspNetCore.Testing } } - // All these test cases must end in '\n', otherwise the server will spin forever - public static IEnumerable InvalidRequestLineData + public static IEnumerable InvalidRequestLineData => new[] { - get - { - var invalidRequestLines = new[] - { - "G\r\n", - "GE\r\n", - "GET\r\n", - "GET \r\n", - "GET /\r\n", - "GET / \r\n", - "GET/HTTP/1.1\r\n", - "GET /HTTP/1.1\r\n", - " \r\n", - " \r\n", - "/ HTTP/1.1\r\n", - " / HTTP/1.1\r\n", - "/ \r\n", - "GET \r\n", - "GET HTTP/1.0\r\n", - "GET HTTP/1.1\r\n", - "GET / \n", - "GET / HTTP/1.0\n", - "GET / HTTP/1.1\n", - "GET / HTTP/1.0\rA\n", - "GET / HTTP/1.1\ra\n", - "GET? / HTTP/1.1\r\n", - "GET ? HTTP/1.1\r\n", - "GET /a?b=cHTTP/1.1\r\n", - "GET /a%20bHTTP/1.1\r\n", - "GET /a%20b?c=dHTTP/1.1\r\n", - "GET %2F HTTP/1.1\r\n", - "GET %00 HTTP/1.1\r\n", - "CUSTOM \r\n", - "CUSTOM /\r\n", - "CUSTOM / \r\n", - "CUSTOM /HTTP/1.1\r\n", - "CUSTOM \r\n", - "CUSTOM HTTP/1.0\r\n", - "CUSTOM HTTP/1.1\r\n", - "CUSTOM / \n", - "CUSTOM / HTTP/1.0\n", - "CUSTOM / HTTP/1.1\n", - "CUSTOM / HTTP/1.0\rA\n", - "CUSTOM / HTTP/1.1\ra\n", - "CUSTOM ? HTTP/1.1\r\n", - "CUSTOM /a?b=cHTTP/1.1\r\n", - "CUSTOM /a%20bHTTP/1.1\r\n", - "CUSTOM /a%20b?c=dHTTP/1.1\r\n", - "CUSTOM %2F HTTP/1.1\r\n", - "CUSTOM %00 HTTP/1.1\r\n", - // Bad HTTP Methods (invalid according to RFC) - "( / HTTP/1.0\r\n", - ") / HTTP/1.0\r\n", - "< / HTTP/1.0\r\n", - "> / HTTP/1.0\r\n", - "@ / HTTP/1.0\r\n", - ", / HTTP/1.0\r\n", - "; / HTTP/1.0\r\n", - ": / HTTP/1.0\r\n", - "\\ / HTTP/1.0\r\n", - "\" / HTTP/1.0\r\n", - "/ / HTTP/1.0\r\n", - "[ / HTTP/1.0\r\n", - "] / HTTP/1.0\r\n", - "? / HTTP/1.0\r\n", - "= / HTTP/1.0\r\n", - "{ / HTTP/1.0\r\n", - "} / HTTP/1.0\r\n", - "get@ / HTTP/1.0\r\n", - "post= / HTTP/1.0\r\n", - }; + "G\r\n", + "GE\r\n", + "GET\r\n", + "GET \r\n", + "GET /\r\n", + "GET / \r\n", + "GET/HTTP/1.1\r\n", + "GET /HTTP/1.1\r\n", + " \r\n", + " \r\n", + "/ HTTP/1.1\r\n", + " / HTTP/1.1\r\n", + "/ \r\n", + "GET \r\n", + "GET HTTP/1.0\r\n", + "GET HTTP/1.1\r\n", + "GET / \n", + "GET / HTTP/1.0\n", + "GET / HTTP/1.1\n", + "GET / HTTP/1.0\rA\n", + "GET / HTTP/1.1\ra\n", + "GET? / HTTP/1.1\r\n", + "GET ? HTTP/1.1\r\n", + "GET /a?b=cHTTP/1.1\r\n", + "GET /a%20bHTTP/1.1\r\n", + "GET /a%20b?c=dHTTP/1.1\r\n", + "GET %2F HTTP/1.1\r\n", + "GET %00 HTTP/1.1\r\n", + "CUSTOM \r\n", + "CUSTOM /\r\n", + "CUSTOM / \r\n", + "CUSTOM /HTTP/1.1\r\n", + "CUSTOM \r\n", + "CUSTOM HTTP/1.0\r\n", + "CUSTOM HTTP/1.1\r\n", + "CUSTOM / \n", + "CUSTOM / HTTP/1.0\n", + "CUSTOM / HTTP/1.1\n", + "CUSTOM / HTTP/1.0\rA\n", + "CUSTOM / HTTP/1.1\ra\n", + "CUSTOM ? HTTP/1.1\r\n", + "CUSTOM /a?b=cHTTP/1.1\r\n", + "CUSTOM /a%20bHTTP/1.1\r\n", + "CUSTOM /a%20b?c=dHTTP/1.1\r\n", + "CUSTOM %2F HTTP/1.1\r\n", + "CUSTOM %00 HTTP/1.1\r\n", + // Bad HTTP Methods (invalid according to RFC) + "( / HTTP/1.0\r\n", + ") / HTTP/1.0\r\n", + "< / HTTP/1.0\r\n", + "> / HTTP/1.0\r\n", + "@ / HTTP/1.0\r\n", + ", / HTTP/1.0\r\n", + "; / HTTP/1.0\r\n", + ": / HTTP/1.0\r\n", + "\\ / HTTP/1.0\r\n", + "\" / HTTP/1.0\r\n", + "/ / HTTP/1.0\r\n", + "[ / HTTP/1.0\r\n", + "] / HTTP/1.0\r\n", + "? / HTTP/1.0\r\n", + "= / HTTP/1.0\r\n", + "{ / HTTP/1.0\r\n", + "} / HTTP/1.0\r\n", + "get@ / HTTP/1.0\r\n", + "post= / HTTP/1.0\r\n", + }; - var encodedNullCharInTargetRequestLines = new[] - { - "GET /%00 HTTP/1.1\r\n", - "GET /%00%00 HTTP/1.1\r\n", - "GET /%E8%00%84 HTTP/1.1\r\n", - "GET /%E8%85%00 HTTP/1.1\r\n", - "GET /%F3%00%82%86 HTTP/1.1\r\n", - "GET /%F3%85%00%82 HTTP/1.1\r\n", - "GET /%F3%85%82%00 HTTP/1.1\r\n", - "GET /%E8%85%00 HTTP/1.1\r\n", - "GET /%E8%01%00 HTTP/1.1\r\n", - }; - - var nullCharInTargetRequestLines = new[] - { - "GET \0 HTTP/1.1\r\n", - "GET /\0 HTTP/1.1\r\n", - "GET /\0\0 HTTP/1.1\r\n", - "GET /%C8\0 HTTP/1.1\r\n", - }; - - return invalidRequestLines.Select(requestLine => new object[] - { - requestLine, - typeof(BadHttpRequestException), - $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}" - }) - .Concat(encodedNullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - typeof(InvalidOperationException), - $"The path contains null characters." - })) - .Concat(nullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - typeof(InvalidOperationException), - new InvalidOperationException().Message - })); - } - } - - public static TheoryData UnrecognizedHttpVersionData + public static IEnumerable EncodedNullCharInTargetRequestLines => new[] { - get + "GET /%00 HTTP/1.1\r\n", + "GET /%00%00 HTTP/1.1\r\n", + "GET /%E8%00%84 HTTP/1.1\r\n", + "GET /%E8%85%00 HTTP/1.1\r\n", + "GET /%F3%00%82%86 HTTP/1.1\r\n", + "GET /%F3%85%00%82 HTTP/1.1\r\n", + "GET /%F3%85%82%00 HTTP/1.1\r\n", + "GET /%E8%85%00 HTTP/1.1\r\n", + "GET /%E8%01%00 HTTP/1.1\r\n", + }; + + public static IEnumerable NullCharInTargetRequestLines => new[] { - return new TheoryData - { - "H", - "HT", - "HTT", - "HTTP", - "HTTP/", - "HTTP/1", - "HTTP/1.", - "http/1.0", - "http/1.1", - "HTTP/1.1 ", - "HTTP/1.0a", - "HTTP/1.0ab", - "HTTP/1.1a", - "HTTP/1.1ab", - "HTTP/1.2", - "HTTP/3.0", - "hello", - "8charact", - }; - } - } + "GET \0 HTTP/1.1\r\n", + "GET /\0 HTTP/1.1\r\n", + "GET /\0\0 HTTP/1.1\r\n", + "GET /%C8\0 HTTP/1.1\r\n", + }; + + public static TheoryData UnrecognizedHttpVersionData => new TheoryData + { + "H", + "HT", + "HTT", + "HTTP", + "HTTP/", + "HTTP/1", + "HTTP/1.", + "http/1.0", + "http/1.1", + "HTTP/1.1 ", + "HTTP/1.0a", + "HTTP/1.0ab", + "HTTP/1.1a", + "HTTP/1.1ab", + "HTTP/1.2", + "HTTP/3.0", + "hello", + "8charact", + }; public static IEnumerable InvalidRequestHeaderData { From 06134bc6e0795f0908c6dcdccec5915c3185dce3 Mon Sep 17 00:00:00 2001 From: John Luo Date: Sat, 4 Mar 2017 22:03:27 -0800 Subject: [PATCH 22/28] Add IPv6 loopback address by default #1434 --- .../Internal/Infrastructure/Constants.cs | 2 +- .../KestrelServer.cs | 12 +++++-- .../AddressRegistrationTests.cs | 31 +++++++++++++------ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs index e0f592154b..2c201fea7c 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/Constants.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure /// /// The IPEndPoint Kestrel will bind to if nothing else is specified. /// - public static readonly IPEndPoint DefaultIPEndPoint = new IPEndPoint(IPAddress.Loopback, 5000); + public static readonly string DefaultServerAddress = "http://localhost:5000"; /// /// Prefix of host name used to specify Unix sockets in the configuration. diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs index 1d05a0dd2b..a36da353f4 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/KestrelServer.cs @@ -142,8 +142,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel } else if (!hasListenOptions && !hasServerAddresses) { - _logger.LogDebug($"No listening endpoints were configured. Binding to {Constants.DefaultIPEndPoint} by default."); - listenOptions.Add(new ListenOptions(Constants.DefaultIPEndPoint)); + _logger.LogDebug($"No listening endpoints were configured. Binding to {Constants.DefaultServerAddress} by default."); + + // "localhost" for both IPv4 and IPv6 can't be represented as an IPEndPoint. + StartLocalhost(engine, ServerAddress.FromUrl(Constants.DefaultServerAddress)); + + // If StartLocalhost doesn't throw, there is at least one listener. + // The port cannot change for "localhost". + _serverAddresses.Addresses.Add(Constants.DefaultServerAddress); + + return; } else if (!hasListenOptions) { diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs index fd7b950e24..572b45aa68 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs @@ -132,16 +132,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [ConditionalFact] [PortSupportedCondition(5000)] - public async Task DefaultsToPort5000() + public Task DefaultsServerAddress_BindsToIPv4() + { + return RegisterDefaultServerAddresses_Success(new[] { "http://127.0.0.1:5000" }); + } + + [ConditionalFact] + [IPv6SupportedCondition] + [PortSupportedCondition(5000)] + public Task DefaultsServerAddress_BindsToIPv6() + { + return RegisterDefaultServerAddresses_Success(new[] { "http://127.0.0.1:5000", "http://[::1]:5000" }); + } + + private async Task RegisterDefaultServerAddresses_Success(IEnumerable addresses) { var testLogger = new TestApplicationErrorLogger(); var hostBuilder = new WebHostBuilder() .UseKestrel() .ConfigureServices(services => - { - services.AddSingleton(new KestrelTestLoggerFactory(testLogger)); - }) + { + services.AddSingleton(new KestrelTestLoggerFactory(testLogger)); + }) .Configure(ConfigureEchoAddress); using (var host = hostBuilder.Build()) @@ -150,12 +163,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests Assert.Equal(5000, host.GetPort()); Assert.Single(testLogger.Messages, log => log.LogLevel == LogLevel.Debug && - string.Equals($"No listening endpoints were configured. Binding to {Constants.DefaultIPEndPoint} by default.", + string.Equals($"No listening endpoints were configured. Binding to {Constants.DefaultServerAddress} by default.", log.Message, StringComparison.Ordinal)); - foreach (var testUrl in new[] { "http://127.0.0.1:5000", /* "http://[::1]:5000" */}) + foreach (var address in addresses) { - Assert.Equal(new Uri(testUrl).ToString(), await HttpClientSlim.GetStringAsync(testUrl)); + Assert.Equal(new Uri(address).ToString(), await HttpClientSlim.GetStringAsync(address)); } } } @@ -373,8 +386,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var dataset = new TheoryData>(); // Default host and port - dataset.Add(null, _ => new[] { "http://127.0.0.1:5000/", /*"http://[::1]:5000/"*/ }); - dataset.Add(string.Empty, _ => new[] { "http://127.0.0.1:5000/", /*"http://[::1]:5000/"*/ }); + dataset.Add(null, _ => new[] { "http://127.0.0.1:5000/", "http://[::1]:5000/" }); + dataset.Add(string.Empty, _ => new[] { "http://127.0.0.1:5000/", "http://[::1]:5000/" }); // Static ports var port = GetNextPort(); From 02a434290824a1d9ef4496f6bb6a742d156be465 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 7 Mar 2017 08:58:55 -0800 Subject: [PATCH 23/28] Made changes to TakeSingleHeader (#1453) * Made changes to TakeSingleHeader - Remove state machine and just parse in place - Inline OnHeader into TakeSingleHeader - Use IndexOfVectorized instead of custom indexof - Normalize header whitespace error - Combine IndexOf and IndexOfAny into a single IndexOfNameEnd call --- .../BadHttpRequestException.cs | 3 - .../Internal/Http/KestrelHttpParser.cs | 273 ++++++++---------- .../Internal/Http/RequestRejectionReason.cs | 1 - .../FrameTests.cs | 2 +- test/shared/HttpParsingData.cs | 4 +- 5 files changed, 131 insertions(+), 152 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index a19fd6db35..54533f05e1 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs @@ -25,9 +25,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel case RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence: ex = new BadHttpRequestException("Headers corrupted, invalid header sequence.", StatusCodes.Status400BadRequest); break; - case RequestRejectionReason.HeaderLineMustNotStartWithWhitespace: - ex = new BadHttpRequestException("Header line must not start with whitespace.", StatusCodes.Status400BadRequest); - break; case RequestRejectionReason.NoColonCharacterFoundInHeaderLine: ex = new BadHttpRequestException("No ':' character found in header line.", StatusCodes.Status400BadRequest); break; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index 998aa6f46a..f85af592fb 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.IO.Pipelines; +using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.Extensions.Logging; @@ -316,9 +317,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Headers don't end in CRLF line. RejectRequest(RequestRejectionReason.HeadersCorruptedInvalidHeaderSequence); } - else if (ch1 == ByteSpace || ch1 == ByteTab) + else if(ch1 == ByteSpace || ch1 == ByteTab) { - RejectRequest(RequestRejectionReason.HeaderLineMustNotStartWithWhitespace); + RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); } // We moved the reader so look ahead 2 bytes so reset both the reader @@ -329,13 +330,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http index = reader.Index; } - Span headerSpan; - - var endIndex = IndexOf(pBuffer, index, remaining, ByteLF); + var endIndex = new Span(pBuffer + index, remaining).IndexOfVectorized(ByteLF); + var length = 0; if (endIndex != -1) { - headerSpan = new Span(pBuffer + index, (endIndex - index + 1)); + length = endIndex + 1; + var pHeader = pBuffer + index; + + TakeSingleHeader(pHeader, length, handler); } else { @@ -350,28 +353,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http // Make sure LF is included in lineEnd lineEnd = buffer.Move(lineEnd, 1); - headerSpan = buffer.Slice(current, lineEnd).ToSpan(); + var headerSpan = buffer.Slice(current, lineEnd).ToSpan(); + length = headerSpan.Length; + + fixed (byte* pHeader = &headerSpan.DangerousGetPinnableReference()) + { + TakeSingleHeader(pHeader, length, handler); + } // We're going to the next span after this since we know we crossed spans here // so mark the remaining as equal to the headerSpan so that we end up at 0 // on the next iteration - remaining = headerSpan.Length; - } - - if (!TakeSingleHeader(headerSpan, out var nameStart, out var nameEnd, out var valueStart, out var valueEnd)) - { - return false; + remaining = length; } // Skip the reader forward past the header line - reader.Skip(headerSpan.Length); - - remaining -= headerSpan.Length; - - var nameBuffer = headerSpan.Slice(nameStart, nameEnd - nameStart); - var valueBuffer = headerSpan.Slice(valueStart, valueEnd - valueStart); - - handler.OnHeader(nameBuffer, valueBuffer); + reader.Skip(length); + remaining -= length; } } } @@ -390,142 +388,127 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } - private unsafe int IndexOf(byte* pBuffer, int index, int length, byte value) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private unsafe int IndexOfNameEnd(byte* pBuffer, int index, int length) { - for (int i = 0; i < length; i++, index++) + var pCurrent = pBuffer + index; + var pEnd = pBuffer + index + length; + var result = -1; + var sawWhitespace = false; + + while (pCurrent < pEnd) { - if (pBuffer[index] == value) + var ch = *pCurrent; + if (ch == ByteColon) { - return index; + if (sawWhitespace) + { + RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); + } + + result = index; + break; } + + if (ch == ByteTab || ch == ByteSpace) + { + sawWhitespace = true; + } + + index++; + pCurrent++; } - return -1; + return result; } - private unsafe bool TakeSingleHeader(Span span, out int nameStart, out int nameEnd, out int valueStart, out int valueEnd) + private unsafe void TakeSingleHeader(byte* pHeader, int headerLineLength, T handler) where T : IHttpHeadersHandler { - nameStart = 0; - nameEnd = -1; - valueStart = -1; - valueEnd = -1; - var headerLineLength = span.Length; - var nameHasWhitespace = false; - var previouslyWhitespace = false; + var nameEnd = -1; + var valueStart = -1; + var valueEnd = -1; + var index = 0; + var pCurrent = pHeader + index; + var pEnd = pHeader + headerLineLength; - int i = 0; - var done = false; - fixed (byte* pHeader = &span.DangerousGetPinnableReference()) + nameEnd = IndexOfNameEnd(pHeader, index, headerLineLength); + + if (nameEnd == -1) { - switch (HeaderState.Name) - { - case HeaderState.Name: - for (; i < headerLineLength; i++) - { - var ch = pHeader[i]; - if (ch == ByteColon) - { - if (nameHasWhitespace) - { - RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); - } - nameEnd = i; - - // Consume space - i++; - - goto case HeaderState.Whitespace; - } - - if (ch == ByteSpace || ch == ByteTab) - { - nameHasWhitespace = true; - } - } - RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); - - break; - case HeaderState.Whitespace: - for (; i < headerLineLength; i++) - { - var ch = pHeader[i]; - var whitespace = ch == ByteTab || ch == ByteSpace || ch == ByteCR; - - if (!whitespace) - { - // Mark the first non whitespace char as the start of the - // header value and change the state to expect to the header value - valueStart = i; - - goto case HeaderState.ExpectValue; - } - // If we see a CR then jump to the next state directly - else if (ch == ByteCR) - { - goto case HeaderState.ExpectValue; - } - } - - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); - - break; - case HeaderState.ExpectValue: - for (; i < headerLineLength; i++) - { - var ch = pHeader[i]; - var whitespace = ch == ByteTab || ch == ByteSpace; - - if (whitespace) - { - if (!previouslyWhitespace) - { - // If we see a whitespace char then maybe it's end of the - // header value - valueEnd = i; - } - } - else if (ch == ByteCR) - { - // If we see a CR and we haven't ever seen whitespace then - // this is the end of the header value - if (valueEnd == -1) - { - valueEnd = i; - } - - // We never saw a non whitespace character before the CR - if (valueStart == -1) - { - valueStart = valueEnd; - } - - // Consume space - i++; - - goto case HeaderState.ExpectNewLine; - } - else - { - // If we find a non whitespace char that isn't CR then reset the end index - valueEnd = -1; - } - - previouslyWhitespace = whitespace; - } - RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); - break; - case HeaderState.ExpectNewLine: - if (pHeader[i] != ByteLF) - { - RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); - } - goto case HeaderState.Complete; - case HeaderState.Complete: - done = true; - break; - } + RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); } - return done; + // Skip colon + index += nameEnd + 1; + pCurrent += index; + valueStart = index; + var pValueStart = pCurrent; + + while (pCurrent < pEnd) + { + var ch = *pCurrent; + if (ch != ByteTab && ch != ByteSpace && ch != ByteCR) + { + valueStart = index; + pValueStart = pCurrent; + break; + } + else if (ch == ByteCR) + { + break; + } + pCurrent++; + index++; + } + + var endIndex = headerLineLength - 1; + pEnd--; + + if (*pEnd != ByteLF) + { + RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + } + + endIndex--; + pEnd--; + + if (*pEnd != ByteCR) + { + RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); + } + + endIndex--; + pEnd--; + + while (pEnd >= pValueStart) + { + var ch = *pEnd; + if (ch != ByteTab && ch != ByteSpace && ch != ByteCR && valueEnd == -1) + { + valueEnd = endIndex; + } + else if (ch == ByteCR) + { + RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); + } + + pEnd--; + endIndex--; + } + + if (valueEnd == -1) + { + valueEnd = valueStart; + } + else + { + valueEnd++; + } + + + var nameBuffer = new Span(pHeader, nameEnd); + var valueBuffer = new Span(pHeader + valueStart, valueEnd - valueStart); + + handler.OnHeader(nameBuffer, valueBuffer); } private static bool IsValidTokenChar(char c) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs index 042a25d99c..b0aa5a1372 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs @@ -7,7 +7,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { UnrecognizedHTTPVersion, HeadersCorruptedInvalidHeaderSequence, - HeaderLineMustNotStartWithWhitespace, NoColonCharacterFoundInHeaderLine, WhitespaceIsNotAllowedInHeaderName, HeaderValueMustNotContainCR, diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index ebfd64fea4..46d54d1b40 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -200,7 +200,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); _socketInput.Reader.Advance(_consumed, _examined); - Assert.Equal("Header line must not start with whitespace.", exception.Message); + Assert.Equal("Whitespace is not allowed in header name.", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 50a5633c9c..56688b67f0 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -254,10 +254,10 @@ namespace Microsoft.AspNetCore.Testing return new[] { - Tuple.Create(headersWithLineFolding,"Header line must not start with whitespace."), + Tuple.Create(headersWithLineFolding,"Whitespace is not allowed in header name."), Tuple.Create(headersWithCRInValue,"Header value must not contain CR characters."), Tuple.Create(headersWithMissingColon,"No ':' character found in header line."), - Tuple.Create(headersStartingWithWhitespace, "Header line must not start with whitespace."), + Tuple.Create(headersStartingWithWhitespace, "Whitespace is not allowed in header name."), Tuple.Create(headersWithWithspaceInName,"Whitespace is not allowed in header name."), Tuple.Create(headersNotEndingInCrLfLine, "Headers corrupted, invalid header sequence.") } From 1294c006183af28bd3f34ccc01ea1354d3e830d9 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 7 Mar 2017 11:52:45 -0800 Subject: [PATCH 24/28] Cleanup unused code (#1458) --- .../Internal/Http/UrlPathDecoder.cs | 312 ------ .../Infrastructure/MemoryPoolIterator.cs | 895 +----------------- .../AsciiDecoding.cs | 144 +-- .../MemoryPoolBlockTests.cs | 182 +--- .../MemoryPoolExtensions.cs | 7 +- .../MemoryPoolIteratorTests.cs | 803 +--------------- .../UrlPathDecoder.cs | 226 ----- 7 files changed, 34 insertions(+), 2535 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/UrlPathDecoder.cs delete mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/UrlPathDecoder.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/UrlPathDecoder.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/UrlPathDecoder.cs deleted file mode 100644 index 67004cf24f..0000000000 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/UrlPathDecoder.cs +++ /dev/null @@ -1,312 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; - -namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http -{ - public class UrlPathDecoder - { - /// - /// Unescapes the string between given memory iterators in place. - /// - /// The iterator points to the beginning of the sequence. - /// The iterator points to the byte behind the end of the sequence. - /// The iterator points to the byte behind the end of the processed sequence. - public static MemoryPoolIterator Unescape(MemoryPoolIterator start, MemoryPoolIterator end) - { - // the slot to read the input - var reader = start; - - // the slot to write the unescaped byte - var writer = reader; - - while (true) - { - if (CompareIterators(ref reader, ref end)) - { - return writer; - } - - if (reader.Peek() == '%') - { - var decodeReader = reader; - - // If decoding process succeeds, the writer iterator will be moved - // to the next write-ready location. On the other hand if the scanned - // percent-encodings cannot be interpreted as sequence of UTF-8 octets, - // these bytes should be copied to output as is. - // The decodeReader iterator is always moved to the first byte not yet - // be scanned after the process. A failed decoding means the chars - // between the reader and decodeReader can be copied to output untouched. - if (!DecodeCore(ref decodeReader, ref writer, end)) - { - Copy(reader, decodeReader, ref writer); - } - - reader = decodeReader; - } - else - { - writer.Put((byte)reader.Take()); - } - } - } - - /// - /// Unescape the percent-encodings - /// - /// The iterator point to the first % char - /// The place to write to - /// The end of the sequence - private static bool DecodeCore(ref MemoryPoolIterator reader, ref MemoryPoolIterator writer, MemoryPoolIterator end) - { - // preserves the original head. if the percent-encodings cannot be interpreted as sequence of UTF-8 octets, - // bytes from this till the last scanned one will be copied to the memory pointed by writer. - var byte1 = UnescapePercentEncoding(ref reader, end); - - if (byte1 == 0) - { - throw BadHttpRequestException.GetException(RequestRejectionReason.PathContainsNullCharacters); - } - - if (byte1 == -1) - { - return false; - } - - if (byte1 <= 0x7F) - { - // first byte < U+007f, it is a single byte ASCII - writer.Put((byte)byte1); - return true; - } - - int byte2 = 0, byte3 = 0, byte4 = 0; - - // anticipate more bytes - var currentDecodeBits = 0; - var byteCount = 1; - var expectValueMin = 0; - if ((byte1 & 0xE0) == 0xC0) - { - // 110x xxxx, expect one more byte - currentDecodeBits = byte1 & 0x1F; - byteCount = 2; - expectValueMin = 0x80; - } - else if ((byte1 & 0xF0) == 0xE0) - { - // 1110 xxxx, expect two more bytes - currentDecodeBits = byte1 & 0x0F; - byteCount = 3; - expectValueMin = 0x800; - } - else if ((byte1 & 0xF8) == 0xF0) - { - // 1111 0xxx, expect three more bytes - currentDecodeBits = byte1 & 0x07; - byteCount = 4; - expectValueMin = 0x10000; - } - else - { - // invalid first byte - return false; - } - - var remainingBytes = byteCount - 1; - while (remainingBytes > 0) - { - // read following three chars - if (CompareIterators(ref reader, ref end)) - { - return false; - } - - var nextItr = reader; - var nextByte = UnescapePercentEncoding(ref nextItr, end); - if (nextByte == -1) - { - return false; - } - - if ((nextByte & 0xC0) != 0x80) - { - // the follow up byte is not in form of 10xx xxxx - return false; - } - - currentDecodeBits = (currentDecodeBits << 6) | (nextByte & 0x3F); - remainingBytes--; - - if (remainingBytes == 1 && currentDecodeBits >= 0x360 && currentDecodeBits <= 0x37F) - { - // this is going to end up in the range of 0xD800-0xDFFF UTF-16 surrogates that - // are not allowed in UTF-8; - return false; - } - - if (remainingBytes == 2 && currentDecodeBits >= 0x110) - { - // this is going to be out of the upper Unicode bound 0x10FFFF. - return false; - } - - reader = nextItr; - if (byteCount - remainingBytes == 2) - { - byte2 = nextByte; - } - else if (byteCount - remainingBytes == 3) - { - byte3 = nextByte; - } - else if (byteCount - remainingBytes == 4) - { - byte4 = nextByte; - } - } - - if (currentDecodeBits < expectValueMin) - { - // overlong encoding (e.g. using 2 bytes to encode something that only needed 1). - return false; - } - - // all bytes are verified, write to the output - if (byteCount > 0) - { - writer.Put((byte)byte1); - } - if (byteCount > 1) - { - writer.Put((byte)byte2); - } - if (byteCount > 2) - { - writer.Put((byte)byte3); - } - if (byteCount > 3) - { - writer.Put((byte)byte4); - } - - return true; - } - - private static void Copy(MemoryPoolIterator head, MemoryPoolIterator tail, ref MemoryPoolIterator writer) - { - while (!CompareIterators(ref head, ref tail)) - { - writer.Put((byte)head.Take()); - } - } - - /// - /// Read the percent-encoding and try unescape it. - /// - /// The operation first peek at the character the - /// iterator points at. If it is % the is then - /// moved on to scan the following to characters. If the two following - /// characters are hexadecimal literals they will be unescaped and the - /// value will be returned. - /// - /// If the first character is not % the iterator - /// will be removed beyond the location of % and -1 will be returned. - /// - /// If the following two characters can't be successfully unescaped the - /// iterator will be move behind the % and -1 - /// will be returned. - /// - /// The value to read - /// The end of the sequence - /// The unescaped byte if success. Otherwise return -1. - private static int UnescapePercentEncoding(ref MemoryPoolIterator scan, MemoryPoolIterator end) - { - if (scan.Take() != '%') - { - return -1; - } - - var probe = scan; - - int value1 = ReadHex(ref probe, end); - if (value1 == -1) - { - return -1; - } - - int value2 = ReadHex(ref probe, end); - if (value2 == -1) - { - return -1; - } - - if (SkipUnescape(value1, value2)) - { - return -1; - } - - scan = probe; - return (value1 << 4) + value2; - } - - /// - /// Read the next char and convert it into hexadecimal value. - /// - /// The iterator will be moved to the next - /// byte no matter no matter whether the operation successes. - /// - /// The value to read - /// The end of the sequence - /// The hexadecimal value if successes, otherwise -1. - private static int ReadHex(ref MemoryPoolIterator scan, MemoryPoolIterator end) - { - if (CompareIterators(ref scan, ref end)) - { - return -1; - } - - var value = scan.Take(); - var isHead = (((value >= '0') && (value <= '9')) || - ((value >= 'A') && (value <= 'F')) || - ((value >= 'a') && (value <= 'f'))); - - if (!isHead) - { - return -1; - } - - if (value <= '9') - { - return value - '0'; - } - else if (value <= 'F') - { - return (value - 'A') + 10; - } - else // a - f - { - return (value - 'a') + 10; - } - } - - private static bool SkipUnescape(int value1, int value2) - { - // skip %2F - if (value1 == 2 && value2 == 15) - { - return true; - } - - return false; - } - - private static bool CompareIterators(ref MemoryPoolIterator lhs, ref MemoryPoolIterator rhs) - { - // uses ref parameter to save cost of copying - return (lhs.Block == rhs.Block) && (lhs.Index == rhs.Index); - } - } -} diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/MemoryPoolIterator.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/MemoryPoolIterator.cs index 07491b7377..0f4d3cebe5 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/MemoryPoolIterator.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/MemoryPoolIterator.cs @@ -14,15 +14,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure public struct MemoryPoolIterator { private const int _maxULongByteLength = 20; - private const ulong _xorPowerOfTwoToHighByte = (0x07ul | - 0x06ul << 8 | - 0x05ul << 16 | - 0x04ul << 24 | - 0x03ul << 32 | - 0x02ul << 40 | - 0x01ul << 48 ) + 1; - - private static readonly int _vectorSpan = Vector.Count; [ThreadStatic] private static byte[] _numericBytesScratch; @@ -135,668 +126,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure } } while (true); } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void Skip(int bytesToSkip) - { - var block = _block; - if (block == null && bytesToSkip > 0) - { - ThrowInvalidOperationException_SkipMoreThanAvailable(); - } - - // Always set wasLastBlock before checking .End to avoid race which may cause data loss - var wasLastBlock = block.Next == null; - var following = block.End - _index; - - if (following >= bytesToSkip) - { - _index += bytesToSkip; - return; - } - - if (wasLastBlock) - { - ThrowInvalidOperationException_SkipMoreThanAvailable(); - } - - SkipMultiBlock(bytesToSkip, following); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private void SkipMultiBlock(int bytesToSkip, int following) - { - var block = _block; - do - { - bytesToSkip -= following; - block = block.Next; - var index = block.Start; - - // Always set wasLastBlock before checking .End to avoid race which may cause data loss - var wasLastBlock = block.Next == null; - following = block.End - index; - - if (following >= bytesToSkip) - { - _block = block; - _index = index + bytesToSkip; - return; - } - - if (wasLastBlock) - { - ThrowInvalidOperationException_SkipMoreThanAvailable(); - } - } while (true); - } - - private static void ThrowInvalidOperationException_SkipMoreThanAvailable() - { - throw new InvalidOperationException("Attempted to skip more bytes than available."); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public int Peek() - { - var block = _block; - if (block == null) - { - return -1; - } - - var index = _index; - - // Always set wasLastBlock before checking .End to avoid race which may cause data loss - var wasLastBlock = block.Next == null; - if (index < block.End) - { - return block.Array[index]; - } - - return wasLastBlock ? -1 : PeekMultiBlock(); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private int PeekMultiBlock() - { - var block = _block; - do - { - block = block.Next; - var index = block.Start; - - // Always set wasLastBlock before checking .End to avoid race which may cause data loss - var wasLastBlock = block.Next == null; - - if (index < block.End) - { - return block.Array[index]; - } - if (wasLastBlock) - { - return -1; - } - } while (true); - } - - // NOTE: Little-endian only! - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public unsafe bool TryPeekLong(out ulong longValue) - { - longValue = 0; - - var block = _block; - if (block == null) - { - return false; - } - - // Always set wasLastBlock before checking .End to avoid race which may cause data loss - var wasLastBlock = block.Next == null; - var blockBytes = block.End - _index; - - if (blockBytes >= sizeof(ulong)) - { - longValue = *(ulong*)(block.DataFixedPtr + _index); - return true; - } - - return wasLastBlock ? false : TryPeekLongMultiBlock(ref longValue, blockBytes); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private unsafe bool TryPeekLongMultiBlock(ref ulong longValue, int blockBytes) - { - // Each block will be filled with at least 2048 bytes before the Next pointer is set, so a long - // will cross at most one block boundary assuming there are at least 8 bytes following the iterator. - var nextBytes = sizeof(ulong) - blockBytes; - - var block = _block; - if (block.Next.End - block.Next.Start < nextBytes) - { - return false; - } - - var nextLong = *(ulong*)(block.Next.DataFixedPtr + block.Next.Start); - - if (blockBytes == 0) - { - // This case can not fall through to the else block since that would cause a 64-bit right shift - // on blockLong which is equivalent to no shift at all instead of shifting in all zeros. - // https://msdn.microsoft.com/en-us/library/xt18et0d.aspx - longValue = nextLong; - } - else - { - var blockLong = *(ulong*)(block.DataFixedPtr + block.End - sizeof(ulong)); - - // Ensure that the right shift has a ulong operand so a logical shift is performed. - longValue = (blockLong >> nextBytes * 8) | (nextLong << blockBytes * 8); - } - - return true; - } - - public int Seek(byte byte0) - { - int bytesScanned; - return Seek(byte0, out bytesScanned); - } - - public unsafe int Seek( - byte byte0, - out int bytesScanned, - int limit = int.MaxValue) - { - bytesScanned = 0; - - var block = _block; - if (block == null || limit <= 0) - { - return -1; - } - - var index = _index; - var wasLastBlock = block.Next == null; - var following = block.End - index; - byte[] array; - var byte0Vector = GetVector(byte0); - - while (true) - { - while (following == 0) - { - if (bytesScanned >= limit || wasLastBlock) - { - _block = block; - _index = index; - return -1; - } - - block = block.Next; - index = block.Start; - wasLastBlock = block.Next == null; - following = block.End - index; - } - array = block.Array; - while (following > 0) - { - // Need unit tests to test Vector path -#if !DEBUG - // Check will be Jitted away https://github.com/dotnet/coreclr/issues/1079 - if (Vector.IsHardwareAccelerated) - { -#endif - if (following >= _vectorSpan) - { - var byte0Equals = Vector.Equals(new Vector(array, index), byte0Vector); - - if (byte0Equals.Equals(Vector.Zero)) - { - if (bytesScanned + _vectorSpan >= limit) - { - _block = block; - // Ensure iterator is left at limit position - _index = index + (limit - bytesScanned); - bytesScanned = limit; - return -1; - } - - bytesScanned += _vectorSpan; - following -= _vectorSpan; - index += _vectorSpan; - continue; - } - - _block = block; - - var firstEqualByteIndex = LocateFirstFoundByte(byte0Equals); - var vectorBytesScanned = firstEqualByteIndex + 1; - - if (bytesScanned + vectorBytesScanned > limit) - { - // Ensure iterator is left at limit position - _index = index + (limit - bytesScanned); - bytesScanned = limit; - return -1; - } - - _index = index + firstEqualByteIndex; - bytesScanned += vectorBytesScanned; - - return byte0; - } - // Need unit tests to test Vector path -#if !DEBUG - } -#endif - - var pCurrent = (block.DataFixedPtr + index); - var pEnd = pCurrent + Math.Min(following, limit - bytesScanned); - do - { - bytesScanned++; - if (*pCurrent == byte0) - { - _block = block; - _index = index; - return byte0; - } - pCurrent++; - index++; - } while (pCurrent < pEnd); - - following = 0; - break; - } - } - } - - public unsafe int Seek( - byte byte0, - ref MemoryPoolIterator limit) - { - var block = _block; - if (block == null) - { - return -1; - } - - var index = _index; - var wasLastBlock = block.Next == null; - var following = block.End - index; - - while (true) - { - while (following == 0) - { - if ((block == limit.Block && index > limit.Index) || - wasLastBlock) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - block = block.Next; - index = block.Start; - wasLastBlock = block.Next == null; - following = block.End - index; - } - var array = block.Array; - while (following > 0) - { -// Need unit tests to test Vector path -#if !DEBUG - // Check will be Jitted away https://github.com/dotnet/coreclr/issues/1079 - if (Vector.IsHardwareAccelerated) - { -#endif - if (following >= _vectorSpan) - { - var byte0Equals = Vector.Equals(new Vector(array, index), GetVector(byte0)); - - if (byte0Equals.Equals(Vector.Zero)) - { - if (block == limit.Block && index + _vectorSpan > limit.Index) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - following -= _vectorSpan; - index += _vectorSpan; - continue; - } - - _block = block; - - var firstEqualByteIndex = LocateFirstFoundByte(byte0Equals); - - if (_block == limit.Block && index + firstEqualByteIndex > limit.Index) - { - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - _index = index + firstEqualByteIndex; - - return byte0; - } -// Need unit tests to test Vector path -#if !DEBUG - } -#endif - - var pCurrent = (block.DataFixedPtr + index); - var pEnd = block == limit.Block ? block.DataFixedPtr + limit.Index + 1 : pCurrent + following; - do - { - if (*pCurrent == byte0) - { - _block = block; - _index = index; - return byte0; - } - pCurrent++; - index++; - } while (pCurrent < pEnd); - - following = 0; - break; - } - } - } - - public int Seek(byte byte0, byte byte1) - { - var limit = new MemoryPoolIterator(); - return Seek(byte0, byte1, ref limit); - } - - public unsafe int Seek( - byte byte0, - byte byte1, - ref MemoryPoolIterator limit) - { - var block = _block; - if (block == null) - { - return -1; - } - - var index = _index; - var wasLastBlock = block.Next == null; - var following = block.End - index; - int byteIndex = int.MaxValue; - - while (true) - { - while (following == 0) - { - if ((block == limit.Block && index > limit.Index) || - wasLastBlock) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - block = block.Next; - index = block.Start; - wasLastBlock = block.Next == null; - following = block.End - index; - } - var array = block.Array; - while (following > 0) - { - -// Need unit tests to test Vector path -#if !DEBUG - // Check will be Jitted away https://github.com/dotnet/coreclr/issues/1079 - if (Vector.IsHardwareAccelerated) - { -#endif - if (following >= _vectorSpan) - { - var data = new Vector(array, index); - - var byteEquals = Vector.Equals(data, GetVector(byte0)); - byteEquals = Vector.ConditionalSelect(byteEquals, byteEquals, Vector.Equals(data, GetVector(byte1))); - - if (!byteEquals.Equals(Vector.Zero)) - { - byteIndex = LocateFirstFoundByte(byteEquals); - } - - if (byteIndex == int.MaxValue) - { - following -= _vectorSpan; - index += _vectorSpan; - - if (block == limit.Block && index > limit.Index) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - continue; - } - - _block = block; - - _index = index + byteIndex; - - if (block == limit.Block && _index > limit.Index) - { - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - _index = index + byteIndex; - - if (block == limit.Block && _index > limit.Index) - { - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - return block.Array[index + byteIndex]; - } -// Need unit tests to test Vector path -#if !DEBUG - } -#endif - var pCurrent = (block.DataFixedPtr + index); - var pEnd = block == limit.Block ? block.DataFixedPtr + limit.Index + 1 : pCurrent + following; - do - { - if (*pCurrent == byte0) - { - _block = block; - _index = index; - return byte0; - } - if (*pCurrent == byte1) - { - _block = block; - _index = index; - return byte1; - } - pCurrent++; - index++; - } while (pCurrent != pEnd); - - following = 0; - break; - } - } - } - - public int Seek(byte byte0, byte byte1, byte byte2) - { - var limit = new MemoryPoolIterator(); - return Seek(byte0, byte1, byte2, ref limit); - } - - public unsafe int Seek( - byte byte0, - byte byte1, - byte byte2, - ref MemoryPoolIterator limit) - { - var block = _block; - if (block == null) - { - return -1; - } - - var index = _index; - var wasLastBlock = block.Next == null; - var following = block.End - index; - int byteIndex = int.MaxValue; - - while (true) - { - while (following == 0) - { - if ((block == limit.Block && index > limit.Index) || - wasLastBlock) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - block = block.Next; - index = block.Start; - wasLastBlock = block.Next == null; - following = block.End - index; - } - var array = block.Array; - while (following > 0) - { -// Need unit tests to test Vector path -#if !DEBUG - // Check will be Jitted away https://github.com/dotnet/coreclr/issues/1079 - if (Vector.IsHardwareAccelerated) - { -#endif - if (following >= _vectorSpan) - { - var data = new Vector(array, index); - - var byteEquals = Vector.Equals(data, GetVector(byte0)); - byteEquals = Vector.ConditionalSelect(byteEquals, byteEquals, Vector.Equals(data, GetVector(byte1))); - byteEquals = Vector.ConditionalSelect(byteEquals, byteEquals, Vector.Equals(data, GetVector(byte2))); - - if (!byteEquals.Equals(Vector.Zero)) - { - byteIndex = LocateFirstFoundByte(byteEquals); - } - - if (byteIndex == int.MaxValue) - { - following -= _vectorSpan; - index += _vectorSpan; - - if (block == limit.Block && index > limit.Index) - { - _block = block; - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - continue; - } - - _block = block; - - _index = index + byteIndex; - - if (block == limit.Block && _index > limit.Index) - { - // Ensure iterator is left at limit position - _index = limit.Index; - return -1; - } - - return block.Array[index + byteIndex]; - } -// Need unit tests to test Vector path -#if !DEBUG - } -#endif - var pCurrent = (block.DataFixedPtr + index); - var pEnd = block == limit.Block ? block.DataFixedPtr + limit.Index + 1 : pCurrent + following; - do - { - if (*pCurrent == byte0) - { - _block = block; - _index = index; - return byte0; - } - if (*pCurrent == byte1) - { - _block = block; - _index = index; - return byte1; - } - if (*pCurrent == byte2) - { - _block = block; - _index = index; - return byte2; - } - pCurrent++; - index++; - } while (pCurrent != pEnd); - - following = 0; - break; - } - } - } - - /// - /// Locate the first of the found bytes - /// - /// - /// The first index of the result vector - // Force inlining (64 IL bytes, 91 bytes asm) Issue: https://github.com/dotnet/coreclr/issues/7386 - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static int LocateFirstFoundByte(Vector byteEquals) - { - var vector64 = Vector.AsVectorUInt64(byteEquals); - ulong longValue = 0; - var i = 0; - // Pattern unrolled by jit https://github.com/dotnet/coreclr/pull/8001 - for (; i < Vector.Count; i++) - { - longValue = vector64[i]; - if (longValue == 0) continue; - break; - } - - // Flag least significant power of two bit - var powerOfTwoFlag = (longValue ^ (longValue - 1)); - // Shift all powers of two into the high byte and extract - var foundByteIndex = (int)((powerOfTwoFlag * _xorPowerOfTwoToHighByte) >> 57); - // Single LEA instruction with jitted const (using function result) - return i * 8 + foundByteIndex; - } - + /// /// Save the data at the current location then move to the next available space. /// @@ -914,57 +244,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure } } - public MemoryPoolIterator CopyTo(byte[] array, int offset, int count, out int actual) - { - var block = _block; - if (block == null) - { - actual = 0; - return this; - } - - var index = _index; - var remaining = count; - while (true) - { - // Determine if we might attempt to copy data from block.Next before - // calculating "following" so we don't risk skipping data that could - // be added after block.End when we decide to copy from block.Next. - // block.End will always be advanced before block.Next is set. - var wasLastBlock = block.Next == null; - var following = block.End - index; - if (remaining <= following) - { - actual = count; - if (array != null) - { - Buffer.BlockCopy(block.Array, index, array, offset, remaining); - } - return new MemoryPoolIterator(block, index + remaining); - } - else if (wasLastBlock) - { - actual = count - remaining + following; - if (array != null) - { - Buffer.BlockCopy(block.Array, index, array, offset, following); - } - return new MemoryPoolIterator(block, index + following); - } - else - { - if (array != null) - { - Buffer.BlockCopy(block.Array, index, array, offset, following); - } - offset += following; - remaining -= following; - block = block.Next; - index = block.Start; - } - } - } - public void CopyFrom(byte[] data) { CopyFrom(data, 0, data.Length); @@ -1180,177 +459,5 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure CopyFrom(byteBuffer, position, _maxULongByteLength - position); } - - public unsafe string GetAsciiString(ref MemoryPoolIterator end) - { - var block = _block; - if (block == null || end.IsDefault) - { - return null; - } - - var length = GetLength(end); - - if (length == 0) - { - return null; - } - - var inputOffset = _index; - - var asciiString = new string('\0', length); - - fixed (char* outputStart = asciiString) - { - var output = outputStart; - var remaining = length; - - var endBlock = end.Block; - var endIndex = end.Index; - - var outputOffset = 0; - while (true) - { - int following = (block != endBlock ? block.End : endIndex) - inputOffset; - - if (following > 0) - { - if (!AsciiUtilities.TryGetAsciiString(block.DataFixedPtr + inputOffset, output + outputOffset, following)) - { - throw BadHttpRequestException.GetException(RequestRejectionReason.NonAsciiOrNullCharactersInInputString); - } - - outputOffset += following; - remaining -= following; - } - - if (remaining == 0) - { - break; - } - - block = block.Next; - inputOffset = block.Start; - } - } - - return asciiString; - } - - public string GetUtf8String(ref MemoryPoolIterator end) - { - var block = _block; - if (block == null || end.IsDefault) - { - return default(string); - } - - var index = _index; - if (end.Block == block) - { - return Encoding.UTF8.GetString(block.Array, index, end.Index - index); - } - - var decoder = Encoding.UTF8.GetDecoder(); - - var length = GetLength(end); - var charLength = length; - // Worse case is 1 byte = 1 char - var chars = new char[charLength]; - var charIndex = 0; - - var remaining = length; - while (true) - { - int bytesUsed; - int charsUsed; - bool completed; - var following = block.End - index; - if (remaining <= following) - { - decoder.Convert( - block.Array, - index, - remaining, - chars, - charIndex, - charLength - charIndex, - true, - out bytesUsed, - out charsUsed, - out completed); - return new string(chars, 0, charIndex + charsUsed); - } - else if (block.Next == null) - { - decoder.Convert( - block.Array, - index, - following, - chars, - charIndex, - charLength - charIndex, - true, - out bytesUsed, - out charsUsed, - out completed); - return new string(chars, 0, charIndex + charsUsed); - } - else - { - decoder.Convert( - block.Array, - index, - following, - chars, - charIndex, - charLength - charIndex, - false, - out bytesUsed, - out charsUsed, - out completed); - charIndex += charsUsed; - remaining -= following; - block = block.Next; - index = block.Start; - } - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public ArraySegment GetArraySegment(MemoryPoolIterator end) - { - var block = _block; - if (block == null || end.IsDefault) - { - return default(ArraySegment); - } - - var index = _index; - if (end.Block == block) - { - return new ArraySegment(block.Array, index, end.Index - index); - } - - return GetArraySegmentMultiBlock(ref end); - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private ArraySegment GetArraySegmentMultiBlock(ref MemoryPoolIterator end) - { - var length = GetLength(end); - var array = new byte[length]; - CopyTo(array, 0, length, out length); - return new ArraySegment(array, 0, length); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static Vector GetVector(byte vectorByte) - { - // Vector .ctor doesn't become an intrinsic due to detection issue - // However this does cause it to become an intrinsic (with additional multiply and reg->reg copy) - // https://github.com/dotnet/coreclr/issues/7459#issuecomment-253965670 - return Vector.AsVectorByte(new Vector(vectorByte * 0x01010101u)); - } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs index 220ae3d80b..5252a96e9a 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/AsciiDecoding.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Linq; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; @@ -14,27 +15,16 @@ namespace Microsoft.AspNetCore.Server.KestrelTests private void FullAsciiRangeSupported() { var byteRange = Enumerable.Range(1, 127).Select(x => (byte)x).ToArray(); - using (var pool = new MemoryPool()) + var s = new Span(byteRange).GetAsciiStringNonNullCharacters(); + + Assert.Equal(s.Length, byteRange.Length); + + for (var i = 1; i < byteRange.Length; i++) { - var mem = pool.Lease(); - mem.GetIterator().CopyFrom(byteRange); + var sb = (byte)s[i]; + var b = byteRange[i]; - var begin = mem.GetIterator(); - var end = GetIterator(begin, byteRange.Length); - - var s = begin.GetAsciiString(ref end); - - Assert.Equal(s.Length, byteRange.Length); - - for (var i = 1; i < byteRange.Length; i++) - { - var sb = (byte)s[i]; - var b = byteRange[i]; - - Assert.Equal(sb, b); - } - - pool.Return(mem); + Assert.Equal(sb, b); } } @@ -49,123 +39,29 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { var byteRange = Enumerable.Range(1, length).Select(x => (byte)x).ToArray(); byteRange[position] = b; - - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - mem.GetIterator().CopyFrom(byteRange); - - var begin = mem.GetIterator(); - var end = GetIterator(begin, byteRange.Length); - - Assert.Throws(() => begin.GetAsciiString(ref end)); - - pool.Return(mem); - } + + Assert.Throws(() => new Span(byteRange).GetAsciiStringNonNullCharacters()); } } } - [Fact] - private void MultiBlockProducesCorrectResults() - { - var byteRange = Enumerable.Range(0, 512 + 64).Select(x => (byte)((x & 0x7f) | 0x01)).ToArray(); - var expectedByteRange = byteRange - .Concat(byteRange) - .Concat(byteRange) - .Concat(byteRange) - .ToArray(); - - using (var pool = new MemoryPool()) - { - var mem0 = pool.Lease(); - var mem1 = pool.Lease(); - var mem2 = pool.Lease(); - var mem3 = pool.Lease(); - mem0.GetIterator().CopyFrom(byteRange); - mem1.GetIterator().CopyFrom(byteRange); - mem2.GetIterator().CopyFrom(byteRange); - mem3.GetIterator().CopyFrom(byteRange); - - mem0.Next = mem1; - mem1.Next = mem2; - mem2.Next = mem3; - - var begin = mem0.GetIterator(); - var end = GetIterator(begin, expectedByteRange.Length); - - var s = begin.GetAsciiString(ref end); - - Assert.Equal(s.Length, expectedByteRange.Length); - - for (var i = 0; i < expectedByteRange.Length; i++) - { - var sb = (byte)((s[i] & 0x7f) | 0x01); - var b = expectedByteRange[i]; - - Assert.Equal(sb, b); - } - - pool.Return(mem0); - pool.Return(mem1); - pool.Return(mem2); - pool.Return(mem3); - } - } - [Fact] private void LargeAllocationProducesCorrectResults() { var byteRange = Enumerable.Range(0, 16384 + 64).Select(x => (byte)((x & 0x7f) | 0x01)).ToArray(); var expectedByteRange = byteRange.Concat(byteRange).ToArray(); - using (var pool = new MemoryPool()) + + var s = new Span(expectedByteRange).GetAsciiStringNonNullCharacters(); + + Assert.Equal(expectedByteRange.Length, s.Length); + + for (var i = 0; i < expectedByteRange.Length; i++) { - var mem0 = pool.Lease(); - var mem1 = pool.Lease(); - mem0.GetIterator().CopyFrom(byteRange); - mem1.GetIterator().CopyFrom(byteRange); + var sb = (byte)((s[i] & 0x7f) | 0x01); + var b = expectedByteRange[i]; - var lastBlock = mem0; - while (lastBlock.Next != null) - { - lastBlock = lastBlock.Next; - } - lastBlock.Next = mem1; - - var begin = mem0.GetIterator(); - var end = GetIterator(begin, expectedByteRange.Length); - - var s = begin.GetAsciiString(ref end); - - Assert.Equal(expectedByteRange.Length, s.Length); - - for (var i = 0; i < expectedByteRange.Length; i++) - { - var sb = (byte)((s[i] & 0x7f) | 0x01); - var b = expectedByteRange[i]; - - Assert.Equal(sb, b); - } - - var block = mem0; - while (block != null) - { - var returnBlock = block; - block = block.Next; - pool.Return(returnBlock); - } + Assert.Equal(sb, b); } } - - private MemoryPoolIterator GetIterator(MemoryPoolIterator begin, int displacement) - { - var result = begin; - for (int i = 0; i < displacement; ++i) - { - result.Take(); - } - - return result; - } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolBlockTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolBlockTests.cs index 4cde6c079a..2213f81f07 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolBlockTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolBlockTests.cs @@ -8,105 +8,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public class MemoryPoolBlockTests { - [Fact] - public void SeekWorks() - { - using (var pool = new MemoryPool()) - { - var block = pool.Lease(); - foreach (var ch in Enumerable.Range(0, 256).Select(x => (byte)x)) - { - block.Array[block.End++] = ch; - } - - var iterator = block.GetIterator(); - foreach (var ch in Enumerable.Range(0, 256).Select(x => (byte)x)) - { - var hit = iterator; - hit.Seek(ch); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(ch, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(byte.MaxValue, ch); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(ch, byte.MaxValue, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(byte.MaxValue, ch, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(ch, byte.MaxValue, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - } - - pool.Return(block); - } - } - - [Fact] - public void SeekWorksAcrossBlocks() - { - using (var pool = new MemoryPool()) - { - var block1 = pool.Lease(); - var block2 = block1.Next = pool.Lease(); - var block3 = block2.Next = pool.Lease(); - - foreach (var ch in Enumerable.Range(0, 34).Select(x => (byte)x)) - { - block1.Array[block1.End++] = ch; - } - foreach (var ch in Enumerable.Range(34, 25).Select(x => (byte)x)) - { - block2.Array[block2.End++] = ch; - } - foreach (var ch in Enumerable.Range(59, 197).Select(x => (byte)x)) - { - block3.Array[block3.End++] = ch; - } - - var iterator = block1.GetIterator(); - foreach (var ch in Enumerable.Range(0, 256).Select(x => (byte)x)) - { - var hit = iterator; - hit.Seek(ch); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(ch, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(byte.MaxValue, ch); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(ch, byte.MaxValue, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(byte.MaxValue, ch, byte.MaxValue); - Assert.Equal(ch, iterator.GetLength(hit)); - - hit = iterator; - hit.Seek(byte.MaxValue, byte.MaxValue, ch); - Assert.Equal(ch, iterator.GetLength(hit)); - } - - pool.Return(block1); - pool.Return(block2); - pool.Return(block3); - } - } - + [Fact] public void GetLengthBetweenIteratorsWorks() { @@ -192,87 +94,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests pool.Return(block2); } } - - [Fact] - public void CopyToCorrectlyTraversesBlocks() - { - using (var pool = new MemoryPool()) - { - var block1 = pool.Lease(); - var block2 = block1.Next = pool.Lease(); - - for (int i = 0; i < 128; i++) - { - block1.Array[block1.End++] = (byte)i; - } - for (int i = 128; i < 256; i++) - { - block2.Array[block2.End++] = (byte)i; - } - - var beginIterator = block1.GetIterator(); - - var array = new byte[256]; - int actual; - var endIterator = beginIterator.CopyTo(array, 0, 256, out actual); - - Assert.Equal(256, actual); - - for (int i = 0; i < 256; i++) - { - Assert.Equal(i, array[i]); - } - - endIterator.CopyTo(array, 0, 256, out actual); - Assert.Equal(0, actual); - - pool.Return(block1); - pool.Return(block2); - } - } - - [Fact] - public void CopyFromCorrectlyTraversesBlocks() - { - using (var pool = new MemoryPool()) - { - var block1 = pool.Lease(); - var start = block1.GetIterator(); - var end = start; - var bufferSize = block1.Data.Count * 3; - var buffer = new byte[bufferSize]; - - for (int i = 0; i < bufferSize; i++) - { - buffer[i] = (byte)(i % 73); - } - - Assert.Null(block1.Next); - - end.CopyFrom(new ArraySegment(buffer)); - - Assert.NotNull(block1.Next); - - for (int i = 0; i < bufferSize; i++) - { - Assert.Equal(i % 73, start.Take()); - } - - Assert.Equal(-1, start.Take()); - Assert.Equal(start.Block, end.Block); - Assert.Equal(start.Index, end.Index); - - var block = block1; - while (block != null) - { - var returnBlock = block; - block = block.Next; - - pool.Return(returnBlock); - } - } - } - + [Fact] public void IsEndCorrectlyTraversesBlocks() { diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolExtensions.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolExtensions.cs index 6348258ce9..1d62dd2793 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolExtensions.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolExtensions.cs @@ -6,8 +6,11 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public static MemoryPoolIterator Add(this MemoryPoolIterator iterator, int count) { - int actual; - return iterator.CopyTo(new byte[count], 0, count, out actual); + for (int i = 0; i < count; i++) + { + iterator.Take(); + } + return iterator; } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolIteratorTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolIteratorTests.cs index 83cf36c4c8..4643b5236b 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolIteratorTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/MemoryPoolIteratorTests.cs @@ -30,80 +30,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _pool.Dispose(); } - [Fact] - public void TestFindFirstEqualByte() - { - var bytes = Enumerable.Repeat(0xff, Vector.Count).ToArray(); - for (int i = 0; i < Vector.Count; i++) - { - Vector vector = new Vector(bytes); - Assert.Equal(i, MemoryPoolIterator.LocateFirstFoundByte(vector)); - bytes[i] = 0; - } - - for (int i = 0; i < Vector.Count; i++) - { - bytes[i] = 1; - Vector vector = new Vector(bytes); - Assert.Equal(i, MemoryPoolIterator.LocateFirstFoundByte(vector)); - bytes[i] = 0; - } - } - - [Theory] - [InlineData("a", "a", 'a', 0)] - [InlineData("ab", "a", 'a', 0)] - [InlineData("aab", "a", 'a', 0)] - [InlineData("acab", "a", 'a', 0)] - [InlineData("acab", "c", 'c', 1)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "lo", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "ol", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "ll", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "lmr", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "rml", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyz", "mlr", 'l', 11)] - [InlineData("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "lmr", 'l', 11)] - [InlineData("aaaaaaaaaaalmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "lmr", 'l', 11)] - [InlineData("aaaaaaaaaaacmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "lmr", 'm', 12)] - [InlineData("aaaaaaaaaaarmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz", "lmr", 'r', 11)] - [InlineData("/localhost:5000/PATH/%2FPATH2/ HTTP/1.1", " %?", '%', 21)] - [InlineData("/localhost:5000/PATH/%2FPATH2/?key=value HTTP/1.1", " %?", '%', 21)] - [InlineData("/localhost:5000/PATH/PATH2/?key=value HTTP/1.1", " %?", '?', 27)] - [InlineData("/localhost:5000/PATH/PATH2/ HTTP/1.1", " %?", ' ', 27)] - public void MemorySeek(string raw, string search, char expectResult, int expectIndex) - { - var block = _pool.Lease(); - var chars = raw.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars, 0, block.Array, block.Start, chars.Length); - block.End += chars.Length; - - var begin = block.GetIterator(); - var searchFor = search.ToCharArray(); - - int found = -1; - if (searchFor.Length == 1) - { - found = begin.Seek((byte)searchFor[0]); - } - else if (searchFor.Length == 2) - { - found = begin.Seek((byte)searchFor[0], (byte)searchFor[1]); - } - else if (searchFor.Length == 3) - { - found = begin.Seek((byte)searchFor[0], (byte)searchFor[1], (byte)searchFor[2]); - } - else - { - Assert.False(true, "Invalid test sample."); - } - - Assert.Equal(expectResult, found); - Assert.Equal(expectIndex, begin.Index - block.Start); - - _pool.Return(block); - } - [Fact] public void Put() { @@ -241,728 +167,20 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } - [Fact] - public void PeekLong() - { - // Arrange - var block = _pool.Lease(); - var bytes = BitConverter.GetBytes(0x0102030405060708UL); - Buffer.BlockCopy(bytes, 0, block.Array, block.Start, bytes.Length); - block.End += bytes.Length; - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Assert - ulong result; - Assert.True(scan.TryPeekLong(out result)); - Assert.Equal(0x0102030405060708UL, result); - Assert.Equal(originalIndex, scan.Index); - - _pool.Return(block); - } - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - [InlineData(3)] - [InlineData(4)] - [InlineData(5)] - [InlineData(6)] - [InlineData(7)] - public void PeekLongNotEnoughBytes(int totalBytes) - { - // Arrange - var block = _pool.Lease(); - var bytes = BitConverter.GetBytes(0x0102030405060708UL); - var bytesLength = totalBytes; - Buffer.BlockCopy(bytes, 0, block.Array, block.Start, bytesLength); - block.End += bytesLength; - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Assert - ulong result; - Assert.False(scan.TryPeekLong(out result)); - Assert.Equal(originalIndex, scan.Index); - _pool.Return(block); - } - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - [InlineData(3)] - [InlineData(4)] - [InlineData(5)] - [InlineData(6)] - [InlineData(7)] - public void PeekLongNotEnoughBytesAtBlockBoundary(int firstBlockBytes) - { - // Arrange - var expectedResult = 0x0102030405060708UL; - var nextBlockBytes = 7 - firstBlockBytes; - - var block = _pool.Lease(); - block.End += firstBlockBytes; - - var nextBlock = _pool.Lease(); - nextBlock.End += nextBlockBytes; - - block.Next = nextBlock; - - var bytes = BitConverter.GetBytes(expectedResult); - Buffer.BlockCopy(bytes, 0, block.Array, block.Start, firstBlockBytes); - Buffer.BlockCopy(bytes, firstBlockBytes, nextBlock.Array, nextBlock.Start, nextBlockBytes); - - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Assert - ulong result; - Assert.False(scan.TryPeekLong(out result)); - Assert.Equal(originalIndex, scan.Index); - - _pool.Return(block); - _pool.Return(nextBlock); - } - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - [InlineData(3)] - [InlineData(4)] - [InlineData(5)] - [InlineData(6)] - [InlineData(7)] - [InlineData(8)] - public void PeekLongAtBlockBoundary(int firstBlockBytes) - { - // Arrange - var expectedResult = 0x0102030405060708UL; - var nonZeroData = 0xFF00FFFF0000FFFFUL; - var nextBlockBytes = 8 - firstBlockBytes; - - var block = _pool.Lease(); - block.Start += 8; - block.End = block.Start + firstBlockBytes; - - var nextBlock = _pool.Lease(); - nextBlock.Start += 8; - nextBlock.End = nextBlock.Start + nextBlockBytes; - - block.Next = nextBlock; - - var bytes = BitConverter.GetBytes(expectedResult); - Buffer.BlockCopy(bytes, 0, block.Array, block.Start, firstBlockBytes); - Buffer.BlockCopy(bytes, firstBlockBytes, nextBlock.Array, nextBlock.Start, nextBlockBytes); - - // Fill in surrounding bytes with non-zero data - var nonZeroBytes = BitConverter.GetBytes(nonZeroData); - Buffer.BlockCopy(nonZeroBytes, 0, block.Array, block.Start - 8, 8); - Buffer.BlockCopy(nonZeroBytes, 0, block.Array, block.End, 8); - Buffer.BlockCopy(nonZeroBytes, 0, nextBlock.Array, nextBlock.Start - 8, 8); - Buffer.BlockCopy(nonZeroBytes, 0, nextBlock.Array, nextBlock.End, 8); - - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Assert - ulong result; - Assert.True(scan.TryPeekLong(out result)); - Assert.Equal(expectedResult, result); - Assert.Equal(originalIndex, scan.Index); - - _pool.Return(block); - _pool.Return(nextBlock); - } - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - [InlineData(3)] - [InlineData(4)] - [InlineData(5)] - [InlineData(6)] - [InlineData(7)] - [InlineData(8)] - public void PeekLongAtBlockBoundarayWithMostSignificatBitsSet(int firstBlockBytes) - { - // Arrange - var expectedResult = 0xFF02030405060708UL; - var nonZeroData = 0xFF00FFFF0000FFFFUL; - var nextBlockBytes = 8 - firstBlockBytes; - - var block = _pool.Lease(); - block.Start += 8; - block.End = block.Start + firstBlockBytes; - - var nextBlock = _pool.Lease(); - nextBlock.Start += 8; - nextBlock.End = nextBlock.Start + nextBlockBytes; - - block.Next = nextBlock; - - var expectedBytes = BitConverter.GetBytes(expectedResult); - Buffer.BlockCopy(expectedBytes, 0, block.Array, block.Start, firstBlockBytes); - Buffer.BlockCopy(expectedBytes, firstBlockBytes, nextBlock.Array, nextBlock.Start, nextBlockBytes); - - // Fill in surrounding bytes with non-zero data - var nonZeroBytes = BitConverter.GetBytes(nonZeroData); - Buffer.BlockCopy(nonZeroBytes, 0, block.Array, block.Start - 8, 8); - Buffer.BlockCopy(nonZeroBytes, 0, block.Array, block.End, 8); - Buffer.BlockCopy(nonZeroBytes, 0, nextBlock.Array, nextBlock.Start - 8, 8); - Buffer.BlockCopy(nonZeroBytes, 0, nextBlock.Array, nextBlock.End, 8); - - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Assert - ulong result; - Assert.True(scan.TryPeekLong(out result)); - Assert.Equal(expectedResult, result); - Assert.Equal(originalIndex, scan.Index); - - _pool.Return(block); - _pool.Return(nextBlock); - } - - [Theory] - [InlineData(0)] - [InlineData(1)] - [InlineData(2)] - [InlineData(3)] - [InlineData(4)] - [InlineData(5)] - [InlineData(6)] - [InlineData(7)] - [InlineData(8)] - [InlineData(9)] - public void SkipAtBlockBoundary(int blockBytes) - { - // Arrange - var nextBlockBytes = 10 - blockBytes; - - var block = _pool.Lease(); - block.End += blockBytes; - - var nextBlock = _pool.Lease(); - nextBlock.End += nextBlockBytes; - - block.Next = nextBlock; - - var bytes = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 }; - Buffer.BlockCopy(bytes, 0, block.Array, block.Start, blockBytes); - Buffer.BlockCopy(bytes, blockBytes, nextBlock.Array, nextBlock.Start, nextBlockBytes); - - var scan = block.GetIterator(); - var originalIndex = scan.Index; - - // Act - scan.Skip(8); - var result = scan.Take(); - - // Assert - Assert.Equal(0x08, result); - Assert.NotEqual(originalIndex, scan.Index); - - _pool.Return(block); - _pool.Return(nextBlock); - } - - [Fact] - public void SkipThrowsWhenSkippingMoreBytesThanAvailableInSingleBlock() - { - // Arrange - var block = _pool.Lease(); - block.End += 5; - - var scan = block.GetIterator(); - - // Act/Assert - Assert.ThrowsAny(() => scan.Skip(8)); - - _pool.Return(block); - } - - [Fact] - public void SkipThrowsWhenSkippingMoreBytesThanAvailableInMultipleBlocks() - { - // Arrange - var firstBlock = _pool.Lease(); - firstBlock.End += 3; - - var middleBlock = _pool.Lease(); - middleBlock.End += 1; - firstBlock.Next = middleBlock; - - var finalBlock = _pool.Lease(); - finalBlock.End += 2; - middleBlock.Next = finalBlock; - - var scan = firstBlock.GetIterator(); - - // Act/Assert - Assert.ThrowsAny(() => scan.Skip(8)); - - _pool.Return(firstBlock); - _pool.Return(middleBlock); - _pool.Return(finalBlock); - } - - - [Theory] - [MemberData(nameof(SeekByteLimitData))] - public void TestSeekByteLimitWithinSameBlock(string input, char seek, int limit, int expectedBytesScanned, int expectedReturnValue) - { - MemoryPoolBlock block = null; - - try - { - // Arrange - - block = _pool.Lease(); - var chars = input.ToString().ToCharArray().Select(c => (byte) c).ToArray(); - Buffer.BlockCopy(chars, 0, block.Array, block.Start, chars.Length); - block.End += chars.Length; - var scan = block.GetIterator(); - - // Act - int bytesScanned; - var returnValue = scan.Seek((byte)seek, out bytesScanned, limit); - - // Assert - Assert.Equal(expectedBytesScanned, bytesScanned); - Assert.Equal(expectedReturnValue, returnValue); - - Assert.Same(block, scan.Block); - var expectedEndIndex = expectedReturnValue != -1 ? - block.Start + input.IndexOf(seek) : - block.Start + expectedBytesScanned; - Assert.Equal(expectedEndIndex, scan.Index); - } - finally - { - // Cleanup - if (block != null) _pool.Return(block); - } - } - - [Theory] - [MemberData(nameof(SeekByteLimitData))] - public void TestSeekByteLimitAcrossBlocks(string input, char seek, int limit, int expectedBytesScanned, int expectedReturnValue) - { - MemoryPoolBlock block1 = null; - MemoryPoolBlock block2 = null; - MemoryPoolBlock emptyBlock = null; - - try - { - // Arrange - var input1 = input.Substring(0, input.Length / 2); - block1 = _pool.Lease(); - var chars1 = input1.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars1, 0, block1.Array, block1.Start, chars1.Length); - block1.End += chars1.Length; - - emptyBlock = _pool.Lease(); - block1.Next = emptyBlock; - - var input2 = input.Substring(input.Length / 2); - block2 = _pool.Lease(); - var chars2 = input2.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars2, 0, block2.Array, block2.Start, chars2.Length); - block2.End += chars2.Length; - emptyBlock.Next = block2; - - var scan = block1.GetIterator(); - - // Act - int bytesScanned; - var returnValue = scan.Seek((byte)seek, out bytesScanned, limit); - - // Assert - Assert.Equal(expectedBytesScanned, bytesScanned); - Assert.Equal(expectedReturnValue, returnValue); - - var seekCharIndex = input.IndexOf(seek); - var expectedEndBlock = limit <= input.Length / 2 ? - block1 : - (seekCharIndex != -1 && seekCharIndex < input.Length / 2 ? block1 : block2); - Assert.Same(expectedEndBlock, scan.Block); - var expectedEndIndex = expectedReturnValue != -1 ? - expectedEndBlock.Start + (expectedEndBlock == block1 ? input1.IndexOf(seek) : input2.IndexOf(seek)) : - expectedEndBlock.Start + (expectedEndBlock == block1 ? expectedBytesScanned : expectedBytesScanned - (input.Length / 2)); - Assert.Equal(expectedEndIndex, scan.Index); - } - finally - { - // Cleanup - if (block1 != null) _pool.Return(block1); - if (emptyBlock != null) _pool.Return(emptyBlock); - if (block2 != null) _pool.Return(block2); - } - } - - [Theory] - [MemberData(nameof(SeekIteratorLimitData))] - public void TestSeekIteratorLimitWithinSameBlock(string input, char seek, char limitAt, int expectedReturnValue) - { - MemoryPoolBlock block = null; - - try - { - // Arrange - var afterSeek = (byte)'B'; - - block = _pool.Lease(); - var chars = input.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars, 0, block.Array, block.Start, chars.Length); - block.End += chars.Length; - var scan1 = block.GetIterator(); - var scan2_1 = scan1; - var scan2_2 = scan1; - var scan3_1 = scan1; - var scan3_2 = scan1; - var scan3_3 = scan1; - var end = scan1; - - // Act - var endReturnValue = end.Seek((byte)limitAt); - var returnValue1 = scan1.Seek((byte)seek, ref end); - var returnValue2_1 = scan2_1.Seek((byte)seek, afterSeek, ref end); - var returnValue2_2 = scan2_2.Seek(afterSeek, (byte)seek, ref end); - var returnValue3_1 = scan3_1.Seek((byte)seek, afterSeek, afterSeek, ref end); - var returnValue3_2 = scan3_2.Seek(afterSeek, (byte)seek, afterSeek, ref end); - var returnValue3_3 = scan3_3.Seek(afterSeek, afterSeek, (byte)seek, ref end); - - // Assert - Assert.Equal(input.Contains(limitAt) ? limitAt : -1, endReturnValue); - Assert.Equal(expectedReturnValue, returnValue1); - Assert.Equal(expectedReturnValue, returnValue2_1); - Assert.Equal(expectedReturnValue, returnValue2_2); - Assert.Equal(expectedReturnValue, returnValue3_1); - Assert.Equal(expectedReturnValue, returnValue3_2); - Assert.Equal(expectedReturnValue, returnValue3_3); - - Assert.Same(block, scan1.Block); - Assert.Same(block, scan2_1.Block); - Assert.Same(block, scan2_2.Block); - Assert.Same(block, scan3_1.Block); - Assert.Same(block, scan3_2.Block); - Assert.Same(block, scan3_3.Block); - - var expectedEndIndex = expectedReturnValue != -1 ? block.Start + input.IndexOf(seek) : end.Index; - Assert.Equal(expectedEndIndex, scan1.Index); - Assert.Equal(expectedEndIndex, scan2_1.Index); - Assert.Equal(expectedEndIndex, scan2_2.Index); - Assert.Equal(expectedEndIndex, scan3_1.Index); - Assert.Equal(expectedEndIndex, scan3_2.Index); - Assert.Equal(expectedEndIndex, scan3_3.Index); - } - finally - { - // Cleanup - if (block != null) _pool.Return(block); - } - } - - [Theory] - [MemberData(nameof(SeekIteratorLimitData))] - public void TestSeekIteratorLimitAcrossBlocks(string input, char seek, char limitAt, int expectedReturnValue) - { - MemoryPoolBlock block1 = null; - MemoryPoolBlock block2 = null; - MemoryPoolBlock emptyBlock = null; - - try - { - // Arrange - var afterSeek = (byte)'B'; - - var input1 = input.Substring(0, input.Length / 2); - block1 = _pool.Lease(); - var chars1 = input1.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars1, 0, block1.Array, block1.Start, chars1.Length); - block1.End += chars1.Length; - - emptyBlock = _pool.Lease(); - block1.Next = emptyBlock; - - var input2 = input.Substring(input.Length / 2); - block2 = _pool.Lease(); - var chars2 = input2.ToCharArray().Select(c => (byte)c).ToArray(); - Buffer.BlockCopy(chars2, 0, block2.Array, block2.Start, chars2.Length); - block2.End += chars2.Length; - emptyBlock.Next = block2; - - var scan1 = block1.GetIterator(); - var scan2_1 = scan1; - var scan2_2 = scan1; - var scan3_1 = scan1; - var scan3_2 = scan1; - var scan3_3 = scan1; - var end = scan1; - - // Act - var endReturnValue = end.Seek((byte)limitAt); - var returnValue1 = scan1.Seek((byte)seek, ref end); - var returnValue2_1 = scan2_1.Seek((byte)seek, afterSeek, ref end); - var returnValue2_2 = scan2_2.Seek(afterSeek, (byte)seek, ref end); - var returnValue3_1 = scan3_1.Seek((byte)seek, afterSeek, afterSeek, ref end); - var returnValue3_2 = scan3_2.Seek(afterSeek, (byte)seek, afterSeek, ref end); - var returnValue3_3 = scan3_3.Seek(afterSeek, afterSeek, (byte)seek, ref end); - - // Assert - Assert.Equal(input.Contains(limitAt) ? limitAt : -1, endReturnValue); - Assert.Equal(expectedReturnValue, returnValue1); - Assert.Equal(expectedReturnValue, returnValue2_1); - Assert.Equal(expectedReturnValue, returnValue2_2); - Assert.Equal(expectedReturnValue, returnValue3_1); - Assert.Equal(expectedReturnValue, returnValue3_2); - Assert.Equal(expectedReturnValue, returnValue3_3); - - var seekCharIndex = input.IndexOf(seek); - var limitAtIndex = input.IndexOf(limitAt); - var expectedEndBlock = seekCharIndex != -1 && seekCharIndex < input.Length / 2 ? - block1 : - (limitAtIndex != -1 && limitAtIndex < input.Length / 2 ? block1 : block2); - Assert.Same(expectedEndBlock, scan1.Block); - Assert.Same(expectedEndBlock, scan2_1.Block); - Assert.Same(expectedEndBlock, scan2_2.Block); - Assert.Same(expectedEndBlock, scan3_1.Block); - Assert.Same(expectedEndBlock, scan3_2.Block); - Assert.Same(expectedEndBlock, scan3_3.Block); - - var expectedEndIndex = expectedReturnValue != -1 ? - expectedEndBlock.Start + (expectedEndBlock == block1 ? input1.IndexOf(seek) : input2.IndexOf(seek)) : - end.Index; - Assert.Equal(expectedEndIndex, scan1.Index); - Assert.Equal(expectedEndIndex, scan2_1.Index); - Assert.Equal(expectedEndIndex, scan2_2.Index); - Assert.Equal(expectedEndIndex, scan3_1.Index); - Assert.Equal(expectedEndIndex, scan3_2.Index); - Assert.Equal(expectedEndIndex, scan3_3.Index); - } - finally - { - // Cleanup - if (block1 != null) _pool.Return(block1); - if (emptyBlock != null) _pool.Return(emptyBlock); - if (block2 != null) _pool.Return(block2); - } - } [Fact] public void EmptyIteratorBehaviourIsValid() { const byte byteCr = (byte)'\n'; - ulong longValue; var end = default(MemoryPoolIterator); - - Assert.False(default(MemoryPoolIterator).TryPeekLong(out longValue)); - Assert.Null(default(MemoryPoolIterator).GetAsciiString(ref end)); - Assert.Null(default(MemoryPoolIterator).GetUtf8String(ref end)); - // Assert.Equal doesn't work for default(ArraySegments) - Assert.True(default(MemoryPoolIterator).GetArraySegment(end).Equals(default(ArraySegment))); + Assert.True(default(MemoryPoolIterator).IsDefault); Assert.True(default(MemoryPoolIterator).IsEnd); - Assert.Equal(default(MemoryPoolIterator).Take(), -1); - Assert.Equal(default(MemoryPoolIterator).Peek(), -1); - Assert.Equal(default(MemoryPoolIterator).Seek(byteCr), -1); - Assert.Equal(default(MemoryPoolIterator).Seek(byteCr, ref end), -1); - Assert.Equal(default(MemoryPoolIterator).Seek(byteCr, byteCr), -1); - Assert.Equal(default(MemoryPoolIterator).Seek(byteCr, byteCr, byteCr), -1); default(MemoryPoolIterator).CopyFrom(default(ArraySegment)); default(MemoryPoolIterator).CopyFromAscii(""); Assert.ThrowsAny(() => default(MemoryPoolIterator).Put(byteCr)); Assert.ThrowsAny(() => default(MemoryPoolIterator).GetLength(end)); - Assert.ThrowsAny(() => default(MemoryPoolIterator).Skip(1)); - } - - [Fact] - public void TestGetArraySegment() - { - MemoryPoolBlock block0 = null; - MemoryPoolBlock block1 = null; - - var byteRange = Enumerable.Range(1, 127).Select(x => (byte)x).ToArray(); - try - { - // Arrange - block0 = _pool.Lease(); - block1 = _pool.Lease(); - - block0.GetIterator().CopyFrom(byteRange); - block1.GetIterator().CopyFrom(byteRange); - - block0.Next = block1; - - var begin = block0.GetIterator(); - var end0 = begin; - var end1 = begin; - - end0.Skip(byteRange.Length); - end1.Skip(byteRange.Length * 2); - - // Act - var as0 = begin.GetArraySegment(end0); - var as1 = begin.GetArraySegment(end1); - - // Assert - Assert.Equal(as0.Count, byteRange.Length); - Assert.Equal(as1.Count, byteRange.Length * 2); - - for (var i = 1; i < byteRange.Length; i++) - { - var asb0 = as0.Array[i + as0.Offset]; - var asb1 = as1.Array[i + as1.Offset]; - var b = byteRange[i]; - - Assert.Equal(asb0, b); - Assert.Equal(asb1, b); - } - - for (var i = 1 + byteRange.Length; i < byteRange.Length * 2; i++) - { - var asb1 = as1.Array[i + as1.Offset]; - var b = byteRange[i - byteRange.Length]; - - Assert.Equal(asb1, b); - } - - } - finally - { - if (block0 != null) _pool.Return(block0); - if (block1 != null) _pool.Return(block1); - } - } - - [Fact] - public void TestTake() - { - MemoryPoolBlock block0 = null; - MemoryPoolBlock block1 = null; - MemoryPoolBlock block2 = null; - MemoryPoolBlock emptyBlock0 = null; - MemoryPoolBlock emptyBlock1 = null; - - var byteRange = Enumerable.Range(1, 127).Select(x => (byte)x).ToArray(); - try - { - // Arrange - block0 = _pool.Lease(); - block1 = _pool.Lease(); - block2 = _pool.Lease(); - emptyBlock0 = _pool.Lease(); - emptyBlock1 = _pool.Lease(); - - block0.GetIterator().CopyFrom(byteRange); - block1.GetIterator().CopyFrom(byteRange); - block2.GetIterator().CopyFrom(byteRange); - - var begin = block0.GetIterator(); - - // Single block - for (var i = 0; i < byteRange.Length; i++) - { - var t = begin.Take(); - var b = byteRange[i]; - - Assert.Equal(t, b); - } - - Assert.Equal(begin.Take(), -1); - - // Dual block - block0.Next = block1; - begin = block0.GetIterator(); - - for (var block = 0; block < 2; block++) - { - for (var i = 0; i < byteRange.Length; i++) - { - var t = begin.Take(); - var b = byteRange[i]; - - Assert.Equal(t, b); - } - } - - Assert.Equal(begin.Take(), -1); - - // Multi block - block1.Next = emptyBlock0; - emptyBlock0.Next = emptyBlock1; - emptyBlock1.Next = block2; - begin = block0.GetIterator(); - - for (var block = 0; block < 3; block++) - { - for (var i = 0; i < byteRange.Length; i++) - { - var t = begin.Take(); - var b = byteRange[i]; - - Assert.Equal(t, b); - } - } - - Assert.Equal(begin.Take(), -1); - } - finally - { - if (block0 != null) _pool.Return(block0); - if (block1 != null) _pool.Return(block1); - if (block2 != null) _pool.Return(block2); - if (emptyBlock0 != null) _pool.Return(emptyBlock0); - if (emptyBlock1 != null) _pool.Return(emptyBlock1); - } - } - - [Fact] - public void TestTakeEmptyBlocks() - { - MemoryPoolBlock emptyBlock0 = null; - MemoryPoolBlock emptyBlock1 = null; - MemoryPoolBlock emptyBlock2 = null; - try - { - // Arrange - emptyBlock0 = _pool.Lease(); - emptyBlock1 = _pool.Lease(); - emptyBlock2 = _pool.Lease(); - - var beginEmpty = emptyBlock0.GetIterator(); - - // Assert - - // No blocks - Assert.Equal(default(MemoryPoolIterator).Take(), -1); - - // Single empty block - Assert.Equal(beginEmpty.Take(), -1); - - // Dual empty block - emptyBlock0.Next = emptyBlock1; - beginEmpty = emptyBlock0.GetIterator(); - Assert.Equal(beginEmpty.Take(), -1); - - // Multi empty block - emptyBlock1.Next = emptyBlock2; - beginEmpty = emptyBlock0.GetIterator(); - Assert.Equal(beginEmpty.Take(), -1); - } - finally - { - if (emptyBlock0 != null) _pool.Return(emptyBlock0); - if (emptyBlock1 != null) _pool.Return(emptyBlock1); - if (emptyBlock2 != null) _pool.Return(emptyBlock2); - } } [Theory] @@ -973,23 +191,14 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [InlineData("abcde", "abcde", 6)] public void TestGetAsciiStringEscaped(string input, string expected, int maxChars) { - MemoryPoolBlock block = null; - - try - { // Arrange - var buffer = new Span(Encoding.ASCII.GetBytes(input)); + var buffer = new Span(Encoding.ASCII.GetBytes(input)); - // Act - var result = buffer.GetAsciiStringEscaped(maxChars); + // Act + var result = buffer.GetAsciiStringEscaped(maxChars); - // Assert - Assert.Equal(expected, result); - } - finally - { - if (block != null) _pool.Return(block); - } + // Assert + Assert.Equal(expected, result); } [Fact] diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/UrlPathDecoder.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/UrlPathDecoder.cs deleted file mode 100644 index 0acfc27fe8..0000000000 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/UrlPathDecoder.cs +++ /dev/null @@ -1,226 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Linq; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; -using Xunit; - -namespace Microsoft.AspNetCore.Server.KestrelTests -{ - public class UrlPathDecoderTests - { - [Fact] - public void Empty() - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, string.Empty, string.Empty); - - pool.Return(mem); - } - } - - [Fact] - public void WhiteSpace() - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, " ", " "); - - pool.Return(mem); - } - } - - [Theory] - [InlineData("/foo/bar", "/foo/bar")] - [InlineData("/foo/BAR", "/foo/BAR")] - [InlineData("/foo/", "/foo/")] - [InlineData("/", "/")] - public void NormalCases(string raw, string expect) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, raw, expect); - - pool.Return(mem); - } - } - - [Theory] - [InlineData("%2F", "%2F")] - [InlineData("/foo%2Fbar", "/foo%2Fbar")] - [InlineData("/foo%2F%20bar", "/foo%2F bar")] - public void SkipForwardSlash(string raw, string expect) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, raw, expect); - - pool.Return(mem); - } - } - - [Theory] - [InlineData("%D0%A4", "Ф")] - [InlineData("%d0%a4", "Ф")] - [InlineData("%E0%A4%AD", "भ")] - [InlineData("%e0%A4%Ad", "भ")] - [InlineData("%F0%A4%AD%A2", "𤭢")] - [InlineData("%F0%a4%Ad%a2", "𤭢")] - [InlineData("%48%65%6C%6C%6F%20%57%6F%72%6C%64", "Hello World")] - [InlineData("%48%65%6C%6C%6F%2D%C2%B5%40%C3%9F%C3%B6%C3%A4%C3%BC%C3%A0%C3%A1", "Hello-µ@ßöäüàá")] - // Test the borderline cases of overlong UTF8. - [InlineData("%C2%80", "\u0080")] - [InlineData("%E0%A0%80", "\u0800")] - [InlineData("%F0%90%80%80", "\U00010000")] - [InlineData("%63", "c")] - [InlineData("%32", "2")] - [InlineData("%20", " ")] - public void ValidUTF8(string raw, string expect) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, raw, expect); - - pool.Return(mem); - } - } - - [Theory] - [InlineData("%C3%84ra%20Benetton", "Ära Benetton")] - [InlineData("%E6%88%91%E8%87%AA%E6%A8%AA%E5%88%80%E5%90%91%E5%A4%A9%E7%AC%91%E5%8E%BB%E7%95%99%E8%82%9D%E8%83%86%E4%B8%A4%E6%98%86%E4%BB%91", "我自横刀向天笑去留肝胆两昆仑")] - public void Internationalized(string raw, string expect) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, raw, expect); - - pool.Return(mem); - } - } - - [Theory] - // Overlong ASCII - [InlineData("%C0%A4", "%C0%A4")] - [InlineData("%C1%BF", "%C1%BF")] - [InlineData("%E0%80%AF", "%E0%80%AF")] - [InlineData("%E0%9F%BF", "%E0%9F%BF")] - [InlineData("%F0%80%80%AF", "%F0%80%80%AF")] - [InlineData("%F0%8F%8F%BF", "%F0%8F%8F%BF")] - // Incomplete - [InlineData("%", "%")] - [InlineData("%%", "%%")] - [InlineData("%A", "%A")] - [InlineData("%Y", "%Y")] - // Mixed - [InlineData("%%32", "%2")] - [InlineData("%%20", "% ")] - [InlineData("%C0%A4%32", "%C0%A42")] - [InlineData("%32%C0%A4%32", "2%C0%A42")] - [InlineData("%C0%32%A4", "%C02%A4")] - public void InvalidUTF8(string raw, string expect) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - PositiveAssert(mem, raw, expect); - - pool.Return(mem); - } - } - - [Theory] - [InlineData("/foo%2Fbar", 10, "/foo%2Fbar", 10)] - [InlineData("/foo%2Fbar", 9, "/foo%2Fba", 9)] - [InlineData("/foo%2Fbar", 8, "/foo%2Fb", 8)] - [InlineData("%D0%A4", 6, "Ф", 1)] - [InlineData("%D0%A4", 5, "%D0%A", 5)] - [InlineData("%D0%A4", 4, "%D0%", 4)] - [InlineData("%D0%A4", 3, "%D0", 3)] - [InlineData("%D0%A4", 2, "%D", 2)] - [InlineData("%D0%A4", 1, "%", 1)] - [InlineData("%D0%A4", 0, "", 0)] - [InlineData("%C2%B5%40%C3%9F%C3%B6%C3%A4%C3%BC%C3%A0%C3%A1", 45, "µ@ßöäüàá", 8)] - [InlineData("%C2%B5%40%C3%9F%C3%B6%C3%A4%C3%BC%C3%A0%C3%A1", 44, "µ@ßöäüà%C3%A", 12)] - public void DecodeWithBoundary(string raw, int rawLength, string expect, int expectLength) - { - using (var pool = new MemoryPool()) - { - var mem = pool.Lease(); - - var begin = BuildSample(mem, raw); - var end = GetIterator(begin, rawLength); - - var end2 = UrlPathDecoder.Unescape(begin, end); - var result = begin.GetUtf8String(ref end2); - - Assert.Equal(expectLength, result.Length); - Assert.Equal(expect, result); - - pool.Return(mem); - } - } - - private MemoryPoolIterator BuildSample(MemoryPoolBlock mem, string data) - { - var store = data.Select(c => (byte)c).ToArray(); - mem.GetIterator().CopyFrom(new ArraySegment(store)); - - return mem.GetIterator(); - } - - private MemoryPoolIterator GetIterator(MemoryPoolIterator begin, int displacement) - { - var result = begin; - for (int i = 0; i < displacement; ++i) - { - result.Take(); - } - - return result; - } - - private void PositiveAssert(MemoryPoolBlock mem, string raw, string expect) - { - var begin = BuildSample(mem, raw); - var end = GetIterator(begin, raw.Length); - - var result = UrlPathDecoder.Unescape(begin, end); - Assert.Equal(expect, begin.GetUtf8String(ref result)); - } - - private void PositiveAssert(MemoryPoolBlock mem, string raw) - { - var begin = BuildSample(mem, raw); - var end = GetIterator(begin, raw.Length); - - var result = UrlPathDecoder.Unescape(begin, end); - Assert.NotEqual(raw.Length, begin.GetUtf8String(ref result).Length); - } - - private void NegativeAssert(MemoryPoolBlock mem, string raw) - { - var begin = BuildSample(mem, raw); - var end = GetIterator(begin, raw.Length); - - var resultEnd = UrlPathDecoder.Unescape(begin, end); - var result = begin.GetUtf8String(ref resultEnd); - Assert.Equal(raw, result); - } - } -} From e2f8c226efbb8008a342e1b3a12badeb77b7a86d Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 7 Mar 2017 21:30:57 +0000 Subject: [PATCH 25/28] Simplify TakeSingleHeader and Vectorize (#1457) * Sanitize unsafe code * Vectorize * Extract Contains and add more tests --- .../Internal/Http/KestrelHttpParser.cs | 175 ++++++++++-------- test/shared/HttpParsingData.cs | 19 ++ 2 files changed, 113 insertions(+), 81 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index f85af592fb..d8da5a89bc 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -4,6 +4,7 @@ using System; using System.Buffers; using System.IO.Pipelines; +using System.Numerics; using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.Extensions.Logging; @@ -389,128 +390,126 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private unsafe int IndexOfNameEnd(byte* pBuffer, int index, int length) + private static unsafe int FindEndOfName(byte* headerLine, int length) { - var pCurrent = pBuffer + index; - var pEnd = pBuffer + index + length; - var result = -1; + var index = 0; var sawWhitespace = false; - - while (pCurrent < pEnd) + for (; index < length; index++) { - var ch = *pCurrent; + var ch = headerLine[index]; if (ch == ByteColon) { - if (sawWhitespace) - { - RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); - } - - result = index; break; } - - if (ch == ByteTab || ch == ByteSpace) + if (ch == ByteTab || ch == ByteSpace || ch == ByteCR) { sawWhitespace = true; } - - index++; - pCurrent++; } - return result; - } - private unsafe void TakeSingleHeader(byte* pHeader, int headerLineLength, T handler) where T : IHttpHeadersHandler - { - var nameEnd = -1; - var valueStart = -1; - var valueEnd = -1; - var index = 0; - var pCurrent = pHeader + index; - var pEnd = pHeader + headerLineLength; - - nameEnd = IndexOfNameEnd(pHeader, index, headerLineLength); - - if (nameEnd == -1) + if (index == length) { RejectRequest(RequestRejectionReason.NoColonCharacterFoundInHeaderLine); } - - // Skip colon - index += nameEnd + 1; - pCurrent += index; - valueStart = index; - var pValueStart = pCurrent; - - while (pCurrent < pEnd) + if (sawWhitespace) { - var ch = *pCurrent; - if (ch != ByteTab && ch != ByteSpace && ch != ByteCR) - { - valueStart = index; - pValueStart = pCurrent; - break; - } - else if (ch == ByteCR) - { - break; - } - pCurrent++; - index++; + RejectRequest(RequestRejectionReason.WhitespaceIsNotAllowedInHeaderName); } + return index; + } - var endIndex = headerLineLength - 1; - pEnd--; + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private unsafe void TakeSingleHeader(byte* headerLine, int length, T handler) where T : IHttpHeadersHandler + { + // Skip CR, LF from end position + var valueEnd = length - 3; + var nameEnd = FindEndOfName(headerLine, length); - if (*pEnd != ByteLF) + if (headerLine[valueEnd + 2] != ByteLF) { RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); } - - endIndex--; - pEnd--; - - if (*pEnd != ByteCR) + if (headerLine[valueEnd + 1] != ByteCR) { RejectRequest(RequestRejectionReason.MissingCRInHeaderLine); } - endIndex--; - pEnd--; - - while (pEnd >= pValueStart) + // Skip colon from value start + var valueStart = nameEnd + 1; + // Ignore start whitespace + for(; valueStart < valueEnd; valueStart++) { - var ch = *pEnd; - if (ch != ByteTab && ch != ByteSpace && ch != ByteCR && valueEnd == -1) + var ch = headerLine[valueStart]; + if (ch != ByteTab && ch != ByteSpace && ch != ByteCR) { - valueEnd = endIndex; + break; } else if (ch == ByteCR) { RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); } - - pEnd--; - endIndex--; } - if (valueEnd == -1) + + // Check for CR in value + var i = valueStart + 1; + if (Contains(headerLine + i, valueEnd - i, ByteCR)) { - valueEnd = valueStart; + RejectRequest(RequestRejectionReason.HeaderValueMustNotContainCR); } - else + + // Ignore end whitespace + for (; valueEnd > valueStart; valueEnd--) { - valueEnd++; + var ch = headerLine[valueEnd]; + if (ch != ByteTab && ch != ByteSpace) + { + break; + } } - - var nameBuffer = new Span(pHeader, nameEnd); - var valueBuffer = new Span(pHeader + valueStart, valueEnd - valueStart); + var nameBuffer = new Span(headerLine, nameEnd); + var valueBuffer = new Span(headerLine + valueStart, valueEnd - valueStart + 1); handler.OnHeader(nameBuffer, valueBuffer); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static unsafe bool Contains(byte* searchSpace, int length, byte value) + { + var i = 0; + if (Vector.IsHardwareAccelerated) + { + // Check Vector lengths + if (length - Vector.Count >= i) + { + var vValue = GetVector(value); + do + { + if (!Vector.Zero.Equals(Vector.Equals(vValue, Unsafe.Read>(searchSpace + i)))) + { + goto found; + } + + i += Vector.Count; + } while (length - Vector.Count >= i); + } + } + + // Check remaining for CR + for (; i <= length; i++) + { + var ch = searchSpace[i]; + if (ch == value) + { + goto found; + } + } + return false; + found: + return true; + } + private static bool IsValidTokenChar(char c) { // Determines if a character is valid as a 'token' as defined in the @@ -536,20 +535,25 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http c == '~'; } - public void RejectRequest(RequestRejectionReason reason) + public static void RejectRequest(RequestRejectionReason reason) { throw BadHttpRequestException.GetException(reason); } - public void RejectRequest(RequestRejectionReason reason, string value) + public static void RejectRequest(RequestRejectionReason reason, string value) { throw BadHttpRequestException.GetException(reason, value); } private void RejectRequestLine(Span span) + { + throw GetRejectRequestLineException(span); + } + + private BadHttpRequestException GetRejectRequestLineException(Span span) { const int MaxRequestLineError = 32; - RejectRequest(RequestRejectionReason.InvalidRequestLine, + return BadHttpRequestException.GetException(RequestRejectionReason.InvalidRequestLine, Log.IsEnabled(LogLevel.Information) ? span.GetAsciiStringEscaped(MaxRequestLineError) : string.Empty); } @@ -558,6 +562,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static Vector GetVector(byte vectorByte) + { + // Vector .ctor doesn't become an intrinsic due to detection issue + // However this does cause it to become an intrinsic (with additional multiply and reg->reg copy) + // https://github.com/dotnet/coreclr/issues/7459#issuecomment-253965670 + return Vector.AsVectorByte(new Vector(vectorByte * 0x01010101u)); + } + private enum HeaderState { Name, diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 56688b67f0..72a579feb0 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -212,6 +212,14 @@ namespace Microsoft.AspNetCore.Testing "Header-1: value1\rHeader-2: value2\r\n\r\n", "Header-1: value1\r\nHeader-2: value2\r\r\n", "Header-1: value1\r\nHeader-2: v\ralue2\r\n", + "Header-1: Value__\rVector16________Vector32\r\n", + "Header-1: Value___Vector16\r________Vector32\r\n", + "Header-1: Value___Vector16_______\rVector32\r\n", + "Header-1: Value___Vector16________Vector32\r\r\n", + "Header-1: Value___Vector16________Vector32_\r\r\n", + "Header-1: Value___Vector16________Vector32Value___Vector16_______\rVector32\r\n", + "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32\r\r\n", + "Header-1: Value___Vector16________Vector32Value___Vector16________Vector32_\r\r\n", }; // Missing colon @@ -236,9 +244,20 @@ namespace Microsoft.AspNetCore.Testing { "Header : value\r\n\r\n", "Header\t: value\r\n\r\n", + "Header\r: value\r\n\r\n", + "Header_\rVector16: value\r\n\r\n", + "Header__Vector16\r: value\r\n\r\n", + "Header__Vector16_\r: value\r\n\r\n", + "Header_\rVector16________Vector32: value\r\n\r\n", + "Header__Vector16________Vector32\r: value\r\n\r\n", + "Header__Vector16________Vector32_\r: value\r\n\r\n", + "Header__Vector16________Vector32Header_\rVector16________Vector32: value\r\n\r\n", + "Header__Vector16________Vector32Header__Vector16________Vector32\r: value\r\n\r\n", + "Header__Vector16________Vector32Header__Vector16________Vector32_\r: value\r\n\r\n", "Header 1: value1\r\nHeader-2: value2\r\n\r\n", "Header 1 : value1\r\nHeader-2: value2\r\n\r\n", "Header 1\t: value1\r\nHeader-2: value2\r\n\r\n", + "Header 1\r: value1\r\nHeader-2: value2\r\n\r\n", "Header-1: value1\r\nHeader 2: value2\r\n\r\n", "Header-1: value1\r\nHeader-2 : value2\r\n\r\n", "Header-1: value1\r\nHeader-2\t: value2\r\n\r\n", From 5743d740b454874a3d056d2f480297be51abcb71 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Mon, 6 Mar 2017 17:15:59 -0800 Subject: [PATCH 26/28] Convert HTTP parsing FrameTests to IHttpParser tests (#1416). - Also fix an issue in KestrelHttpParser where "Header: \r\n" is parsed with a value of " " instead of "". --- .../Internal/Http/KestrelHttpParser.cs | 2 +- .../BadHttpRequestTests.cs | 42 +- .../FrameTests.cs | 398 ++++-------------- .../HttpParserTests.cs | 383 +++++++++++++++++ test/shared/HttpParsingData.cs | 61 ++- 5 files changed, 531 insertions(+), 355 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs index d8da5a89bc..a5ce732aec 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/KestrelHttpParser.cs @@ -459,7 +459,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } // Ignore end whitespace - for (; valueEnd > valueStart; valueEnd--) + for (; valueEnd >= valueStart; valueEnd--) { var ch = headerLine[valueEnd]; if (ch != ByteTab && ch != ByteSpace) diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs index 80f127c9d1..565b06c2f1 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/BadHttpRequestTests.cs @@ -1,9 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; @@ -127,25 +125,33 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests ""); } - public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData - .Select(requestLine => new object[] + public static TheoryData InvalidRequestLineData + { + get { - requestLine, - $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", - }) - .Concat(HttpParsingData.EncodedNullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - "Invalid request line." - })) - .Concat(HttpParsingData.NullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - "Invalid request line." - })); + var data = new TheoryData(); + + foreach (var requestLine in HttpParsingData.RequestLineInvalidData) + { + data.Add(requestLine, $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}"); + } + + foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) + { + data.Add(requestLine, "Invalid request line."); + } + + foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData) + { + data.Add(requestLine, "Invalid request line."); + } + + return data; + } + } public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; - public static IEnumerable InvalidRequestHeaderData => HttpParsingData.InvalidRequestHeaderData; + public static IEnumerable InvalidRequestHeaderData => HttpParsingData.RequestHeaderInvalidData; } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs index 46d54d1b40..8aeb066862 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/FrameTests.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.IO; using System.IO.Pipelines; -using System.Linq; using System.Net; using System.Text; using System.Threading; @@ -25,7 +24,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { public class FrameTests : IDisposable { - private readonly IPipe _socketInput; + private readonly IPipe _input; private readonly TestFrame _frame; private readonly ServiceContext _serviceContext; private readonly ConnectionContext _connectionContext; @@ -50,7 +49,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { var trace = new KestrelTrace(new TestKestrelTrace()); _pipelineFactory = new PipeFactory(); - _socketInput = _pipelineFactory.Create(); + _input = _pipelineFactory.Create(); _serviceContext = new ServiceContext { @@ -65,7 +64,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests }; _connectionContext = new ConnectionContext(listenerContext) { - Input = _socketInput, + Input = _input, Output = new MockSocketOutput(), ConnectionControl = Mock.Of() }; @@ -77,149 +76,30 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public void Dispose() { - _socketInput.Reader.Complete(); - _socketInput.Writer.Complete(); + _input.Reader.Complete(); + _input.Writer.Complete(); _pipelineFactory.Dispose(); } - [Fact] - public async Task CanReadHeaderValueWithoutLeadingWhitespace() - { - _frame.InitializeHeaders(); - - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header:value\r\n\r\n")); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.True(success); - Assert.Equal(1, _frame.RequestHeaders.Count); - Assert.Equal("value", _frame.RequestHeaders["Header"]); - Assert.Equal(readableBuffer.End, _consumed); - } - - [Theory] - [InlineData("Header: value\r\n\r\n")] - [InlineData("Header: value\r\n\r\n")] - [InlineData("Header:\tvalue\r\n\r\n")] - [InlineData("Header: \tvalue\r\n\r\n")] - [InlineData("Header:\t value\r\n\r\n")] - [InlineData("Header:\t\tvalue\r\n\r\n")] - [InlineData("Header:\t\t value\r\n\r\n")] - [InlineData("Header: \t\tvalue\r\n\r\n")] - [InlineData("Header: \t\t value\r\n\r\n")] - [InlineData("Header: \t \t value\r\n\r\n")] - public async Task LeadingWhitespaceIsNotIncludedInHeaderValue(string rawHeaders) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.True(success); - Assert.Equal(1, _frame.RequestHeaders.Count); - Assert.Equal("value", _frame.RequestHeaders["Header"]); - Assert.Equal(readableBuffer.End, _consumed); - } - - [Theory] - [InlineData("Header: value \r\n\r\n")] - [InlineData("Header: value\t\r\n\r\n")] - [InlineData("Header: value \t\r\n\r\n")] - [InlineData("Header: value\t \r\n\r\n")] - [InlineData("Header: value\t\t\r\n\r\n")] - [InlineData("Header: value\t\t \r\n\r\n")] - [InlineData("Header: value \t\t\r\n\r\n")] - [InlineData("Header: value \t\t \r\n\r\n")] - [InlineData("Header: value \t \t \r\n\r\n")] - public async Task TrailingWhitespaceIsNotIncludedInHeaderValue(string rawHeaders) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.True(success); - Assert.Equal(1, _frame.RequestHeaders.Count); - Assert.Equal("value", _frame.RequestHeaders["Header"]); - Assert.Equal(readableBuffer.End, _consumed); - } - - [Theory] - [InlineData("Header: one two three\r\n\r\n", "one two three")] - [InlineData("Header: one two three\r\n\r\n", "one two three")] - [InlineData("Header: one\ttwo\tthree\r\n\r\n", "one\ttwo\tthree")] - [InlineData("Header: one two\tthree\r\n\r\n", "one two\tthree")] - [InlineData("Header: one\ttwo three\r\n\r\n", "one\ttwo three")] - [InlineData("Header: one \ttwo \tthree\r\n\r\n", "one \ttwo \tthree")] - [InlineData("Header: one\t two\t three\r\n\r\n", "one\t two\t three")] - [InlineData("Header: one \ttwo\t three\r\n\r\n", "one \ttwo\t three")] - public async Task WhitespaceWithinHeaderValueIsPreserved(string rawHeaders, string expectedValue) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.True(success); - Assert.Equal(1, _frame.RequestHeaders.Count); - Assert.Equal(expectedValue, _frame.RequestHeaders["Header"]); - Assert.Equal(readableBuffer.End, _consumed); - } - - [Theory] - [MemberData(nameof(InvalidRequestHeaderData))] - public async Task TakeMessageHeadersThrowsOnInvalidRequestHeaders(string rawHeaders, string expectedExceptionMessage) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - - var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.Equal(expectedExceptionMessage, exception.Message); - Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); - } - [Fact] public async Task TakeMessageHeadersThrowsOnHeaderValueWithLineFolding_CharacterNotAvailableOnFirstAttempt() { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n")); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n")); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(" ")); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(" ")); - readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.Equal("Whitespace is not allowed in header name.", exception.Message); Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); } - [Fact] - public async Task TakeMessageHeadersConsumesBytesCorrectlyAtEnd() - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("Header-1: value1\r\n\r")); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - Assert.False(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); - - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("\n")); - - readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - Assert.True(_frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); - } - [Fact] public async Task TakeMessageHeadersThrowsWhenHeadersExceedTotalSizeLimit() { @@ -227,11 +107,11 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _serviceContext.ServerOptions.Limits.MaxRequestHeadersTotalSize = headerLine.Length - 1; _frame.Reset(); - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine}\r\n")); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine}\r\n")); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.Equal("Request headers too long.", exception.Message); Assert.Equal(StatusCodes.Status431RequestHeaderFieldsTooLarge, exception.StatusCode); @@ -243,36 +123,16 @@ namespace Microsoft.AspNetCore.Server.KestrelTests const string headerLines = "Header-1: value1\r\nHeader-2: value2\r\n"; _serviceContext.ServerOptions.Limits.MaxRequestHeaderCount = 1; - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLines}\r\n")); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLines}\r\n")); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var exception = Assert.Throws(() => _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.Equal("Request contains too many headers.", exception.Message); Assert.Equal(StatusCodes.Status431RequestHeaderFieldsTooLarge, exception.StatusCode); } - [Theory] - [InlineData("Cookie: \r\n\r\n", 1)] - [InlineData("Cookie:\r\n\r\n", 1)] - [InlineData("Cookie: \r\nConnection: close\r\n\r\n", 2)] - [InlineData("Cookie:\r\nConnection: close\r\n\r\n", 2)] - [InlineData("Connection: close\r\nCookie: \r\n\r\n", 2)] - [InlineData("Connection: close\r\nCookie:\r\n\r\n", 2)] - public async Task EmptyHeaderValuesCanBeParsed(string rawHeaders, int numHeaders) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeaders)); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - - var success = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.True(success); - Assert.Equal(numHeaders, _frame.RequestHeaders.Count); - Assert.Equal(readableBuffer.End, _consumed); - } - [Fact] public void ResetResetsScheme() { @@ -296,11 +156,11 @@ namespace Microsoft.AspNetCore.Server.KestrelTests options.Limits.MaxRequestHeaderCount = 1; _serviceContext.ServerOptions = options; - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine1}\r\n")); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine1}\r\n")); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.True(takeMessageHeaders); Assert.Equal(1, _frame.RequestHeaders.Count); @@ -308,11 +168,11 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _frame.Reset(); - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine2}\r\n")); - readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes($"{headerLine2}\r\n")); + readableBuffer = (await _input.Reader.ReadAsync()).Buffer; takeMessageHeaders = _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.True(takeMessageHeaders); Assert.Equal(1, _frame.RequestHeaders.Count); @@ -407,86 +267,37 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public async Task TakeStartLineSetsFrameProperties( string requestLine, string expectedMethod, - string expectedPath, + string expectedRawTarget, + string expectedRawPath, + string expectedDecodedPath, string expectedQueryString, string expectedHttpVersion) { var requestLineBytes = Encoding.ASCII.GetBytes(requestLine); - await _socketInput.Writer.WriteAsync(requestLineBytes); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(requestLineBytes); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var returnValue = _frame.TakeStartLine(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.True(returnValue); Assert.Equal(expectedMethod, _frame.Method); - Assert.Equal(expectedPath, _frame.Path); + Assert.Equal(expectedRawTarget, _frame.RawTarget); + Assert.Equal(expectedDecodedPath, _frame.Path); Assert.Equal(expectedQueryString, _frame.QueryString); Assert.Equal(expectedHttpVersion, _frame.HttpVersion); } - [Fact] - public async Task TakeStartLineCallsConsumingCompleteWithFurthestExamined() - { - var requestLineBytes = Encoding.ASCII.GetBytes("GET / "); - await _socketInput.Writer.WriteAsync(requestLineBytes); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - - _frame.TakeStartLine(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.Equal(readableBuffer.Start, _consumed); - Assert.Equal(readableBuffer.End, _examined); - - requestLineBytes = Encoding.ASCII.GetBytes("HTTP/1.1\r\n"); - await _socketInput.Writer.WriteAsync(requestLineBytes); - readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - - _frame.TakeStartLine(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.Equal(readableBuffer.End, _consumed); - Assert.Equal(readableBuffer.End, _examined); - } - - [Theory] - [InlineData("G")] - [InlineData("GE")] - [InlineData("GET")] - [InlineData("GET ")] - [InlineData("GET /")] - [InlineData("GET / ")] - [InlineData("GET / H")] - [InlineData("GET / HT")] - [InlineData("GET / HTT")] - [InlineData("GET / HTTP")] - [InlineData("GET / HTTP/")] - [InlineData("GET / HTTP/1")] - [InlineData("GET / HTTP/1.")] - [InlineData("GET / HTTP/1.1")] - [InlineData("GET / HTTP/1.1\r")] - public async Task TakeStartLineReturnsWhenGivenIncompleteRequestLines(string requestLine) - { - var requestLineBytes = Encoding.ASCII.GetBytes(requestLine); - await _socketInput.Writer.WriteAsync(requestLineBytes); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - var returnValue = _frame.TakeStartLine(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.False(returnValue); - } - [Fact] public async Task ParseRequestStartsRequestHeadersTimeoutOnFirstByteAvailable() { var connectionControl = new Mock(); _connectionContext.ConnectionControl = connectionControl.Object; - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes("G")); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes("G")); - _frame.ParseRequest((await _socketInput.Reader.ReadAsync()).Buffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); + _frame.ParseRequest((await _input.Reader.ReadAsync()).Buffer, out _consumed, out _examined); + _input.Reader.Advance(_consumed, _examined); var expectedRequestHeadersTimeout = (long)_serviceContext.ServerOptions.Limits.RequestHeadersTimeout.TotalMilliseconds; connectionControl.Verify(cc => cc.ResetTimeout(expectedRequestHeadersTimeout, TimeoutAction.SendTimeoutResponse)); @@ -498,93 +309,42 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _serviceContext.ServerOptions.Limits.MaxRequestLineSize = "GET / HTTP/1.1\r\n".Length; var requestLineBytes = Encoding.ASCII.GetBytes("GET /a HTTP/1.1\r\n"); - await _socketInput.Writer.WriteAsync(requestLineBytes); + await _input.Writer.WriteAsync(requestLineBytes); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); Assert.Equal("Request line too long.", exception.Message); Assert.Equal(StatusCodes.Status414UriTooLong, exception.StatusCode); } [Theory] - [MemberData(nameof(InvalidRequestLineData))] - public async Task TakeStartLineThrowsOnInvalidRequestLine(string requestLine, Type expectedExceptionType, string expectedExceptionMessage) + [MemberData(nameof(RequestLineWithEncodedNullCharInTargetData))] + public async Task TakeStartLineThrowsOnEncodedNullCharInTarget(string requestLine) { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - var exception = Assert.Throws(expectedExceptionType, () => + var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); + _input.Reader.Advance(_consumed, _examined); - Assert.Equal(expectedExceptionMessage, exception.Message); - - if (expectedExceptionType == typeof(BadHttpRequestException)) - { - Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); - } + Assert.Equal("The path contains null characters.", exception.Message); } [Theory] - [MemberData(nameof(UnrecognizedHttpVersionData))] - public async Task TakeStartLineThrowsOnUnrecognizedHttpVersion(string httpVersion) + [MemberData(nameof(RequestLineWithNullCharInTargetData))] + public async Task TakeStartLineThrowsOnNullCharInTarget(string requestLine) { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes($"GET / {httpVersion}\r\n")); + await _input.Writer.WriteAsync(Encoding.ASCII.GetBytes(requestLine)); + var readableBuffer = (await _input.Reader.ReadAsync()).Buffer; - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; + var exception = Assert.Throws(() => + _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); + _input.Reader.Advance(_consumed, _examined); - var exception = Assert.Throws(() => _frame.TakeStartLine(readableBuffer, out _consumed, out _examined)); - _socketInput.Reader.Advance(_consumed, _examined); - - Assert.Equal($"Unrecognized HTTP version: {httpVersion}", exception.Message); - Assert.Equal(StatusCodes.Status505HttpVersionNotsupported, exception.StatusCode); - } - - [Fact] - public async Task TakeMessageHeadersCallsConsumingCompleteWithFurthestExamined() - { - foreach (var rawHeader in new[] { "Header: ", "value\r\n", "\r\n" }) - { - await _socketInput.Writer.WriteAsync(Encoding.ASCII.GetBytes(rawHeader)); - - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - _frame.TakeMessageHeaders(readableBuffer, out _consumed, out _examined); - _socketInput.Reader.Advance(_consumed, _examined); - Assert.Equal(readableBuffer.End, _examined); - } - } - - [Theory] - [InlineData("\r")] - [InlineData("H")] - [InlineData("He")] - [InlineData("Hea")] - [InlineData("Head")] - [InlineData("Heade")] - [InlineData("Header")] - [InlineData("Header:")] - [InlineData("Header: ")] - [InlineData("Header: v")] - [InlineData("Header: va")] - [InlineData("Header: val")] - [InlineData("Header: valu")] - [InlineData("Header: value")] - [InlineData("Header: value\r")] - [InlineData("Header: value\r\n")] - [InlineData("Header: value\r\n\r")] - public async Task TakeMessageHeadersReturnsWhenGivenIncompleteHeaders(string headers) - { - var headerBytes = Encoding.ASCII.GetBytes(headers); - await _socketInput.Writer.WriteAsync(headerBytes); - - ReadCursor consumed; - ReadCursor examined; - var readableBuffer = (await _socketInput.Reader.ReadAsync()).Buffer; - - Assert.Equal(false, _frame.TakeMessageHeaders(readableBuffer, out consumed, out examined)); - _socketInput.Reader.Advance(consumed, examined); + Assert.Equal(new InvalidOperationException().Message, exception.Message); } [Fact] @@ -599,7 +359,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests connectionControl.Verify(cc => cc.SetTimeout(expectedKeepAliveTimeout, TimeoutAction.CloseConnection)); _frame.StopAsync(); - _socketInput.Writer.Complete(); + _input.Writer.Complete(); requestProcessingTask.Wait(); } @@ -681,13 +441,13 @@ namespace Microsoft.AspNetCore.Server.KestrelTests _frame.Start(); var data = Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n\r\n"); - await _socketInput.Writer.WriteAsync(data); + await _input.Writer.WriteAsync(data); var requestProcessingTask = _frame.StopAsync(); Assert.IsNotType(typeof(Task), requestProcessingTask); await requestProcessingTask.TimeoutAfter(TimeSpan.FromSeconds(10)); - _socketInput.Writer.Complete(); + _input.Writer.Complete(); } [Fact] @@ -758,30 +518,36 @@ namespace Microsoft.AspNetCore.Server.KestrelTests Assert.NotSame(original, _frame.RequestAborted.WaitHandle); } - public static IEnumerable ValidRequestLineData => HttpParsingData.ValidRequestLineData; + public static IEnumerable ValidRequestLineData => HttpParsingData.RequestLineValidData; - public static IEnumerable InvalidRequestLineData => HttpParsingData.InvalidRequestLineData - .Select(requestLine => new object[] + public static TheoryData RequestLineWithEncodedNullCharInTargetData + { + get { - requestLine, - typeof(BadHttpRequestException), - $"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", - }) - .Concat(HttpParsingData.EncodedNullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - typeof(InvalidOperationException), - "The path contains null characters." - })) - .Concat(HttpParsingData.NullCharInTargetRequestLines.Select(requestLine => new object[] - { - requestLine, - typeof(InvalidOperationException), - new InvalidOperationException().Message - })); + var data = new TheoryData(); - public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; + foreach (var requestLine in HttpParsingData.RequestLineWithEncodedNullCharInTargetData) + { + data.Add(requestLine); + } - public static IEnumerable InvalidRequestHeaderData => HttpParsingData.InvalidRequestHeaderData; + return data; + } + } + + public static TheoryData RequestLineWithNullCharInTargetData + { + get + { + var data = new TheoryData(); + + foreach (var requestLine in HttpParsingData.RequestLineWithNullCharInTargetData) + { + data.Add(requestLine); + } + + return data; + } + } } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs new file mode 100644 index 0000000000..7d16b5f06c --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/HttpParserTests.cs @@ -0,0 +1,383 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO.Pipelines; +using System.Linq; +using System.Text; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.KestrelTests +{ + public class HttpParserTests + { + // Returns true when all headers parsed + // Return false otherwise + + [Theory] + [MemberData(nameof(RequestLineValidData))] + public void ParsesRequestLine( + string requestLine, + string expectedMethod, + string expectedRawTarget, + string expectedRawPath, + string expectedDecodedPath, + string expectedQueryString, + string expectedVersion) + { + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(requestLine)); + + string parsedMethod = null; + string parsedVersion = null; + string parsedRawTarget = null; + string parsedRawPath = null; + string parsedQuery = null; + var requestLineHandler = new Mock(); + requestLineHandler + .Setup(handler => handler.OnStartLine( + It.IsAny(), + It.IsAny(), + It.IsAny>(), + It.IsAny>(), + It.IsAny>(), + It.IsAny>())) + .Callback, Span, Span, Span>((method, version, target, path, query, customMethod) => + { + parsedMethod = method != HttpMethod.Custom ? HttpUtilities.MethodToString(method) : customMethod.GetAsciiStringNonNullCharacters(); + parsedVersion = HttpUtilities.VersionToString(version); + parsedRawTarget = target.GetAsciiStringNonNullCharacters(); + parsedRawPath = path.GetAsciiStringNonNullCharacters(); + parsedQuery = query.GetAsciiStringNonNullCharacters(); + }); + + Assert.True(parser.ParseRequestLine(requestLineHandler.Object, buffer, out var consumed, out var examined)); + + Assert.Equal(parsedMethod, expectedMethod); + Assert.Equal(parsedVersion, expectedVersion); + Assert.Equal(parsedRawTarget, expectedRawTarget); + Assert.Equal(parsedRawPath, expectedRawPath); + Assert.Equal(parsedVersion, expectedVersion); + Assert.Equal(buffer.End, consumed); + Assert.Equal(buffer.End, examined); + } + + [Theory] + [MemberData(nameof(RequestLineIncompleteData))] + public void ParseRequestLineReturnsFalseWhenGivenIncompleteRequestLines(string requestLine) + { + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(requestLine)); + + Assert.False(parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + } + + [Theory] + [MemberData(nameof(RequestLineIncompleteData))] + public void ParseRequestLineDoesNotConsumeIncompleteRequestLine(string requestLine) + { + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(requestLine)); + + Assert.False(parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal(buffer.Start, consumed); + Assert.Equal(buffer.End, examined); + } + + [Theory] + [MemberData(nameof(RequestLineInvalidData))] + public void ParseRequestLineThrowsOnInvalidRequestLine(string requestLine) + { + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(requestLine)); + + var exception = Assert.Throws(() => + parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal($"Invalid request line: {requestLine.Replace("\r", "<0x0D>").Replace("\n", "<0x0A>")}", exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, (exception as BadHttpRequestException).StatusCode); + } + + [Theory] + [MemberData(nameof(UnrecognizedHttpVersionData))] + public void ParseRequestLineThrowsOnUnrecognizedHttpVersion(string httpVersion) + { + var requestLine = $"GET / {httpVersion}\r\n"; + + var mockTrace = new Mock(); + mockTrace + .Setup(trace => trace.IsEnabled(LogLevel.Information)) + .Returns(true); + + var parser = CreateParser(mockTrace.Object); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(requestLine)); + + var exception = Assert.Throws(() => + parser.ParseRequestLine(Mock.Of(), buffer, out var consumed, out var examined)); + + Assert.Equal($"Unrecognized HTTP version: {httpVersion}", exception.Message); + Assert.Equal(StatusCodes.Status505HttpVersionNotsupported, (exception as BadHttpRequestException).StatusCode); + } + + [Theory] + [InlineData("\r")] + [InlineData("H")] + [InlineData("He")] + [InlineData("Hea")] + [InlineData("Head")] + [InlineData("Heade")] + [InlineData("Header")] + [InlineData("Header:")] + [InlineData("Header: ")] + [InlineData("Header: v")] + [InlineData("Header: va")] + [InlineData("Header: val")] + [InlineData("Header: valu")] + [InlineData("Header: value")] + [InlineData("Header: value\r")] + [InlineData("Header: value\r\n")] + [InlineData("Header: value\r\n\r")] + [InlineData("Header-1: value1\r\nH")] + [InlineData("Header-1: value1\r\nHe")] + [InlineData("Header-1: value1\r\nHea")] + [InlineData("Header-1: value1\r\nHead")] + [InlineData("Header-1: value1\r\nHeade")] + [InlineData("Header-1: value1\r\nHeader")] + [InlineData("Header-1: value1\r\nHeader-")] + [InlineData("Header-1: value1\r\nHeader-2")] + [InlineData("Header-1: value1\r\nHeader-2:")] + [InlineData("Header-1: value1\r\nHeader-2: ")] + [InlineData("Header-1: value1\r\nHeader-2: v")] + [InlineData("Header-1: value1\r\nHeader-2: va")] + [InlineData("Header-1: value1\r\nHeader-2: val")] + [InlineData("Header-1: value1\r\nHeader-2: valu")] + [InlineData("Header-1: value1\r\nHeader-2: value")] + [InlineData("Header-1: value1\r\nHeader-2: value2")] + [InlineData("Header-1: value1\r\nHeader-2: value2\r")] + [InlineData("Header-1: value1\r\nHeader-2: value2\r\n")] + [InlineData("Header-1: value1\r\nHeader-2: value2\r\n\r")] + public void ParseHeadersReturnsFalseWhenGivenIncompleteHeaders(string rawHeaders) + { + var parser = CreateParser(Mock.Of()); + + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(rawHeaders)); + Assert.False(parser.ParseHeaders(Mock.Of(), buffer, out var consumed, out var examined, out var consumedBytes)); + } + + [Theory] + [InlineData("\r")] + [InlineData("H")] + [InlineData("He")] + [InlineData("Hea")] + [InlineData("Head")] + [InlineData("Heade")] + [InlineData("Header")] + [InlineData("Header:")] + [InlineData("Header: ")] + [InlineData("Header: v")] + [InlineData("Header: va")] + [InlineData("Header: val")] + [InlineData("Header: valu")] + [InlineData("Header: value")] + [InlineData("Header: value\r")] + public void ParseHeadersDoesNotConsumeIncompleteHeader(string rawHeaders) + { + var parser = CreateParser(Mock.Of()); + + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(rawHeaders)); + parser.ParseHeaders(Mock.Of(), buffer, out var consumed, out var examined, out var consumedBytes); + + Assert.Equal(buffer.Start, consumed); + Assert.Equal(buffer.End, examined); + Assert.Equal(0, consumedBytes); + } + + [Fact] + public void ParseHeadersCanReadHeaderValueWithoutLeadingWhitespace() + { + VerifyHeader("Header", "value", "value"); + } + + [Theory] + [InlineData("Cookie: \r\n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie:\r\n\r\n", "Cookie", "", null, null)] + [InlineData("Cookie: \r\nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Cookie:\r\nConnection: close\r\n\r\n", "Cookie", "", "Connection", "close")] + [InlineData("Connection: close\r\nCookie: \r\n\r\n", "Connection", "close", "Cookie", "")] + [InlineData("Connection: close\r\nCookie:\r\n\r\n", "Connection", "close", "Cookie", "")] + public void ParseHeadersCanParseEmptyHeaderValues( + string rawHeaders, + string expectedHeaderName1, + string expectedHeaderValue1, + string expectedHeaderName2, + string expectedHeaderValue2) + { + var expectedHeaderNames = expectedHeaderName2 == null + ? new[] { expectedHeaderName1 } + : new[] { expectedHeaderName1, expectedHeaderName2 }; + var expectedHeaderValues = expectedHeaderValue2 == null + ? new[] { expectedHeaderValue1 } + : new[] { expectedHeaderValue1, expectedHeaderValue2 }; + + VerifyRawHeaders(rawHeaders, expectedHeaderNames, expectedHeaderValues); + } + + [Theory] + [InlineData(" value")] + [InlineData(" value")] + [InlineData("\tvalue")] + [InlineData(" \tvalue")] + [InlineData("\t value")] + [InlineData("\t\tvalue")] + [InlineData("\t\t value")] + [InlineData(" \t\tvalue")] + [InlineData(" \t\t value")] + [InlineData(" \t \t value")] + public void ParseHeadersDoesNotIncludeLeadingWhitespaceInHeaderValue(string rawHeaderValue) + { + VerifyHeader("Header", rawHeaderValue, "value"); + } + + [Theory] + [InlineData("value ")] + [InlineData("value\t")] + [InlineData("value \t")] + [InlineData("value\t ")] + [InlineData("value\t\t")] + [InlineData("value\t\t ")] + [InlineData("value \t\t")] + [InlineData("value \t\t ")] + [InlineData("value \t \t ")] + public void ParseHeadersDoesNotIncludeTrailingWhitespaceInHeaderValue(string rawHeaderValue) + { + VerifyHeader("Header", rawHeaderValue, "value"); + } + + [Theory] + [InlineData("one two three")] + [InlineData("one two three")] + [InlineData("one\ttwo\tthree")] + [InlineData("one two\tthree")] + [InlineData("one\ttwo three")] + [InlineData("one \ttwo \tthree")] + [InlineData("one\t two\t three")] + [InlineData("one \ttwo\t three")] + public void ParseHeadersPreservesWhitespaceWithinHeaderValue(string headerValue) + { + VerifyHeader("Header", headerValue, headerValue); + } + + [Fact] + public void ParseHeadersConsumesBytesCorrectlyAtEnd() + { + var parser = CreateParser(Mock.Of()); + + const string headerLine = "Header: value\r\n\r"; + var buffer1 = ReadableBuffer.Create(Encoding.ASCII.GetBytes(headerLine)); + Assert.False(parser.ParseHeaders(Mock.Of(), buffer1, out var consumed, out var examined, out var consumedBytes)); + + Assert.Equal(buffer1.Move(buffer1.Start, headerLine.Length - 1), consumed); + Assert.Equal(buffer1.End, examined); + Assert.Equal(headerLine.Length - 1, consumedBytes); + + var buffer2 = ReadableBuffer.Create(Encoding.ASCII.GetBytes("\r\n")); + Assert.True(parser.ParseHeaders(Mock.Of(), buffer2, out consumed, out examined, out consumedBytes)); + + Assert.Equal(buffer2.End, consumed); + Assert.Equal(buffer2.End, examined); + Assert.Equal(2, consumedBytes); + } + + [Theory] + [MemberData(nameof(RequestHeaderInvalidData))] + public void ParseHeadersThrowsOnInvalidRequestHeaders(string rawHeaders, string expectedExceptionMessage) + { + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(rawHeaders)); + + var exception = Assert.Throws(() => + parser.ParseHeaders(Mock.Of(), buffer, out var consumed, out var examined, out var consumedBytes)); + + Assert.Equal(expectedExceptionMessage, exception.Message); + Assert.Equal(StatusCodes.Status400BadRequest, exception.StatusCode); + } + + private void VerifyHeader( + string headerName, + string rawHeaderValue, + string expectedHeaderValue) + { + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes($"{headerName}:{rawHeaderValue}\r\n")); + + string parsedHeaderName = "unexpected"; + string parsedHeaderValue = "unexpected"; + var headersHandler = new Mock(); + headersHandler + .Setup(handler => handler.OnHeader(It.IsAny>(), It.IsAny>())) + .Callback, Span>((name, value) => + { + parsedHeaderName = name.GetAsciiStringNonNullCharacters(); + parsedHeaderValue = value.GetAsciiStringNonNullCharacters(); + }); + + parser.ParseHeaders(headersHandler.Object, buffer, out var consumed, out var examined, out var consumedBytes); + + Assert.Equal(headerName, parsedHeaderName); + Assert.Equal(expectedHeaderValue, parsedHeaderValue); + Assert.Equal(buffer.End, consumed); + Assert.Equal(buffer.End, examined); + } + + private void VerifyRawHeaders(string rawHeaders, IEnumerable expectedHeaderNames, IEnumerable expectedHeaderValues) + { + Assert.True(expectedHeaderNames.Count() == expectedHeaderValues.Count(), $"{nameof(expectedHeaderNames)} and {nameof(expectedHeaderValues)} sizes must match"); + + var parser = CreateParser(Mock.Of()); + var buffer = ReadableBuffer.Create(Encoding.ASCII.GetBytes(rawHeaders)); + + var parsedHeaders = new List>(); + var headersHandler = new Mock(); + headersHandler + .Setup(handler => handler.OnHeader(It.IsAny>(), It.IsAny>())) + .Callback, Span>((name, value) => + { + parsedHeaders.Add(Tuple.Create(name.GetAsciiStringNonNullCharacters(), value.GetAsciiStringNonNullCharacters())); + }); + + parser.ParseHeaders(headersHandler.Object, buffer, out var consumed, out var examined, out var consumedBytes); + + Assert.Equal(expectedHeaderNames.Count(), parsedHeaders.Count); + Assert.Equal(expectedHeaderNames, parsedHeaders.Select(t => t.Item1)); + Assert.Equal(expectedHeaderValues, parsedHeaders.Select(t => t.Item2)); + Assert.Equal(buffer.End, consumed); + Assert.Equal(buffer.End, examined); + } + + private IHttpParser CreateParser(IKestrelTrace log) => new KestrelHttpParser(log); + + public static IEnumerable RequestLineValidData => HttpParsingData.RequestLineValidData; + + public static IEnumerable RequestLineIncompleteData => HttpParsingData.RequestLineIncompleteData.Select(requestLine => new[] { requestLine }); + + public static IEnumerable RequestLineInvalidData => HttpParsingData.RequestLineInvalidData.Select(requestLine => new[] { requestLine }); + + public static TheoryData UnrecognizedHttpVersionData => HttpParsingData.UnrecognizedHttpVersionData; + + public static IEnumerable RequestHeaderInvalidData => HttpParsingData.RequestHeaderInvalidData; + } +} diff --git a/test/shared/HttpParsingData.cs b/test/shared/HttpParsingData.cs index 72a579feb0..7d84ba37b3 100644 --- a/test/shared/HttpParsingData.cs +++ b/test/shared/HttpParsingData.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Testing { public class HttpParsingData { - public static IEnumerable ValidRequestLineData + public static IEnumerable RequestLineValidData { get { @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Testing "GET", "CUSTOM", }; - var targets = new[] + var paths = new[] { Tuple.Create("/", "/"), Tuple.Create("/abc", "/abc"), @@ -56,21 +56,42 @@ namespace Microsoft.AspNetCore.Testing }; return from method in methods - from target in targets + from path in paths from queryString in queryStrings from httpVersion in httpVersions select new[] { - $"{method} {target.Item1}{queryString} {httpVersion}\r\n", + $"{method} {path.Item1}{queryString} {httpVersion}\r\n", method, - $"{target.Item2}", + $"{path.Item1}{queryString}", + $"{path.Item1}", + $"{path.Item2}", queryString, httpVersion }; } } - public static IEnumerable InvalidRequestLineData => new[] + public static IEnumerable RequestLineIncompleteData => new[] + { + "G", + "GE", + "GET", + "GET ", + "GET /", + "GET / ", + "GET / H", + "GET / HT", + "GET / HTT", + "GET / HTTP", + "GET / HTTP/", + "GET / HTTP/1", + "GET / HTTP/1.", + "GET / HTTP/1.1", + "GET / HTTP/1.1\r", + }; + + public static IEnumerable RequestLineInvalidData => new[] { "G\r\n", "GE\r\n", @@ -140,7 +161,7 @@ namespace Microsoft.AspNetCore.Testing "post= / HTTP/1.0\r\n", }; - public static IEnumerable EncodedNullCharInTargetRequestLines => new[] + public static IEnumerable RequestLineWithEncodedNullCharInTargetData => new[] { "GET /%00 HTTP/1.1\r\n", "GET /%00%00 HTTP/1.1\r\n", @@ -149,17 +170,16 @@ namespace Microsoft.AspNetCore.Testing "GET /%F3%00%82%86 HTTP/1.1\r\n", "GET /%F3%85%00%82 HTTP/1.1\r\n", "GET /%F3%85%82%00 HTTP/1.1\r\n", - "GET /%E8%85%00 HTTP/1.1\r\n", "GET /%E8%01%00 HTTP/1.1\r\n", }; - public static IEnumerable NullCharInTargetRequestLines => new[] - { - "GET \0 HTTP/1.1\r\n", - "GET /\0 HTTP/1.1\r\n", - "GET /\0\0 HTTP/1.1\r\n", - "GET /%C8\0 HTTP/1.1\r\n", - }; + public static IEnumerable RequestLineWithNullCharInTargetData => new[] + { + "GET \0 HTTP/1.1\r\n", + "GET /\0 HTTP/1.1\r\n", + "GET /\0\0 HTTP/1.1\r\n", + "GET /%C8\0 HTTP/1.1\r\n", + }; public static TheoryData UnrecognizedHttpVersionData => new TheoryData { @@ -183,7 +203,7 @@ namespace Microsoft.AspNetCore.Testing "8charact", }; - public static IEnumerable InvalidRequestHeaderData + public static IEnumerable RequestHeaderInvalidData { get { @@ -228,6 +248,7 @@ namespace Microsoft.AspNetCore.Testing "Header-1 value1\r\n\r\n", "Header-1 value1\r\nHeader-2: value2\r\n\r\n", "Header-1: value1\r\nHeader-2 value2\r\n\r\n", + "\n" }; // Starting with whitespace @@ -273,11 +294,11 @@ namespace Microsoft.AspNetCore.Testing return new[] { - Tuple.Create(headersWithLineFolding,"Whitespace is not allowed in header name."), - Tuple.Create(headersWithCRInValue,"Header value must not contain CR characters."), - Tuple.Create(headersWithMissingColon,"No ':' character found in header line."), + Tuple.Create(headersWithLineFolding, "Whitespace is not allowed in header name."), + Tuple.Create(headersWithCRInValue, "Header value must not contain CR characters."), + Tuple.Create(headersWithMissingColon, "No ':' character found in header line."), Tuple.Create(headersStartingWithWhitespace, "Whitespace is not allowed in header name."), - Tuple.Create(headersWithWithspaceInName,"Whitespace is not allowed in header name."), + Tuple.Create(headersWithWithspaceInName, "Whitespace is not allowed in header name."), Tuple.Create(headersNotEndingInCrLfLine, "Headers corrupted, invalid header sequence.") } .SelectMany(t => t.Item1.Select(headers => new[] { headers, t.Item2 })); From 13519b6079043f6b800b70011175912abed57e19 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Tue, 7 Mar 2017 17:39:56 -0800 Subject: [PATCH 27/28] Fix CopyTo implementation in benchmark. (#1462) * wip * ooops --- ...pNetCore.Server.Kestrel.Performance.csproj | 3 +- .../RequestParsing.cs | 2 +- .../WritableBufferExtensions.cs | 63 +++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 test/Microsoft.AspNetCore.Server.Kestrel.Performance/WritableBufferExtensions.cs diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj index 746c59cd10..e010d7fce2 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/Microsoft.AspNetCore.Server.Kestrel.Performance.csproj @@ -1,4 +1,4 @@ - + @@ -6,6 +6,7 @@ netcoreapp1.1 Exe true + true false diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs index 90bd7a16c5..8fc4bc0eea 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/RequestParsing.cs @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance private void InsertData(byte[] bytes) { var buffer = Pipe.Writer.Alloc(2048); - buffer.Write(bytes); + buffer.WriteFast(bytes); // There should not be any backpressure and task completes immediately buffer.FlushAsync().GetAwaiter().GetResult(); } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.Performance/WritableBufferExtensions.cs b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/WritableBufferExtensions.cs new file mode 100644 index 0000000000..04a538e4be --- /dev/null +++ b/test/Microsoft.AspNetCore.Server.Kestrel.Performance/WritableBufferExtensions.cs @@ -0,0 +1,63 @@ +using System; +using System.Collections.Generic; +using System.IO.Pipelines; +using System.Text; + +namespace Microsoft.AspNetCore.Server.Kestrel.Performance +{ + internal static class WritableBufferExtensions + { + public static void WriteFast(this WritableBuffer buffer, ReadOnlySpan source) + { + if (buffer.Memory.IsEmpty) + { + buffer.Ensure(); + } + + // Fast path, try copying to the available memory directly + if (source.Length <= buffer.Memory.Length) + { + source.CopyToFast(buffer.Memory.Span); + buffer.Advance(source.Length); + return; + } + + var remaining = source.Length; + var offset = 0; + + while (remaining > 0) + { + var writable = Math.Min(remaining, buffer.Memory.Length); + + buffer.Ensure(writable); + + if (writable == 0) + { + continue; + } + + source.Slice(offset, writable).CopyToFast(buffer.Memory.Span); + + remaining -= writable; + offset += writable; + + buffer.Advance(writable); + } + } + + private unsafe static void CopyToFast(this ReadOnlySpan source, Span destination) + { + if (destination.Length < source.Length) + { + throw new InvalidOperationException(); + } + + // Assume it fits + fixed (byte* pSource = &source.DangerousGetPinnableReference()) + fixed (byte* pDest = &destination.DangerousGetPinnableReference()) + { + Buffer.MemoryCopy(pSource, pDest, destination.Length, source.Length); + } + } + } +} From 1f0bb14546a41ec9b03294df3874206099b52f0b Mon Sep 17 00:00:00 2001 From: David Fowler Date: Wed, 8 Mar 2017 08:43:11 -0800 Subject: [PATCH 28/28] Clean up left over code from port to pipelines (#1465) - Remove unused methods in PipelineExtensions - Remove AwaitableThreadPool since we dispatch reads --- .../Internal/Http/PipelineExtensions.cs | 20 ---------- .../Infrastructure/AwaitableThreadPool.cs | 39 ------------------- 2 files changed, 59 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/AwaitableThreadPool.cs diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs index a37670cfa6..1a014aff56 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/PipelineExtensions.cs @@ -4,9 +4,7 @@ using System; using System.IO.Pipelines; using System.Runtime.CompilerServices; -using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { @@ -48,8 +46,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http { var result = await readingTask; - await AwaitableThreadPool.Yield(); - try { if (!result.Buffer.IsEmpty) @@ -71,13 +67,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } - private static async Task ReadAsyncDispatchedAwaited(ReadableBufferAwaitable awaitable) - { - var result = await awaitable; - await AwaitableThreadPool.Yield(); - return result; - } - [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span ToSpan(this ReadableBuffer buffer) { @@ -88,15 +77,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http return buffer.ToArray(); } - public static ArraySegment ToArraySegment(this ReadableBuffer buffer) - { - if (buffer.IsSingleSpan) - { - return buffer.First.GetArray(); - } - return new ArraySegment(buffer.ToArray()); - } - public static ArraySegment GetArray(this Memory memory) { ArraySegment result; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/AwaitableThreadPool.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/AwaitableThreadPool.cs deleted file mode 100644 index 70930c73ae..0000000000 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Infrastructure/AwaitableThreadPool.cs +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Runtime.CompilerServices; -using System.Threading.Tasks; - -namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure -{ - internal static class AwaitableThreadPool - { - internal static Awaitable Yield() - { - return new Awaitable(); - } - - internal struct Awaitable : ICriticalNotifyCompletion - { - public void GetResult() - { - - } - - public Awaitable GetAwaiter() => this; - - public bool IsCompleted => false; - - public void OnCompleted(Action continuation) - { - Task.Run(continuation); - } - - public void UnsafeOnCompleted(Action continuation) - { - OnCompleted(continuation); - } - } - } -} \ No newline at end of file