From d5b12aeedd015e4bbe9c1ccaa5d95c295dbf1098 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 7 Jan 2022 08:50:16 +1300 Subject: [PATCH 01/12] Detect HTTP version on invalid HTTP/2 connection --- .../src/Internal/Http2/Http2Connection.cs | 33 ++++++++++++++++++- .../Internal/Infrastructure/KestrelTrace.cs | 12 +++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 29e9212a206d..3304748fb07c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -437,7 +437,7 @@ private async Task TryReadPrefaceAsync() return false; } - private static bool ParsePreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private bool ParsePreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; examined = buffer.End; @@ -452,6 +452,17 @@ private static bool ParsePreface(in ReadOnlySequence buffer, out SequenceP if (!span.SequenceEqual(ClientPreface)) { + // This incoming request data isn't valid HTTP/2. Do some additional investigation of the content to see whether + // we can write a clear log message of what is wrong. + // A common pit of failure is not using TLS and having pre-negotated HTTP version. + // + // Do this check when not using TLS. With TLS, ALPN should have already errored if the wrong version is used. + var tlsFeature = ConnectionFeatures.Get(); + if (tlsFeature == null) + { + CheckWrongHttpVersion(buffer); + } + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); } @@ -459,6 +470,26 @@ private static bool ParsePreface(in ReadOnlySequence buffer, out SequenceP return true; } + private void CheckWrongHttpVersion(in ReadOnlySequence buffer) + { + // Check to see if the request bytes are an HTTP/1.x request. + // Initial request line will end with "HTTP/1.0" or "HTTP/1.1". + // Note that this test isn't perfect. It is possible the entire first request line isn't in the buffer./ + // In that case nothing is written to the log. + var reader = new SequenceReader(buffer); + if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n', advancePastDelimiter: true)) + { + if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r') + { + var detectedVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8)); + if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11) + { + Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); + } + } + } + } + private Task ProcessFrameAsync(IHttpApplication application, in ReadOnlySequence payload) where TContext : notnull { // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1.1 diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs index 8c6ecf18ca21..fe0a5a91c898 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs @@ -3,6 +3,7 @@ using System.Net.Http; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http3; using Microsoft.Extensions.Logging; @@ -394,6 +395,17 @@ public void Http3GoAwayStreamId(string connectionId, long goAwayStreamId) Http3GoAwayStreamId(_http3Logger, connectionId, goAwayStreamId); } + [LoggerMessage(54, LogLevel.Debug, @"Connection id ""{ConnectionId}"": Invalid content received on connection. Possible incorrect HTTP version detected. Expected {ExpectedHttpVersion} but received {DetectedHttpVersion}.", EventName = "PossibleInvalidHttpVersionDetected", SkipEnabledCheck = true)] + private static partial void PossibleInvalidHttpVersionDetected(ILogger logger, string connectionId, string expectedHttpVersion, string detectedHttpVersion); + + public void PossibleInvalidHttpVersionDetected(string connectionId, HttpVersion expectedHttpVersion, HttpVersion detectedHttpVersion) + { + if (_generalLogger.IsEnabled(LogLevel.Debug)) + { + PossibleInvalidHttpVersionDetected(_generalLogger, connectionId, HttpUtilities.VersionToString(expectedHttpVersion), HttpUtilities.VersionToString(detectedHttpVersion)); + } + } + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => _generalLogger.Log(logLevel, eventId, state, exception, formatter); From 5c40e68e8a276f028684822de4ef8a4114ca9568 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 7 Jan 2022 09:12:02 +1300 Subject: [PATCH 02/12] Clean up --- .../Core/src/Internal/Http2/Http2Connection.cs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 3304748fb07c..c045ea099de8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -452,11 +452,10 @@ private bool ParsePreface(in ReadOnlySequence buffer, out SequencePosition if (!span.SequenceEqual(ClientPreface)) { - // This incoming request data isn't valid HTTP/2. Do some additional investigation of the content to see whether - // we can write a clear log message of what is wrong. - // A common pit of failure is not using TLS and having pre-negotated HTTP version. + // The incoming request data isn't valid HTTP/2. Investigate the content to see whether we can write a log message of what is wrong. + // A common pit of failure is pre-negotated requests and sending the wrong HTTP version. // - // Do this check when not using TLS. With TLS, ALPN should have already errored if the wrong version is used. + // With TLS, ALPN should have already errored if the wrong HTTP version is used. Do this check when not using TLS. var tlsFeature = ConnectionFeatures.Get(); if (tlsFeature == null) { @@ -472,13 +471,12 @@ private bool ParsePreface(in ReadOnlySequence buffer, out SequencePosition private void CheckWrongHttpVersion(in ReadOnlySequence buffer) { - // Check to see if the request bytes are an HTTP/1.x request. - // Initial request line will end with "HTTP/1.0" or "HTTP/1.1". - // Note that this test isn't perfect. It is possible the entire first request line isn't in the buffer./ - // In that case nothing is written to the log. + // Check to see if the request bytes are an HTTP/1.x request. Initial request line will end with "HTTP/1.0" or "HTTP/1.1". + // Note that this test isn't perfect. It is possible the entire first request line isn't in the buffer. In that case nothing is written to the log. var reader = new SequenceReader(buffer); - if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n', advancePastDelimiter: true)) + if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n')) { + // Line should be long enough for HTTP/1.X and end with \r\n if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r') { var detectedVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8)); From 8b2d98ea17d92d5bd257324e1cdb399e7c816634 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 8 Jan 2022 11:07:29 +1300 Subject: [PATCH 03/12] Return 400 response --- .../src/Internal/Http2/Http2Connection.cs | 133 +++++++++++++----- 1 file changed, 96 insertions(+), 37 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index c045ea099de8..b60b9deaa0ea 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -8,6 +8,7 @@ using System.Net.Http; using System.Net.Http.HPack; using System.Security.Authentication; +using System.Text; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; @@ -402,8 +403,29 @@ private void ValidateTlsRequirements() } } + private enum ReadPrefaceState + { + Preface, + Http1x + } + private async Task TryReadPrefaceAsync() { + // HTTP/1.x and HTTP/2 support connections without TLS. That means ALPN hasn't been used to ensure both sides are + // using the same protocol. A common problem is someone using HTTP/1.x to talk to a HTTP/2 only endpoint. + // + // HTTP/2 starts a connection with a preface. This method reads and validates it. If the connection doesn't start + // with the preface, and it isn't using TLS, then we attempt to detect what the client is trying to do and send + // back a friendly error message. + // + // Outcomes from this method: + // 1. Successfully read HTTP/2 preface. Connection continues to be established. + // 2. Detect HTTP/1.x request. Send back HTTP/1.x 400 response. + // 3. Unknown content. Report HTTP/2 PROTOCOL_ERROR to client. + // 4. Timeout while waiting for content. + // + // Future improvement: Detect TLS frame. Useful for people starting TLS connection with a non-TLS endpoint. + var state = ReadPrefaceState.Preface; while (_isClosed == 0) { var result = await Input.ReadAsync(); @@ -415,9 +437,54 @@ private async Task TryReadPrefaceAsync() { if (!readableBuffer.IsEmpty) { - if (ParsePreface(readableBuffer, out consumed, out examined)) + switch (state) { - return true; + case ReadPrefaceState.Preface: + if (readableBuffer.Length >= ClientPreface.Length) + { + if (IsPreface(readableBuffer, out consumed, out examined)) + { + return true; + } + else + { + // With TLS, ALPN should have already errored if the wrong HTTP version is used. + // Only perform additional validation if endpoint doesn't use TLS. + var tlsFeature = ConnectionFeatures.Get(); + if (tlsFeature == null) + { + state = ReadPrefaceState.Http1x; + goto case ReadPrefaceState.Http1x; + } + + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); + } + } + break; + case ReadPrefaceState.Http1x: + if (ParseHttp1x(readableBuffer, out var detectedVersion)) + { + if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11) + { + Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); + + // TODO: Cache bytes + await _context.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes(@"HTTP/1.1 400 Bad Request +Connection: close + +")); + await _context.Transport.Output.FlushAsync(); + + // Close connection here so no GOAWAY frame is written. + TryClose(); + return false; + } + else + { + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); + } + } + break; } } @@ -437,57 +504,49 @@ private async Task TryReadPrefaceAsync() return false; } - private bool ParsePreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private bool ParseHttp1x(ReadOnlySequence buffer, out HttpVersion httpVersion) { - consumed = buffer.Start; - examined = buffer.End; + httpVersion = HttpVersion.Unknown; - if (buffer.Length < ClientPreface.Length) + var reader = new SequenceReader(buffer); + if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n') && requestLine.Length <= Limits.MaxRequestLineSize) { - return false; + // Line should be long enough for HTTP/1.X and end with \r\n + if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r') + { + httpVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8)); + return true; + } } + // Couldn't find newline within max request line size so this isn't valid HTTP/1.x. + if (buffer.Length > Limits.MaxRequestLineSize) + { + return true; + } + + return false; + } + + private bool IsPreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + { + consumed = buffer.Start; + examined = buffer.End; + + Debug.Assert(buffer.Length >= ClientPreface.Length, "Not enough content to match preface."); + var preface = buffer.Slice(0, ClientPreface.Length); var span = preface.ToSpan(); if (!span.SequenceEqual(ClientPreface)) { - // The incoming request data isn't valid HTTP/2. Investigate the content to see whether we can write a log message of what is wrong. - // A common pit of failure is pre-negotated requests and sending the wrong HTTP version. - // - // With TLS, ALPN should have already errored if the wrong HTTP version is used. Do this check when not using TLS. - var tlsFeature = ConnectionFeatures.Get(); - if (tlsFeature == null) - { - CheckWrongHttpVersion(buffer); - } - - throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); + return false; } consumed = examined = preface.End; return true; } - private void CheckWrongHttpVersion(in ReadOnlySequence buffer) - { - // Check to see if the request bytes are an HTTP/1.x request. Initial request line will end with "HTTP/1.0" or "HTTP/1.1". - // Note that this test isn't perfect. It is possible the entire first request line isn't in the buffer. In that case nothing is written to the log. - var reader = new SequenceReader(buffer); - if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n')) - { - // Line should be long enough for HTTP/1.X and end with \r\n - if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r') - { - var detectedVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8)); - if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11) - { - Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); - } - } - } - } - private Task ProcessFrameAsync(IHttpApplication application, in ReadOnlySequence payload) where TContext : notnull { // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1.1 From 13c7bac61205c7785090a6c83e978c02f44410f5 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 9 Jan 2022 16:16:16 +1300 Subject: [PATCH 04/12] Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs Co-authored-by: Chris Ross --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index b60b9deaa0ea..61c35c92cb29 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -471,7 +471,10 @@ private async Task TryReadPrefaceAsync() // TODO: Cache bytes await _context.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes(@"HTTP/1.1 400 Bad Request Connection: close +Content-Type: text/plain +Content-Length: 57 +An HTTP/1.x request was sent to an HTTP/2 only endpoint. ")); await _context.Transport.Output.FlushAsync(); From c1e5d770e1820c46c8a74d61194663ca59485090 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 9 Jan 2022 21:34:58 +1300 Subject: [PATCH 05/12] Tests --- .../src/Internal/Http2/Http2Connection.cs | 105 ++++++++++-------- .../Internal/Infrastructure/KestrelTrace.cs | 2 +- .../Http2/Http2ConnectionTests.cs | 80 ++++++++++++- .../Http2/Http2TestBase.cs | 23 +++- 4 files changed, 160 insertions(+), 50 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 61c35c92cb29..26ae2eaf77ba 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -24,6 +25,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; internal partial class Http2Connection : IHttp2StreamLifetimeHandler, IHttpStreamHeadersHandler, IRequestProcessor { public static ReadOnlySpan ClientPreface => ClientPrefaceBytes; + public static byte[]? InvalidHttp1xErrorResponseBytes; private const PseudoHeaderFields _mandatoryRequestPseudoHeaderFields = PseudoHeaderFields.Method | PseudoHeaderFields.Path | PseudoHeaderFields.Scheme; @@ -403,10 +405,13 @@ private void ValidateTlsRequirements() } } + [Flags] private enum ReadPrefaceState { - Preface, - Http1x + None = 0, + Preface = 1, + Http1x = 2, + All = Preface | Http1x } private async Task TryReadPrefaceAsync() @@ -425,7 +430,15 @@ private async Task TryReadPrefaceAsync() // 4. Timeout while waiting for content. // // Future improvement: Detect TLS frame. Useful for people starting TLS connection with a non-TLS endpoint. - var state = ReadPrefaceState.Preface; + var state = ReadPrefaceState.All; + + // With TLS, ALPN should have already errored if the wrong HTTP version is used. + // Only perform additional validation if endpoint doesn't use TLS. + if (ConnectionFeatures.Get() != null) + { + state ^= ReadPrefaceState.Http1x; + } + while (_isClosed == 0) { var result = await Input.ReadAsync(); @@ -437,63 +450,60 @@ private async Task TryReadPrefaceAsync() { if (!readableBuffer.IsEmpty) { - switch (state) + if (state.HasFlag(ReadPrefaceState.Preface)) { - case ReadPrefaceState.Preface: - if (readableBuffer.Length >= ClientPreface.Length) + if (readableBuffer.Length >= ClientPreface.Length) + { + if (IsPreface(readableBuffer, out consumed, out examined)) { - if (IsPreface(readableBuffer, out consumed, out examined)) - { - return true; - } - else - { - // With TLS, ALPN should have already errored if the wrong HTTP version is used. - // Only perform additional validation if endpoint doesn't use TLS. - var tlsFeature = ConnectionFeatures.Get(); - if (tlsFeature == null) - { - state = ReadPrefaceState.Http1x; - goto case ReadPrefaceState.Http1x; - } - - throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); - } + return true; } - break; - case ReadPrefaceState.Http1x: - if (ParseHttp1x(readableBuffer, out var detectedVersion)) + else + { + state ^= ReadPrefaceState.Preface; + } + } + } + + if (state.HasFlag(ReadPrefaceState.Http1x)) + { + if (ParseHttp1x(readableBuffer, out var detectedVersion)) + { + if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11) { - if (detectedVersion == HttpVersion.Http10 || detectedVersion == HttpVersion.Http11) - { - Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); + Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); - // TODO: Cache bytes - await _context.Transport.Output.WriteAsync(Encoding.UTF8.GetBytes(@"HTTP/1.1 400 Bad Request + var responseBytes = InvalidHttp1xErrorResponseBytes ??= Encoding.ASCII.GetBytes(@"HTTP/1.1 400 Bad Request Connection: close Content-Type: text/plain Content-Length: 57 An HTTP/1.x request was sent to an HTTP/2 only endpoint. -")); - await _context.Transport.Output.FlushAsync(); - - // Close connection here so no GOAWAY frame is written. - TryClose(); - return false; - } - else - { - throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); - } +"); + + await _context.Transport.Output.WriteAsync(responseBytes); + + // Close connection here so a GOAWAY frame isn't written. + TryClose(); + + return false; + } + else + { + state ^= ReadPrefaceState.Http1x; } - break; + } + } + + if (state == ReadPrefaceState.None) + { + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); } } if (result.IsCompleted) { - return false; + throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); } } finally @@ -511,15 +521,16 @@ private bool ParseHttp1x(ReadOnlySequence buffer, out HttpVersion httpVers { httpVersion = HttpVersion.Unknown; - var reader = new SequenceReader(buffer); - if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n') && requestLine.Length <= Limits.MaxRequestLineSize) + var reader = new SequenceReader(buffer.Length > Limits.MaxRequestLineSize ? buffer.Slice(0, Limits.MaxRequestLineSize) : buffer); + if (reader.TryReadTo(out ReadOnlySpan requestLine, (byte)'\n')) { // Line should be long enough for HTTP/1.X and end with \r\n if (requestLine.Length > 10 && requestLine[requestLine.Length - 1] == (byte)'\r') { httpVersion = HttpUtilities.GetKnownVersion(requestLine.Slice(requestLine.Length - 9, 8)); - return true; } + + return true; } // Couldn't find newline within max request line size so this isn't valid HTTP/1.x. diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs index fe0a5a91c898..ce3b2c224381 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.cs @@ -402,7 +402,7 @@ public void PossibleInvalidHttpVersionDetected(string connectionId, HttpVersion { if (_generalLogger.IsEnabled(LogLevel.Debug)) { - PossibleInvalidHttpVersionDetected(_generalLogger, connectionId, HttpUtilities.VersionToString(expectedHttpVersion), HttpUtilities.VersionToString(detectedHttpVersion)); + PossibleInvalidHttpVersionDetected(_badRequestsLogger, connectionId, HttpUtilities.VersionToString(expectedHttpVersion), HttpUtilities.VersionToString(detectedHttpVersion)); } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index c3819cd0357f..5fb697529441 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -6,13 +6,14 @@ using System.Globalization; using System.Net.Http; using System.Net.Http.HPack; +using System.Net.Security; +using System.Security.Authentication; using System.Text; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; @@ -5211,6 +5212,83 @@ await WaitForConnectionErrorAsync( }); } + [Fact] + public async Task StartConnection_SendPreface_ReturnSettings() + { + await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + + await SendAsync(Http2Connection.ClientPreface); + + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 3 * Http2FrameReader.SettingSize, + withFlags: 0, + withStreamId: 0); + + await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: true); + } + + [Fact] + public async Task StartConnection_SendHttp1xRequest_ReturnHttp11Status400() + { + await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + + await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n")); + + var data = await ReadAllAsync(); + + Assert.NotNull(Http2Connection.InvalidHttp1xErrorResponseBytes); + Assert.Equal(Http2Connection.InvalidHttp1xErrorResponseBytes, data); + } + + [Fact] + public async Task StartConnection_SendHttp1xRequest_ExceedsRequestLineLimit_ProtocolError() + { + await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + + await SendAsync(Encoding.ASCII.GetBytes($"GET /{new string('a', _connection.Limits.MaxRequestLineSize)} HTTP/1.1\r\n")); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 0, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface); + } + + [Fact] + public async Task StartTlsConnection_SendHttp1xRequest_ProtocolError() + { + CreateConnection(); + + var tlsHandshakeMock = new Mock(); + tlsHandshakeMock.SetupGet(m => m.Protocol).Returns(SslProtocols.Tls12); + _connection.ConnectionFeatures.Set(tlsHandshakeMock.Object); + + await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + + await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n")); + _pair.Application.Output.Complete(); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 0, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface); + } + + [Fact] + public async Task StartConnection_SendNothing_ProtocolError() + { + await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + + _pair.Application.Output.Complete(); + + await WaitForConnectionErrorAsync( + ignoreNonGoAwayFrames: false, + expectedLastStreamId: 0, + expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, + expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface); + } + public static TheoryData UpperCaseHeaderNameData { get diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 21c5bf5ac614..dd7d062f5810 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -470,7 +470,7 @@ protected void CreateConnection() _timeoutControl.Initialize(_serviceContext.SystemClock.UtcNow.Ticks); } - protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 3, bool expectedWindowUpdate = true) + protected async Task InitializeConnectionWithoutPrefaceAsync(RequestDelegate application) { if (_connection == null) { @@ -496,6 +496,11 @@ async Task CompletePipeOnTaskCompletion() // Lose xUnit's AsyncTestSyncContext so middleware always runs inline for better determinism. await ThreadPoolAwaitable.Instance; + } + + protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 3, bool expectedWindowUpdate = true) + { + await InitializeConnectionWithoutPrefaceAsync(application); await SendPreambleAsync(); await SendSettingsAsync(); @@ -1109,6 +1114,22 @@ protected Task SendUnknownFrameTypeAsync(int streamId, int frameType) return FlushAsync(outputWriter); } + internal async Task ReadAllAsync() + { + while (true) + { + var result = await _pair.Application.Input.ReadAsync().AsTask().DefaultTimeout(); + + if (result.IsCompleted) + { + return result.Buffer.ToArray(); + } + + // Consume nothing, just wait for everything + _pair.Application.Input.AdvanceTo(result.Buffer.Start, result.Buffer.End); + } + } + internal async Task ReceiveFrameAsync(uint maxFrameSize = Http2PeerSettings.DefaultMaxFrameSize) { var frame = new Http2FrameWithPayload(); From 4604e9bce59d21b7267d75c3381ee6243760b1ab Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 9 Jan 2022 21:42:45 +1300 Subject: [PATCH 06/12] Fix build --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 26ae2eaf77ba..42be8f52fb60 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -12,7 +12,6 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Hosting.Server; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; From 0b7abc30eb90eaed770d33270989471df3b18224 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 Jan 2022 16:47:25 +1300 Subject: [PATCH 07/12] PR feedback --- .../Core/src/Internal/Http2/Http2Connection.cs | 17 +++++++++-------- .../Http2/Http2ConnectionTests.cs | 17 ++++------------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 42be8f52fb60..972902bc00b0 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -472,13 +472,13 @@ private async Task TryReadPrefaceAsync() { Log.PossibleInvalidHttpVersionDetected(ConnectionId, HttpVersion.Http2, detectedVersion); - var responseBytes = InvalidHttp1xErrorResponseBytes ??= Encoding.ASCII.GetBytes(@"HTTP/1.1 400 Bad Request -Connection: close -Content-Type: text/plain -Content-Length: 57 - -An HTTP/1.x request was sent to an HTTP/2 only endpoint. -"); + var responseBytes = InvalidHttp1xErrorResponseBytes ??= Encoding.ASCII.GetBytes( + "HTTP/1.1 400 Bad Request\r\n" + + "Connection: close\r\n" + + "Content-Type: text/plain\r\n" + + "Content-Length: 56\r\n" + + "\r\n" + + "An HTTP/1.x request was sent to an HTTP/2 only endpoint."); await _context.Transport.Output.WriteAsync(responseBytes); @@ -494,6 +494,7 @@ An HTTP/1.x request was sent to an HTTP/2 only endpoint. } } + // Tested all states. Return HTTP/2 protocol error. if (state == ReadPrefaceState.None) { throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); @@ -502,7 +503,7 @@ An HTTP/1.x request was sent to an HTTP/2 only endpoint. if (result.IsCompleted) { - throw new Http2ConnectionErrorException(CoreStrings.Http2ErrorInvalidPreface, Http2ErrorCode.PROTOCOL_ERROR); + return false; } } finally diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 5fb697529441..51711f307ad4 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -5255,7 +5255,7 @@ await WaitForConnectionErrorAsync( } [Fact] - public async Task StartTlsConnection_SendHttp1xRequest_ProtocolError() + public async Task StartTlsConnection_SendHttp1xRequest_NoError() { CreateConnection(); @@ -5266,27 +5266,18 @@ public async Task StartTlsConnection_SendHttp1xRequest_ProtocolError() await InitializeConnectionWithoutPrefaceAsync(_noopApplication); await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n")); - _pair.Application.Output.Complete(); - await WaitForConnectionErrorAsync( - ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, - expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, - expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface); + await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } [Fact] - public async Task StartConnection_SendNothing_ProtocolError() + public async Task StartConnection_SendNothing_NoError() { await InitializeConnectionWithoutPrefaceAsync(_noopApplication); _pair.Application.Output.Complete(); - await WaitForConnectionErrorAsync( - ignoreNonGoAwayFrames: false, - expectedLastStreamId: 0, - expectedErrorCode: Http2ErrorCode.PROTOCOL_ERROR, - expectedErrorMessage: CoreStrings.Http2ErrorInvalidPreface); + await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } public static TheoryData UpperCaseHeaderNameData From b8e05fef34bcb3ea554902a9499c4b592bb7564e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 11 Jan 2022 16:53:45 +1300 Subject: [PATCH 08/12] Clean up --- .../test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 51711f307ad4..1df812999841 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -5275,8 +5275,6 @@ public async Task StartConnection_SendNothing_NoError() { await InitializeConnectionWithoutPrefaceAsync(_noopApplication); - _pair.Application.Output.Complete(); - await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } From 36892bfbf9ae92d8c79efb9714be5fac6d96ef77 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 12 Jan 2022 16:10:38 +1300 Subject: [PATCH 09/12] Add integration test --- .../Http2/Http2RequestTests.cs | 44 +++++++++++++++++++ .../Http3/Http3RequestTests.cs | 42 +++++++++--------- .../Http3/Http3TlsTests.cs | 14 +++--- .../{Http3/Http3Helpers.cs => HttpHelpers.cs} | 11 +++-- 4 files changed, 79 insertions(+), 32 deletions(-) create mode 100644 src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs rename src/Servers/Kestrel/test/Interop.FunctionalTests/{Http3/Http3Helpers.cs => HttpHelpers.cs} (90%) diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs new file mode 100644 index 000000000000..d349c2adc6f1 --- /dev/null +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs @@ -0,0 +1,44 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Net.Http; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.Hosting; + +namespace Interop.FunctionalTests.Http2; + +public class Http2RequestTests : LoggedTest +{ + [Fact] + public async Task GET_NoTLS_Http11RequestToHttp2Endpoint_400Result() + { + // Arrange + var builder = CreateHostBuilder(c => Task.CompletedTask, protocol: HttpProtocols.Http2); + + using (var host = builder.Build()) + using (var client = HttpHelpers.CreateClient()) + { + await host.StartAsync().DefaultTimeout(); + + var request = new HttpRequestMessage(HttpMethod.Get, $"http://127.0.0.1:{host.GetPort()}/"); + request.Version = HttpVersion.Version11; + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + + // Act + var responseMessage = await client.SendAsync(request, CancellationToken.None).DefaultTimeout(); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, responseMessage.StatusCode); + Assert.Equal("An HTTP/1.x request was sent to an HTTP/2 only endpoint.", await responseMessage.Content.ReadAsStringAsync()); + } + } + + private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null) + { + return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); + } +} diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs index 02533f10d645..8f1ec9e31a59 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs @@ -88,7 +88,7 @@ public async Task GET_MiddlewareIsRunWithConnectionLoggingScopeForHttpRequests(H }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -190,7 +190,7 @@ public async Task GET_ServerStreaming_ClientReadsPartialResponse(HttpProtocols p }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -229,7 +229,7 @@ public async Task POST_ClientSendsOnlyHeaders_RequestReceivedOnServer(HttpProtoc }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -273,7 +273,7 @@ public async Task POST_ServerCompletesWithoutReadingRequestBody_ClientGetsRespon }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -341,7 +341,7 @@ public async Task POST_ClientCancellationUpload_RequestAbortRaised(HttpProtocols }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -414,7 +414,7 @@ public async Task POST_ServerAbort_ClientReceivesAbort(HttpProtocols protocol) }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -480,7 +480,7 @@ public async Task GET_ServerAbort_ClientReceivesAbort(HttpProtocols protocol) }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -526,7 +526,7 @@ public async Task POST_Expect100Continue_Get100Continue() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient(expect100ContinueTimeout: TimeSpan.FromMinutes(20))) + using (var client = HttpHelpers.CreateClient(expect100ContinueTimeout: TimeSpan.FromMinutes(20))) { await host.StartAsync().DefaultTimeout(); @@ -591,7 +591,7 @@ public async Task GET_MultipleRequestsInSequence_ReusedState() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -901,7 +901,7 @@ public async Task StreamResponseContent_DelayAndTrailers_ClientSuccess() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -954,7 +954,7 @@ public async Task GET_MultipleRequests_ConnectionAndTraceIdsUpdated() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1020,7 +1020,7 @@ public async Task GET_MultipleRequestsInSequence_ReusedRequestHeaderStrings() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1073,7 +1073,7 @@ public async Task Get_CompleteAsyncAndReset_StreamNotPooled() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1134,7 +1134,7 @@ public async Task GET_ConnectionLoggingConfigured_OutputToLogs() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1199,7 +1199,7 @@ public async Task GET_ClientDisconnected_ConnectionAbortRaised() { await host.StartAsync(); - var client = Http3Helpers.CreateClient(); + var client = HttpHelpers.CreateClient(); try { var port = host.GetPort(); @@ -1259,7 +1259,7 @@ public async Task ConnectionLifetimeNotificationFeature_RequestClose_ConnectionE }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1364,7 +1364,7 @@ public async Task GET_ServerAbortTransport_ConnectionAbortRaised() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1441,7 +1441,7 @@ public async Task GET_ConnectionInfo_PropertiesSet() }); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync(); @@ -1499,7 +1499,7 @@ public async Task GET_GracefulServerShutdown_AbortRequestsAfterHostTimeout(HttpP }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -1595,7 +1595,7 @@ public async Task GET_GracefulServerShutdown_RequestCompleteSuccessfullyInsideHo }, protocol: protocol); using (var host = builder.Build()) - using (var client = Http3Helpers.CreateClient()) + using (var client = HttpHelpers.CreateClient()) { await host.StartAsync().DefaultTimeout(); @@ -1640,6 +1640,6 @@ public async Task GET_GracefulServerShutdown_RequestCompleteSuccessfullyInsideHo private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null) { - return Http3Helpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); + return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); } } diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3TlsTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3TlsTests.cs index 1427c5d767a6..5c97445fb597 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3TlsTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3TlsTests.cs @@ -42,7 +42,7 @@ public async Task ServerCertificateSelector_Invoked() }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(); + using var client = HttpHelpers.CreateClient(); await host.StartAsync().DefaultTimeout(); @@ -90,7 +90,7 @@ public async Task ClientCertificate_AllowOrRequire_Available_Accepted(ClientCert }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(includeClientCert: true); + using var client = HttpHelpers.CreateClient(includeClientCert: true); await host.StartAsync().DefaultTimeout(); @@ -133,7 +133,7 @@ public async Task ClientCertificate_NoOrDelayed_Available_Ignored(ClientCertific }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(includeClientCert: true); + using var client = HttpHelpers.CreateClient(includeClientCert: true); await host.StartAsync().DefaultTimeout(); @@ -183,7 +183,7 @@ public async Task ClientCertificate_AllowOrRequire_Available_Invalid_Refused(Cli }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(includeClientCert: true); + using var client = HttpHelpers.CreateClient(includeClientCert: true); await host.StartAsync().DefaultTimeout(); @@ -236,7 +236,7 @@ public async Task ClientCertificate_Allow_NotAvailable_Optional() }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(includeClientCert: false); + using var client = HttpHelpers.CreateClient(includeClientCert: false); await host.StartAsync().DefaultTimeout(); @@ -271,7 +271,7 @@ public async Task OnAuthentice_Available_Throws() }); using var host = builder.Build(); - using var client = Http3Helpers.CreateClient(); + using var client = HttpHelpers.CreateClient(); var exception = await Assert.ThrowsAsync(() => host.StartAsync().DefaultTimeout()); @@ -280,6 +280,6 @@ public async Task OnAuthentice_Available_Throws() private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null) { - return Http3Helpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); + return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); } } diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3Helpers.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs similarity index 90% rename from src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3Helpers.cs rename to src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs index 6f9f8414a786..2e3486d8f0bd 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3Helpers.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs @@ -15,9 +15,9 @@ using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; -namespace Interop.FunctionalTests.Http3; +namespace Interop.FunctionalTests; -public static class Http3Helpers +internal static class HttpHelpers { public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, TimeSpan? expect100ContinueTimeout = null, bool includeClientCert = false) { @@ -42,7 +42,7 @@ public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, Time return new HttpMessageInvoker(handler); } - public static IHostBuilder CreateHostBuilder(Action configureServices, RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null) + public static IHostBuilder CreateHostBuilder(Action configureServices, RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null, bool? skipHttps = null) { return new HostBuilder() .ConfigureWebHost(webHostBuilder => @@ -55,7 +55,10 @@ public static IHostBuilder CreateHostBuilder(Action configur o.Listen(IPAddress.Parse("127.0.0.1"), 0, listenOptions => { listenOptions.Protocols = protocol ?? HttpProtocols.Http3; - listenOptions.UseHttps(); + if (skipHttps ?? false) + { + listenOptions.UseHttps(); + } }); } else From 7f9a1c59fc7ef310260567277f087549fe49e2ca Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 12 Jan 2022 21:16:53 +1300 Subject: [PATCH 10/12] Fix build --- src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs index 2e3486d8f0bd..2070ba5c942d 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs @@ -55,7 +55,7 @@ public static IHostBuilder CreateHostBuilder(Action configur o.Listen(IPAddress.Parse("127.0.0.1"), 0, listenOptions => { listenOptions.Protocols = protocol ?? HttpProtocols.Http3; - if (skipHttps ?? false) + if (skipHttps ?? true) { listenOptions.UseHttps(); } From ad59352e3701123fc839e729f27e768de2192a7f Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 12 Jan 2022 21:52:32 +1300 Subject: [PATCH 11/12] Fix build --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index 972902bc00b0..0858890205f5 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -542,7 +542,7 @@ private bool ParseHttp1x(ReadOnlySequence buffer, out HttpVersion httpVers return false; } - private bool IsPreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) + private static bool IsPreface(in ReadOnlySequence buffer, out SequencePosition consumed, out SequencePosition examined) { consumed = buffer.Start; examined = buffer.End; From 4d2a3c00628d54354941d0fc706d7f9953871945 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 13 Jan 2022 06:03:59 +1300 Subject: [PATCH 12/12] Fix build --- .../test/Interop.FunctionalTests/Http2/Http2RequestTests.cs | 6 +++--- .../Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs index d349c2adc6f1..142fb5577033 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/Http2/Http2RequestTests.cs @@ -17,7 +17,7 @@ public class Http2RequestTests : LoggedTest public async Task GET_NoTLS_Http11RequestToHttp2Endpoint_400Result() { // Arrange - var builder = CreateHostBuilder(c => Task.CompletedTask, protocol: HttpProtocols.Http2); + var builder = CreateHostBuilder(c => Task.CompletedTask, protocol: HttpProtocols.Http2, plaintext: true); using (var host = builder.Build()) using (var client = HttpHelpers.CreateClient()) @@ -37,8 +37,8 @@ public async Task GET_NoTLS_Http11RequestToHttp2Endpoint_400Result() } } - private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null) + private IHostBuilder CreateHostBuilder(RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null, bool? plaintext = null) { - return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel); + return HttpHelpers.CreateHostBuilder(AddTestLogging, requestDelegate, protocol, configureKestrel, plaintext); } } diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs index 2070ba5c942d..dcb49dd6ce45 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpHelpers.cs @@ -42,7 +42,7 @@ public static HttpMessageInvoker CreateClient(TimeSpan? idleTimeout = null, Time return new HttpMessageInvoker(handler); } - public static IHostBuilder CreateHostBuilder(Action configureServices, RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null, bool? skipHttps = null) + public static IHostBuilder CreateHostBuilder(Action configureServices, RequestDelegate requestDelegate, HttpProtocols? protocol = null, Action configureKestrel = null, bool? plaintext = null) { return new HostBuilder() .ConfigureWebHost(webHostBuilder => @@ -55,7 +55,7 @@ public static IHostBuilder CreateHostBuilder(Action configur o.Listen(IPAddress.Parse("127.0.0.1"), 0, listenOptions => { listenOptions.Protocols = protocol ?? HttpProtocols.Http3; - if (skipHttps ?? true) + if (!(plaintext ?? false)) { listenOptions.UseHttps(); }