Skip to content

Fix incorrect HTTP/2 method header decoding #40412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ private void OnHeaderCore(HeaderType headerType, int? staticTableIndex, ReadOnly
{
UpdateHeaderParsingState(value, GetPseudoHeaderField(staticTableIndex.GetValueOrDefault()));

_currentHeadersStream.OnHeader(staticTableIndex.GetValueOrDefault(), indexOnly: true, name, value);
_currentHeadersStream.OnHeader(staticTableIndex.GetValueOrDefault(), indexOnly: false, name, value);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ private void OnHeaderCore(HeaderType headerType, int? staticTableIndex, ReadOnly
{
UpdateHeaderParsingState(value, GetPseudoHeaderField(staticTableIndex.GetValueOrDefault()));

OnHeader(staticTableIndex.GetValueOrDefault(), indexOnly: true, name, value);
OnHeader(staticTableIndex.GetValueOrDefault(), indexOnly: false, name, value);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

<ItemGroup>
<Compile Include="$(KestrelSharedSourceRoot)\PooledStreamStack.cs" Link="Internal\PooledStreamStack.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\HPackHeaderWriter.cs" Link="Internal\Http2\HPackHeaderWriter.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\Http2HeadersEnumerator.cs" Link="Internal\Http2\Http2HeadersEnumerator.cs" />
<Compile Include="$(SharedSourceRoot)CertificateGeneration\**\*.cs" />
<Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" />
<Compile Include="$(SharedSourceRoot)UrlDecoder\**\*.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<TestGroupName>Kestrel.Core.Tests</TestGroupName>
<DefineConstants>$(DefineConstants);KESTREL</DefineConstants>
<DefineConstants>$(DefineConstants);KESTREL;IS_TESTS</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)NullScope.cs" />
<Compile Include="$(SharedSourceRoot)Buffers.Testing\*.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)KnownHeaders.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)\HPackHeaderWriter.cs" Link="Http2\HPackHeaderWriter.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\Http2HeadersEnumerator.cs" Link="Http2\Http2HeadersEnumerator.cs" />
<Compile Include="$(RepoRoot)src\Shared\Buffers.MemoryPool\*.cs" LinkBase="MemoryPool" />
<Compile Include="$(KestrelSharedSourceRoot)\CorrelationIdGenerator.cs" Link="Internal\CorrelationIdGenerator.cs" />
<Compile Include="$(SharedSourceRoot)test\Shared.Tests\runtime\Http2\*.cs" LinkBase="Shared\runtime\Http2" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<ItemGroup>
<Compile Include="$(SharedSourceRoot)NullScope.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestResources.cs" Link="shared\TestResources.cs" />
<Content Include="$(KestrelSharedSourceRoot)test\TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
<Content Include="$(KestrelSharedSourceRoot)test\TestCertificates\*.crt" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
<Content Include="$(KestrelSharedSourceRoot)test\TestCertificates\*.key" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@

<ItemGroup>
<Compile Include="$(SharedSourceRoot)NullScope.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)test\HttpEventSourceListener.cs" Link="shared\HttpEventSourceListener.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestResources.cs" Link="shared\TestResources.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\StreamExtensions.cs" Link="shared\StreamExtensions.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\KestrelTestLoggerProvider.cs" Link="shared\KestrelTestLoggerProvider.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestApplicationErrorLoggerLoggedTest.cs" Link="shared\TestApplicationErrorLoggerLoggedTest.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestApplicationErrorLogger.cs" Link="shared\TestApplicationErrorLogger.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TransportTestHelpers\IHostPortExtensions.cs" Link="shared\TransportTestHelpers\IHostPortExtensions.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TransportTestHelpers\MsQuicSupportedAttribute.cs" Link="shared\TransportTestHelpers\MsQuicSupportedAttribute.cs" />
<Content Include="$(KestrelSharedSourceRoot)test\TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Primitives;
using Microsoft.Net.Http.Headers;
using Http2HeadersEnumerator = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2HeadersEnumerator;

namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
using Microsoft.Extensions.Primitives;
using Http2HeadersEnumerator = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2HeadersEnumerator;

namespace Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<ServerGarbageCollection>true</ServerGarbageCollection>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<TieredCompilation>false</TieredCompilation>
<DefineConstants>$(DefineConstants);IS_BENCHMARKS</DefineConstants>
</PropertyGroup>

<ItemGroup>
Expand All @@ -15,6 +16,8 @@
<Compile Include="$(KestrelSharedSourceRoot)\CompletionPipeReader.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\CompletionPipeWriter.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\PipeWriterHttp2FrameExtensions.cs" Link="Internal\PipeWriterHttp2FrameExtensions.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\HPackHeaderWriter.cs" Link="Http2\HPackHeaderWriter.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\Http2HeadersEnumerator.cs" Link="Http2\Http2HeadersEnumerator.cs" />
<Compile Include="$(RepoRoot)src\Shared\Buffers.MemoryPool\*.cs" LinkBase="MemoryPool" />
<Compile Include="$(KestrelSharedSourceRoot)test\KestrelTestLoggerProvider.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestApplicationErrorLogger.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@
using System.Net.Http;
using System.Net.Http.HPack;

#if !(IS_TESTS || IS_BENCHMARKS)
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
#else
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests;
#endif

// This file is used by Kestrel to write response headers and tests to write request headers.
// To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs.
internal static class HPackHeaderWriter
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,32 @@
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;
using Microsoft.Extensions.Primitives;

#if !(IS_TESTS || IS_BENCHMARKS)
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
#else
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests;
#endif

#nullable enable

// This file is used by Kestrel to write response headers and tests to write request headers.
// To avoid adding test code to Kestrel this file is shared. Test specifc code is excluded from Kestrel by ifdefs.
internal sealed class Http2HeadersEnumerator : IEnumerator<KeyValuePair<string, string>>
{
private enum HeadersType : byte
{
Headers,
Trailers,
Untyped
#if IS_TESTS || IS_BENCHMARKS
Untyped,
#endif
}
private HeadersType _headersType;
private HttpResponseHeaders.Enumerator _headersEnumerator;
private HttpResponseTrailers.Enumerator _trailersEnumerator;
#if IS_TESTS || IS_BENCHMARKS
private IEnumerator<KeyValuePair<string, StringValues>>? _genericEnumerator;
#endif
private StringValues.Enumerator _stringValuesEnumerator;
private bool _hasMultipleValues;
private KnownHeaderType _knownHeaderType;
Expand All @@ -47,6 +59,7 @@ public void Initialize(HttpResponseTrailers headers)
_hasMultipleValues = false;
}

#if IS_TESTS || IS_BENCHMARKS
public void Initialize(IDictionary<string, StringValues> headers)
{
switch (headers)
Expand All @@ -67,6 +80,7 @@ public void Initialize(IDictionary<string, StringValues> headers)

_hasMultipleValues = false;
}
#endif

public bool MoveNext()
{
Expand All @@ -89,12 +103,36 @@ public bool MoveNext()
}
else
{
#if IS_TESTS || IS_BENCHMARKS
return _genericEnumerator!.MoveNext()
? SetCurrent(_genericEnumerator.Current.Key, _genericEnumerator.Current.Value, default)
? SetCurrent(_genericEnumerator.Current.Key, _genericEnumerator.Current.Value, GetKnownRequestHeaderType(_genericEnumerator.Current.Key))
: false;
#else
ThrowUnexpectedHeadersType();
return false;
#endif
}
}

#if IS_TESTS || IS_BENCHMARKS
private static KnownHeaderType GetKnownRequestHeaderType(string headerName)
{
switch (headerName)
{
case ":method":
return KnownHeaderType.Method;
default:
return default;
}
}
#else
[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
private static void ThrowUnexpectedHeadersType()
{
throw new InvalidOperationException("Unexpected headers collection type.");
}
#endif

private bool MoveNextOnStringEnumerator(string key)
{
var result = _stringValuesEnumerator.MoveNext();
Expand Down Expand Up @@ -134,7 +172,11 @@ public void Reset()
}
else
{
#if IS_TESTS || IS_BENCHMARKS
_genericEnumerator!.Reset();
#else
ThrowUnexpectedHeadersType();
#endif
}
_stringValuesEnumerator = default;
_knownHeaderType = default;
Expand Down Expand Up @@ -199,6 +241,11 @@ internal static int GetResponseHeaderStaticTableId(KnownHeaderType responseHeade
return H2StaticTable.ContentLength;
default:
return -1;
#if IS_TESTS || IS_BENCHMARKS
// Include request headers for tests.
case KnownHeaderType.Method:
return H2StaticTable.MethodGet;
#endif
}
}
}
8 changes: 4 additions & 4 deletions src/Servers/Kestrel/shared/test/Http3/Http3InMemory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ internal async ValueTask<Http3ControlStream> GetInboundControlStream()
if (_inboundControlStream == null)
{
var reader = MultiplexedConnectionContext.ToClientAcceptQueue.Reader;
#if IS_FUNCTIONAL_TESTS
#if IS_TESTS
while (await reader.WaitToReadAsync().DefaultTimeout())
#else
while (await reader.WaitToReadAsync())
Expand Down Expand Up @@ -147,7 +147,7 @@ internal async Task WaitForConnectionErrorAsync<TException>(bool ignoreNonGoAway
AssertConnectionError<TException>(expectedErrorCode, matchExpectedErrorMessage, expectedErrorMessage);

// Verify HttpConnection.ProcessRequestsAsync has exited.
#if IS_FUNCTIONAL_TESTS
#if IS_TESTS
await _connectionTask.DefaultTimeout();
#else
await _connectionTask;
Expand Down Expand Up @@ -461,7 +461,7 @@ protected Task SendAsync(ReadOnlySpan<byte> span)
protected static Task FlushAsync(PipeWriter writableBuffer)
{
var task = writableBuffer.FlushAsync();
#if IS_FUNCTIONAL_TESTS
#if IS_TESTS
return task.AsTask().DefaultTimeout();
#else
return task.GetAsTask();
Expand All @@ -477,7 +477,7 @@ internal async Task ReceiveEndAsync()
}
}

#if IS_FUNCTIONAL_TESTS
#if IS_TESTS
protected Task<ReadResult> ReadApplicationInputAsync()
{
return Pair.Application.Input.ReadAsync().AsTask().DefaultTimeout();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
using System.IO.Pipelines;
using System.Net.Http.HPack;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2;
using Http2HeadersEnumerator = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.Http2HeadersEnumerator;
using HPackHeaderWriter = Microsoft.AspNetCore.Server.Kestrel.Core.Tests.HPackHeaderWriter;

namespace Microsoft.AspNetCore.Testing;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
using Microsoft.AspNetCore.Server.Kestrel.FunctionalTests;
using Microsoft.AspNetCore.Server.Kestrel.Https;
using Microsoft.AspNetCore.Server.Kestrel.Https.Internal;
using Microsoft.AspNetCore.Server.Kestrel.Tests;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,41 @@ public async Task HEADERS_Received_KnownOrCustomMethods_Accepted(string method)
};
await InitializeConnectionAsync(_echoMethodNoBody);

// First request
await StartStreamAsync(1, headers, endStream: true);

var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
var headersFrame1 = await ExpectAsync(Http2FrameType.HEADERS,
withLength: 45 + method.Length,
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
withStreamId: 1);

await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
_hpackDecoder.Decode(headersFrame1.PayloadSequence, endHeaders: false, handler: this);

_hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
Assert.Equal(4, _decodedHeaders.Count);
Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
Assert.Equal(method, _decodedHeaders["Method"]);
_decodedHeaders.Clear();

// Second request (will use dynamic table indexes)
await StartStreamAsync(3, headers, endStream: true);

var headersFrame2 = await ExpectAsync(Http2FrameType.HEADERS,
withLength: 7,
withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
withStreamId: 3);

await StopConnectionAsync(expectedLastStreamId: 3, ignoreNonGoAwayFrames: false);

_hpackDecoder.Decode(headersFrame2.PayloadSequence, endHeaders: false, handler: this);

Assert.Equal(4, _decodedHeaders.Count);
Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
Assert.Equal(method, _decodedHeaders["Method"]);
_decodedHeaders.Clear();
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
<ServerGarbageCollection>true</ServerGarbageCollection>
<TestGroupName>InMemory.FunctionalTests</TestGroupName>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DefineConstants>$(DefineConstants);IS_FUNCTIONAL_TESTS</DefineConstants>
<DefineConstants>$(DefineConstants);IS_TESTS</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="$(SharedSourceRoot)NullScope.cs" />
<Compile Include="$(SharedSourceRoot)SyncPoint\SyncPoint.cs" Link="SyncPoint.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)test\Http3\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)\HPackHeaderWriter.cs" Link="Http2\HPackHeaderWriter.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\Http2HeadersEnumerator.cs" Link="Http2\Http2HeadersEnumerator.cs" />
<Compile Include="$(RepoRoot)src\Shared\Buffers.MemoryPool\*.cs" LinkBase="MemoryPool" />
<Compile Include="$(KestrelSharedSourceRoot)\CompletionPipeReader.cs" Link="Internal\CompletionPipeReader.cs" />
<Compile Include="$(KestrelSharedSourceRoot)\CompletionPipeWriter.cs" Link="Internal\CompletionPipeWriter.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
Expand All @@ -10,7 +10,16 @@
<Compile Include="..\BindTests\**\*.cs" />
<Compile Include="..\Sockets.FunctionalTests\TransportSelector.cs" />
<Compile Include="$(SharedSourceRoot)NullScope.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\*.cs" LinkBase="shared" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestServiceContext.cs" Link="shared\TestServiceContext.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestConnection.cs" Link="shared\TestConnection.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\MockSystemClock.cs" Link="shared\MockSystemClock.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestApplicationErrorLoggerLoggedTest.cs" Link="shared\TestApplicationErrorLoggerLoggedTest.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestApplicationErrorLogger.cs" Link="shared\TestApplicationErrorLogger.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\StreamBackedTestConnection.cs" Link="shared\StreamBackedTestConnection.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestResources.cs" Link="shared\TestResources.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TestConstants.cs" Link="shared\TestConstants.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\KestrelTestLoggerProvider.cs" Link="shared\KestrelTestLoggerProvider.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\LifetimeNotImplemented.cs" Link="shared\LifetimeNotImplemented.cs" />
<Compile Include="$(KestrelSharedSourceRoot)test\TransportTestHelpers\*.cs" LinkBase="shared\TransportTestHelpers" />
<Content Include="$(KestrelSharedSourceRoot)test\TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
Expand Down
Loading