From 556fb405e70026ab5f7a95a945e0865e53981a74 Mon Sep 17 00:00:00 2001 From: Cory Nelson Date: Fri, 14 Feb 2020 10:34:02 -0800 Subject: [PATCH] Get tests running for HTTP/3 (#31898) Update generic tests to use a Version rather than boolean IsHttp11/IsHttp20, and update some HTTP/2 to work for HTTP/3. Enable tests for HTTP/3, behind a conditional feature test. Fix QPackDecoder lzcnt assuming an 8-bit test. Rename test QPACK classes from QPackEncoder/QPackDecoder -> QPackTestEncoder/QPackTestDecoder to avoid naming confusion with product code classes. Fix QPackTestDecoder bit flag checks. Fix a double call to QuicConnection.CloseAsync(). Update to shutdown QuicConnection in a background task. Fix test cert usage. --- .../Http/aspnetcore/Http3/QPack/QPackDecoder.cs | 2 +- .../tests/System/Net/Configuration.Certificates.cs | 41 +++++++ .../tests/System/Net/Http/GenericLoopbackServer.cs | 5 +- .../System/Net/Http/Http2LoopbackConnection.cs | 2 +- .../tests/System/Net/Http/Http2LoopbackServer.cs | 4 +- .../System/Net/Http/Http3LoopbackConnection.cs | 40 ++++++- .../tests/System/Net/Http/Http3LoopbackServer.cs | 14 +-- .../tests/System/Net/Http/Http3LoopbackStream.cs | 22 ++-- .../Net/Http/HttpClientHandlerTest.Cancellation.cs | 20 ++-- .../Net/Http/HttpClientHandlerTest.Cookies.cs | 2 +- .../tests/System/Net/Http/HttpClientHandlerTest.cs | 28 ++--- .../System/Net/Http/HttpClientHandlerTestBase.cs | 4 +- .../Common/tests/System/Net/Http/LoopbackServer.cs | 6 +- .../Http/{QPackDecoder.cs => QPackTestDecoder.cs} | 104 +++++++--------- .../Http/{QPackEncoder.cs => QPackTestEncoder.cs} | 2 +- ...Net.Http.WinHttpHandler.Functional.Tests.csproj | 8 +- .../Net/Http/SocketsHttpHandler/Http3Connection.cs | 131 +++++++++++++++------ .../Http/SocketsHttpHandler/Http3RequestStream.cs | 36 +++--- .../Http/SocketsHttpHandler/HttpConnectionPool.cs | 32 +++-- .../SocketsHttpHandler/HttpConnectionSettings.cs | 4 + .../HttpClientHandlerTest.AltSvc.cs | 17 ++- .../HttpClientHandlerTest.Headers.cs | 4 +- ...HttpClientHandlerTestBase.SocketsHttpHandler.cs | 15 +++ .../FunctionalTests/HttpClientMiniStressTest.cs | 5 +- .../FunctionalTests/SocketsHttpHandlerTest.cs | 44 ++++--- .../System.Net.Http.Functional.Tests.csproj | 8 +- 26 files changed, 379 insertions(+), 221 deletions(-) rename src/libraries/Common/tests/System/Net/Http/{QPackDecoder.cs => QPackTestDecoder.cs} (73%) rename src/libraries/Common/tests/System/Net/Http/{QPackEncoder.cs => QPackTestEncoder.cs} (99%) diff --git a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs index dfde487..6f63d66 100644 --- a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs +++ b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackDecoder.cs @@ -224,7 +224,7 @@ namespace System.Net.Http.QPack } break; case State.CompressedHeaders: - switch (BitOperations.LeadingZeroCount(b) - 24) + switch (BitOperations.LeadingZeroCount(b) - 24) // byte 'b' is extended to uint, so will have 24 extra 0s. { case 0: // Indexed Header Field prefixInt = IndexedHeaderFieldPrefixMask & b; diff --git a/src/libraries/Common/tests/System/Net/Configuration.Certificates.cs b/src/libraries/Common/tests/System/Net/Configuration.Certificates.cs index 1481fd7..02462be 100644 --- a/src/libraries/Common/tests/System/Net/Configuration.Certificates.cs +++ b/src/libraries/Common/tests/System/Net/Configuration.Certificates.cs @@ -4,6 +4,8 @@ using System.Diagnostics; using System.IO; +using System.Runtime.InteropServices; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Threading; using Xunit; @@ -23,6 +25,7 @@ namespace System.Net.Test.Common private static readonly X509Certificate2 s_noEKUCertificate; private static readonly X509Certificate2 s_selfSignedServerCertificate; private static readonly X509Certificate2 s_selfSignedClientCertificate; + private static X509Certificate2 s_selfSigned13ServerCertificate; static Certificates() { @@ -69,6 +72,44 @@ namespace System.Net.Test.Common public static X509Certificate2 GetNoEKUCertificate() => new X509Certificate2(s_noEKUCertificate); public static X509Certificate2 GetSelfSignedServerCertificate() => new X509Certificate2(s_selfSignedServerCertificate); public static X509Certificate2 GetSelfSignedClientCertificate() => new X509Certificate2(s_selfSignedClientCertificate); + + public static X509Certificate2 GetSelfSigned13ServerCertificate() + { + if (s_selfSigned13ServerCertificate == null) + { + X509Certificate2 cert; + + using (ECDsa dsa = ECDsa.Create()) + { + var certReq = new CertificateRequest("CN=testservereku.contoso.com", dsa, HashAlgorithmName.SHA256); + certReq.CertificateExtensions.Add(new X509BasicConstraintsExtension(false, false, 0, false)); + certReq.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(new OidCollection { new Oid("1.3.6.1.5.5.7.3.1") }, false)); + certReq.CertificateExtensions.Add(new X509KeyUsageExtension(X509KeyUsageFlags.DigitalSignature, false)); + + X509Certificate2 innerCert = certReq.CreateSelfSigned(DateTimeOffset.UtcNow.AddMonths(-1), DateTimeOffset.UtcNow.AddMonths(1)); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + using (innerCert) + { + cert = new X509Certificate2(innerCert.Export(X509ContentType.Pfx)); + } + } + else + { + cert = innerCert; + } + } + + if (Interlocked.CompareExchange(ref s_selfSigned13ServerCertificate, cert, null) != null) + { + // Lost a race to create. + cert.Dispose(); + } + } + + return new X509Certificate2(s_selfSigned13ServerCertificate); + } } } } diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index 1b08fca..9e505e3 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -18,10 +18,7 @@ namespace System.Net.Test.Common public abstract GenericLoopbackServer CreateServer(GenericLoopbackOptions options = null); public abstract Task CreateServerAsync(Func funcAsync, int millisecondsTimeout = 60_000, GenericLoopbackOptions options = null); - // TODO: just expose a version property. - public abstract bool IsHttp11 { get; } - public abstract bool IsHttp2 { get; } - public abstract bool IsHttp3 { get; } + public abstract Version Version { get; } // Common helper methods diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index b8815f3..43be924 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -315,7 +315,7 @@ namespace System.Net.Test.Common private static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan headerBlock, byte prefixMask) { - return QPackDecoder.DecodeInteger(headerBlock, prefixMask); + return QPackTestDecoder.DecodeInteger(headerBlock, prefixMask); } private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan headerBlock) diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs index 92bbb6b..f2aea4c 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs @@ -237,9 +237,7 @@ namespace System.Net.Test.Common } } - public override bool IsHttp11 => false; - public override bool IsHttp2 => true; - public override bool IsHttp3 => false; + public override Version Version => HttpVersion.Version20; } public enum ProtocolErrors diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs index a6b87cb..7c89dc9 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs @@ -13,9 +13,27 @@ namespace System.Net.Test.Common { public sealed class Http3LoopbackConnection : GenericLoopbackConnection { + public const long H3_NO_ERROR = 0x100; + public const long H3_GENERAL_PROTOCOL_ERROR = 0x101; + public const long H3_INTERNAL_ERROR = 0x102; + public const long H3_STREAM_CREATION_ERROR = 0x103; + public const long H3_CLOSED_CRITICAL_STREAM = 0x104; + public const long H3_FRAME_UNEXPECTED = 0x105; + public const long H3_FRAME_ERROR = 0x106; + public const long H3_EXCESSIVE_LOAD = 0x107; + public const long H3_ID_ERROR = 0x108; + public const long H3_SETTINGS_ERROR = 0x109; + public const long H3_MISSING_SETTINGS = 0x10a; + public const long H3_REQUEST_REJECTED = 0x10b; + public const long H3_REQUEST_CANCELLED = 0x10c; + public const long H3_REQUEST_INCOMPLETE = 0x10d; + public const long H3_CONNECT_ERROR = 0x10f; + public const long H3_VERSION_FALLBACK = 0x110; + private readonly QuicConnection _connection; private readonly Dictionary _openStreams = new Dictionary(); private Http3LoopbackStream _currentStream; + private bool _closed; public Http3LoopbackConnection(QuicConnection connection) { @@ -29,9 +47,20 @@ namespace System.Net.Test.Common stream.Dispose(); } + if (!_closed) + { + CloseAsync(H3_INTERNAL_ERROR).GetAwaiter().GetResult(); + } + _connection.Dispose(); } + public async Task CloseAsync(long errorCode) + { + await _connection.CloseAsync(errorCode).ConfigureAwait(false); + _closed = true; + } + public Http3LoopbackStream OpenUnidirectionalStream() { return new Http3LoopbackStream(_connection.OpenUnidirectionalStream()); @@ -73,7 +102,14 @@ namespace System.Net.Test.Common public override async Task ReadRequestDataAsync(bool readBody = true) { - Http3LoopbackStream stream = await AcceptStreamAsync().ConfigureAwait(false); + Http3LoopbackStream stream; + + do + { + stream = await AcceptStreamAsync().ConfigureAwait(false); + } + while (!stream.CanWrite); // skip control stream. + return await stream.ReadRequestDataAsync(readBody).ConfigureAwait(false); } @@ -94,7 +130,7 @@ namespace System.Net.Test.Common if (isFinal) { - stream.ShutdownSend(); + await stream.ShutdownSendAsync().ConfigureAwait(false); stream.Dispose(); } } diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs index 7378fad..59d43d4 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs @@ -22,17 +22,18 @@ namespace System.Net.Test.Common { options ??= new GenericLoopbackOptions(); - _cert = Configuration.Certificates.GetServerCertificate(); + _cert = Configuration.Certificates.GetSelfSigned13ServerCertificate(); var sslOpts = new SslServerAuthenticationOptions { EnabledSslProtocols = options.SslProtocols, ApplicationProtocols = new List { SslApplicationProtocol.Http3 }, - ServerCertificate = _cert, + //ServerCertificate = _cert, ClientCertificateRequired = false }; _listener = new QuicListener(new IPEndPoint(options.Address, 0), sslOpts); + _listener.Start(); } public override void Dispose() @@ -55,10 +56,11 @@ namespace System.Net.Test.Common public override async Task HandleRequestAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList headers = null, string content = "") { - using GenericLoopbackConnection con = await EstablishGenericConnectionAsync().ConfigureAwait(false); + using var con = (Http3LoopbackConnection)await EstablishGenericConnectionAsync().ConfigureAwait(false); HttpRequestData request = await con.ReadRequestDataAsync().ConfigureAwait(false); await con.SendResponseAsync(statusCode, headers, content).ConfigureAwait(false); + await con.CloseAsync(Http3LoopbackConnection.H3_NO_ERROR); return request; } } @@ -67,11 +69,7 @@ namespace System.Net.Test.Common { public static Http3LoopbackServerFactory Singleton { get; } = new Http3LoopbackServerFactory(); - public override bool IsHttp11 => false; - - public override bool IsHttp2 => false; - - public override bool IsHttp3 => true; + public override Version Version => HttpVersion.Version30; public override GenericLoopbackServer CreateServer(GenericLoopbackOptions options = null) { diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs index 1f7f80b..5d6812e 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs @@ -61,22 +61,22 @@ namespace System.Net.Test.Common public async Task SendHeadersFrameAsync(ICollection headers) { - int bufferLength = QPackEncoder.MaxPrefixLength; + int bufferLength = QPackTestEncoder.MaxPrefixLength; foreach (HttpHeaderData header in headers) { // Two varints for length, and double the name/value lengths to account for expanding Huffman coding. - bufferLength += QPackEncoder.MaxVarIntLength * 2 + header.Name.Length * 2 + header.Value.Length * 2; + bufferLength += QPackTestEncoder.MaxVarIntLength * 2 + header.Name.Length * 2 + header.Value.Length * 2; } var buffer = new byte[bufferLength]; int bytesWritten = 0; - bytesWritten += QPackEncoder.EncodePrefix(buffer.AsSpan(bytesWritten), 0, 0); + bytesWritten += QPackTestEncoder.EncodePrefix(buffer.AsSpan(bytesWritten), 0, 0); foreach (HttpHeaderData header in headers) { - bytesWritten += QPackEncoder.EncodeHeader(buffer.AsSpan(bytesWritten), header.Name, header.Value, header.HuffmanEncoded ? QPackFlags.HuffmanEncode : QPackFlags.None); + bytesWritten += QPackTestEncoder.EncodeHeader(buffer.AsSpan(bytesWritten), header.Name, header.Value, header.HuffmanEncoded ? QPackFlags.HuffmanEncode : QPackFlags.None); } await SendFrameAsync(HeadersFrame, buffer.AsMemory(0, bytesWritten)).ConfigureAwait(false); @@ -100,9 +100,10 @@ namespace System.Net.Test.Common await _stream.WriteAsync(framePayload).ConfigureAwait(false); } - public void ShutdownSend() + public async Task ShutdownSendAsync() { _stream.Shutdown(); + await _stream.ShutdownWriteCompleted().ConfigureAwait(false); } static int EncodeHttpInteger(long longToEncode, Span buffer) @@ -176,14 +177,14 @@ namespace System.Net.Test.Common { HttpRequestData request = new HttpRequestData { RequestId = Http3LoopbackConnection.GetRequestId(_stream) }; - (int prefixLength, int requiredInsertCount, int deltaBase) = QPackDecoder.DecodePrefix(buffer); + (int prefixLength, int requiredInsertCount, int deltaBase) = QPackTestDecoder.DecodePrefix(buffer); if (requiredInsertCount != 0 || deltaBase != 0) throw new Exception("QPack dynamic table not yet supported."); buffer = buffer.Slice(prefixLength); while (!buffer.IsEmpty) { - (int headerLength, HttpHeaderData header) = QPackDecoder.DecodeHeader(buffer); + (int headerLength, HttpHeaderData header) = QPackTestDecoder.DecodeHeader(buffer); request.Headers.Add(header); buffer = buffer.Slice(headerLength); @@ -247,7 +248,6 @@ namespace System.Net.Test.Common public async Task ReadInteger() { byte[] buffer = new byte[MaximumVarIntBytes]; - int bufferAvailableOffset = -1; int bufferActiveLength = 0; long integerValue; @@ -255,14 +255,14 @@ namespace System.Net.Test.Common do { - bytesRead = await _stream.ReadAsync(buffer.AsMemory(++bufferAvailableOffset, 1)).ConfigureAwait(false); + bytesRead = await _stream.ReadAsync(buffer.AsMemory(bufferActiveLength++, 1)).ConfigureAwait(false); if (bytesRead == 0) { - return bufferActiveLength == 0 ? (long?)null : throw new Exception("Unable to read varint; unexpected end of stream."); + return bufferActiveLength == 1 ? (long?)null : throw new Exception("Unable to read varint; unexpected end of stream."); } Debug.Assert(bytesRead == 1); } - while (!TryDecodeHttpInteger(buffer.AsSpan(0, ++bufferActiveLength), out integerValue, out bytesRead)); + while (!TryDecodeHttpInteger(buffer.AsSpan(0, bufferActiveLength), out integerValue, out bytesRead)); Debug.Assert(bytesRead == bufferActiveLength); diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs index dba6b37..df495ff 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs @@ -33,9 +33,9 @@ namespace System.Net.Http.Functional.Tests [InlineData(true, CancellationMode.Token)] public async Task PostAsync_CancelDuringRequestContentSend_TaskCanceledQuickly(bool chunkedTransfer, CancellationMode mode) { - if (LoopbackServerFactory.IsHttp2 && chunkedTransfer) + if (LoopbackServerFactory.Version >= HttpVersion.Version20 && chunkedTransfer) { - // There is no chunked encoding in HTTP/2 + // There is no chunked encoding in HTTP/2 and later return; } @@ -80,9 +80,9 @@ namespace System.Net.Http.Functional.Tests [MemberData(nameof(OneBoolAndCancellationMode))] public async Task GetAsync_CancelDuringResponseHeadersReceived_TaskCanceledQuickly(bool connectionClose, CancellationMode mode) { - if (LoopbackServerFactory.IsHttp2 && connectionClose) + if (LoopbackServerFactory.Version >= HttpVersion.Version20 && connectionClose) { - // There is no Connection header in HTTP/2 + // There is no Connection header in HTTP/2 and later return; } @@ -130,9 +130,9 @@ namespace System.Net.Http.Functional.Tests [MemberData(nameof(TwoBoolsAndCancellationMode))] public async Task GetAsync_CancelDuringResponseBodyReceived_Buffered_TaskCanceledQuickly(bool chunkedTransfer, bool connectionClose, CancellationMode mode) { - if (LoopbackServerFactory.IsHttp2 && (chunkedTransfer || connectionClose)) + if (LoopbackServerFactory.Version >= HttpVersion.Version20 && (chunkedTransfer || connectionClose)) { - // There is no chunked encoding or connection header in HTTP/2 + // There is no chunked encoding or connection header in HTTP/2 and later return; } @@ -186,9 +186,9 @@ namespace System.Net.Http.Functional.Tests [MemberData(nameof(ThreeBools))] public async Task GetAsync_CancelDuringResponseBodyReceived_Unbuffered_TaskCanceledQuickly(bool chunkedTransfer, bool connectionClose, bool readOrCopyToAsync) { - if (LoopbackServerFactory.IsHttp2 && (chunkedTransfer || connectionClose)) + if (LoopbackServerFactory.Version >= HttpVersion.Version20 && (chunkedTransfer || connectionClose)) { - // There is no chunked encoding or connection header in HTTP/2 + // There is no chunked encoding or connection header in HTTP/2 and later return; } @@ -312,10 +312,10 @@ namespace System.Net.Http.Functional.Tests [ConditionalFact] public async Task MaxConnectionsPerServer_WaitingConnectionsAreCancelable() { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { // HTTP/2 does not use connection limits. - throw new SkipTestException("Not supported on HTTP/2"); + throw new SkipTestException("Not supported on HTTP/2 and later"); } using (HttpClientHandler handler = CreateHttpClientHandler()) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs index 3716fcb..1b085d1 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cookies.cs @@ -182,7 +182,7 @@ namespace System.Net.Http.Functional.Tests private string GetCookieValue(HttpRequestData request) { - if (!LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version < HttpVersion.Version20) { // HTTP/1.x must have only one value. return request.GetSingleHeaderValue("Cookie"); diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 0f2a55a6..7a6e084 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -479,9 +479,9 @@ namespace System.Net.Http.Functional.Tests [MemberData(nameof(SecureAndNonSecure_IPBasedUri_MemberData))] public async Task GetAsync_SecureAndNonSecureIPBasedUri_CorrectlyFormatted(IPAddress address, bool useSsl) { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - throw new SkipTestException("Host header is not supported on HTTP/2."); + throw new SkipTestException("Host header is not supported on HTTP/2 and later."); } var options = new LoopbackServer.Options { Address = address, UseSsl= useSsl }; @@ -871,9 +871,9 @@ namespace System.Net.Http.Functional.Tests Assert.Equal("X-Underscore_Name", requestData.GetSingleHeaderValue("X-Underscore_Name")); Assert.Equal("End", requestData.GetSingleHeaderValue("X-End")); - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - // HTTP/2 forbids certain headers or values. + // HTTP/2 and later forbids certain headers or values. Assert.Equal("trailers", requestData.GetSingleHeaderValue("TE")); Assert.Equal(0, requestData.GetHeaderValueCount("Upgrade")); Assert.Equal(0, requestData.GetHeaderValueCount("Proxy-Connection")); @@ -904,9 +904,9 @@ namespace System.Net.Http.Functional.Tests [MemberData(nameof(GetAsync_ManyDifferentResponseHeaders_ParsedCorrectly_MemberData))] public async Task GetAsync_ManyDifferentResponseHeaders_ParsedCorrectly(string newline, string fold, bool dribble) { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - throw new SkipTestException("Folding is not supported on HTTP/2."); + throw new SkipTestException("Folding is not supported on HTTP/2 and later."); } // Using examples from https://en.wikipedia.org/wiki/List_of_HTTP_header_fields#Response_fields @@ -1035,9 +1035,9 @@ namespace System.Net.Http.Functional.Tests [ConditionalFact] public async Task GetAsync_NonTraditionalChunkSizes_Accepted() { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - throw new SkipTestException("Chunking is not supported on HTTP/2."); + throw new SkipTestException("Chunking is not supported on HTTP/2 and later."); } await LoopbackServer.CreateServerAsync(async (server, url) => { @@ -1259,9 +1259,9 @@ namespace System.Net.Http.Functional.Tests [InlineData(null)] public async Task ReadAsStreamAsync_HandlerProducesWellBehavedResponseStream(bool? chunked) { - if (LoopbackServerFactory.IsHttp2 && chunked == true) + if (LoopbackServerFactory.Version >= HttpVersion.Version20 && chunked == true) { - throw new SkipTestException("Chunking is not supported on HTTP/2."); + throw new SkipTestException("Chunking is not supported on HTTP/2 and later."); } await LoopbackServerFactory.CreateClientAndServerAsync(async uri => @@ -2068,9 +2068,9 @@ namespace System.Net.Http.Functional.Tests return; } - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - throw new SkipTestException("Upgrade is not supported on HTTP/2"); + throw new SkipTestException("Upgrade is not supported on HTTP/2 and later"); } var clientFinished = new TaskCompletionSource(); @@ -2238,9 +2238,9 @@ namespace System.Net.Http.Functional.Tests [InlineData(HttpStatusCode.MethodNotAllowed, "")] public async Task GetAsync_CallMethod_ExpectedStatusLine(HttpStatusCode statusCode, string reasonPhrase) { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { - // Custom messages are not supported on HTTP2. + // Custom messages are not supported on HTTP2 and later. return; } diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs index 0eed7dc..49b2076 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs @@ -34,8 +34,8 @@ namespace System.Net.Http.Functional.Tests protected HttpClient CreateHttpClient(HttpMessageHandler handler) => new HttpClient(handler) { DefaultRequestVersion = UseVersion }; - protected static HttpClient CreateHttpClient(string useHttp2String) => - CreateHttpClient(CreateHttpClientHandler(useHttp2String), useHttp2String); + protected static HttpClient CreateHttpClient(string useVersionString) => + CreateHttpClient(CreateHttpClientHandler(useVersionString), useVersionString); protected static HttpClient CreateHttpClient(HttpMessageHandler handler, string useVersionString) => new HttpClient(handler) { DefaultRequestVersion = Version.Parse(useVersionString) }; diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index bab3d72..6433f53 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -673,7 +673,7 @@ namespace System.Net.Test.Common { if (requestData.GetHeaderValueCount("Content-Length") != 0) { - _contentLength = Int32.Parse(requestData.GetSingleHeaderValue("Content-Length")); + _contentLength = int.Parse(requestData.GetSingleHeaderValue("Content-Length")); } else if (requestData.GetHeaderValueCount("Transfer-Encoding") != 0 && requestData.GetSingleHeaderValue("Transfer-Encoding") == "chunked") { @@ -897,8 +897,6 @@ namespace System.Net.Test.Common return newOptions; } - public override bool IsHttp11 => true; - public override bool IsHttp2 => false; - public override bool IsHttp3 => false; + public override Version Version => HttpVersion.Version11; } } diff --git a/src/libraries/Common/tests/System/Net/Http/QPackDecoder.cs b/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs similarity index 73% rename from src/libraries/Common/tests/System/Net/Http/QPackDecoder.cs rename to src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs index 3923d8d..da276e1 100644 --- a/src/libraries/Common/tests/System/Net/Http/QPackDecoder.cs +++ b/src/libraries/Common/tests/System/Net/Http/QPackTestDecoder.cs @@ -7,7 +7,7 @@ using System.Text; namespace System.Net.Test.Common { - public static class QPackDecoder + public static class QPackTestDecoder { public static (int bytesConsumed, int requiredInsertCount, int deltaBase) DecodePrefix(ReadOnlySpan buffer) { @@ -18,84 +18,63 @@ namespace System.Net.Test.Common return (2, 0, 0); } + public static (int bytesConsumed, HttpHeaderData) DecodeHeader(ReadOnlySpan buffer) { - int firstByte = buffer[0]; - - // Indexed Header Field, dynamic. - if ((firstByte & 0b1100_0000) == 0b1000_0000) - { - throw new Exception("QPack dynamic table is not yet supported."); - } - - // Indexed Header Field, static. - if ((firstByte & 0b1100_0000) == 0b1100_0000) + switch (BitOperations.LeadingZeroCount(buffer[0]) - 24) // byte 'b' is extended to uint, so will have 24 extra 0s. { - (int bytesConsumed, int staticIndex) = DecodeInteger(buffer, 0b1100_0000); + case 0: // Indexed Header Field + { + if ((buffer[0] & 0b0100_0000) == 0) throw new Exception("QPack dynamic table is not yet supported."); - var staticHeader = s_staticTable[staticIndex]; - var header = new HttpHeaderData(staticHeader.Name, staticHeader.Value, raw: buffer.Slice(0, bytesConsumed).ToArray()); + (int bytesConsumed, int staticIndex) = DecodeInteger(buffer, 0b0011_1111); - return (bytesConsumed, header); - } + var staticHeader = s_staticTable[staticIndex]; + var header = new HttpHeaderData(staticHeader.Name, staticHeader.Value, raw: buffer.Slice(0, bytesConsumed).ToArray()); - // Indexed Header Field With Post-Base Index - if ((firstByte & 0b1111_0000) == 0b0001_0000) - { - throw new Exception("QPack dynamic table is not yet supported."); - } - - // Literal Header Field With Name Reference - if ((firstByte & 0b1100_0000) == 0b0100_0000) - { - if ((firstByte & 0b0001_0000) != 0) - { - throw new Exception("QPack dynamic table is not yet supported."); + return (bytesConsumed, header); } + case 1: // Literal Header Field With Name Reference + { + if ((buffer[0] & 0b0001_0000) == 0) throw new Exception("QPack dynamic table is not yet supported."); - (int nameLength, int staticIndex) = DecodeInteger(buffer, 0b1111_0000); - (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b1000_0000); - - int headerLength = nameLength + valueLength; - var header = new HttpHeaderData(s_staticTable[staticIndex].Name, value, raw: buffer.Slice(0, headerLength).ToArray()); - - return (headerLength, header); - } + (int nameLength, int staticIndex) = DecodeInteger(buffer, 0b0000_1111); + (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b0111_1111); - // Literal Header Field With Post-Base Name Reference. - if ((firstByte & 0b1111_0000) == 0b0000_0000) - { - throw new Exception("QPack dynamic table is not yet supported."); - } + int headerLength = nameLength + valueLength; + var header = new HttpHeaderData(s_staticTable[staticIndex].Name, value, raw: buffer.Slice(0, headerLength).ToArray()); - // Literal Header Field Without Name Reference. - if ((firstByte & 0b1110_0000) == 0b0010_0000) - { - (int nameLength, string name) = DecodeString(buffer, 0b1111_1000); - (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b1000_0000); + return (headerLength, header); + } + case 2: // Literal Header Field Without Name Reference + { + (int nameLength, string name) = DecodeString(buffer, 0b0000_0111); + (int valueLength, string value) = DecodeString(buffer.Slice(nameLength), 0b0111_1111); - int headerLength = nameLength + valueLength; - var header = new HttpHeaderData(name, value, raw: buffer.Slice(0, headerLength).ToArray()); + int headerLength = nameLength + valueLength; + var header = new HttpHeaderData(name, value, raw: buffer.Slice(0, headerLength).ToArray()); - return (headerLength, header); + return (headerLength, header); + } + case 3: // Indexed Header Field With Post-Base Index + default: // Literal Header Field With Post-Base Name Reference (at least 4 zeroes, maybe more) + throw new Exception("QPack dynamic table is not yet supported."); } - - throw new Exception("Invalid QPack."); } private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan buffer, byte prefixMask) { - bool huffman = (buffer[0] & (1 << BitOperations.TrailingZeroCount(prefixMask))) != 0; + bool huffman = (buffer[0] & (1 << BitOperations.TrailingZeroCount(~prefixMask))) != 0; if (huffman) { throw new Exception("Huffman coding not yet supported."); } - (int nameLength, int stringLength) = DecodeInteger(buffer, prefixMask); - string value = Encoding.ASCII.GetString(buffer.Slice(nameLength)); + (int varIntLength, int stringLength) = DecodeInteger(buffer, prefixMask); + string value = Encoding.ASCII.GetString(buffer.Slice(varIntLength, stringLength)); - return (nameLength + stringLength, value); + return (varIntLength + stringLength, value); } public static (int bytesConsumed, int value) DecodeInteger(ReadOnlySpan headerBlock, byte prefixMask) @@ -106,13 +85,20 @@ namespace System.Net.Test.Common return (1, value); } - byte b = headerBlock[1]; - if ((b & 0b10000000) != 0) + ulong extra = 0; + int length = 1; + byte b; + + do { - throw new Exception("long integers currently not supported"); + b = headerBlock[length++]; + extra = checked(extra << 7) | b; } + while ((b & 0b10000000) != 0); + + value = checked((int)(prefixMask + extra)); - return (2, prefixMask + b); + return (length, value); } private static readonly HttpHeaderData[] s_staticTable = new HttpHeaderData[] diff --git a/src/libraries/Common/tests/System/Net/Http/QPackEncoder.cs b/src/libraries/Common/tests/System/Net/Http/QPackTestEncoder.cs similarity index 99% rename from src/libraries/Common/tests/System/Net/Http/QPackEncoder.cs rename to src/libraries/Common/tests/System/Net/Http/QPackTestEncoder.cs index f70ca72..a05ce29 100644 --- a/src/libraries/Common/tests/System/Net/Http/QPackEncoder.cs +++ b/src/libraries/Common/tests/System/Net/Http/QPackTestEncoder.cs @@ -7,7 +7,7 @@ using System.Text; namespace System.Net.Test.Common { - public static class QPackEncoder + public static class QPackTestEncoder { public const int MaxVarIntLength = 6; public const int MaxPrefixLength = MaxVarIntLength * 2; diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj index 02a2afb..e31bec6 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj @@ -85,11 +85,11 @@ Common\System\Net\Http\Http2LoopbackConnection.cs - - Common\System\Net\Http\QPackDecoder.cs + + Common\System\Net\Http\QPackTestDecoder.cs - - Common\System\Net\Http\QPackEncoder.cs + + Common\System\Net\Http\QPackTestEncoder.cs Common\System\Net\Http\Http3LoopbackServer.cs diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index afa94e5..4280cee 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -31,9 +31,10 @@ namespace System.Net.Http private readonly HttpAuthority _authority; private readonly byte[] _altUsedEncodedHeader; private QuicConnection _connection; + private Task _connectionClosedTask; // Keep a collection of requests around so we can process GOAWAY. - private readonly Dictionary _activeRequests = new Dictionary(); + private readonly Dictionary _activeRequests = new Dictionary(); // Set when GOAWAY is being processed, when aborting, or when disposing. private long _lastProcessedStreamId = -1; @@ -133,64 +134,115 @@ namespace System.Net.Http if (_connection != null) { - _connection.CloseAsync((long)Http3ErrorCode.NoError).GetAwaiter().GetResult(); // TODO: async... - _connection.Dispose(); + // Close the QuicConnection in the background. + + if (_connectionClosedTask == null) + { + _connectionClosedTask = _connection.CloseAsync((long)Http3ErrorCode.NoError).AsTask(); + } + + QuicConnection connection = _connection; _connection = null; + + _ = _connectionClosedTask.ContinueWith(closeTask => + { + if (closeTask.IsFaulted && NetEventSource.IsEnabled) + { + Trace($"{nameof(QuicConnection)} failed to close: {closeTask.Exception.InnerException}"); + } + + try + { + connection.Dispose(); + } + catch (Exception ex) + { + Trace($"{nameof(QuicConnection)} failed to dispose: {ex}"); + } + }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); } } - public override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + public override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - Task waitTask = WaitForAvailableRequestStreamAsync(cancellationToken); + // Wait for an available stream (based on QUIC MAX_STREAMS) if there isn't one available yet. - return waitTask.IsCompletedSuccessfully - ? SendWithoutWaitingAsync(request, cancellationToken) - : WaitThenSendAsync(waitTask, request, cancellationToken); - } + TaskCompletionSourceWithCancellation waitForAvailableStreamTcs = null; - private async Task WaitThenSendAsync(Task taskToWaitFor, HttpRequestMessage request, CancellationToken cancellationToken) - { - await taskToWaitFor.ConfigureAwait(false); - return await SendWithoutWaitingAsync(request, cancellationToken).ConfigureAwait(false); - } + lock (SyncObj) + { + long remaining = _requestStreamsRemaining; + + if (remaining > 0) + { + _requestStreamsRemaining = remaining - 1; + } + else + { + waitForAvailableStreamTcs = new TaskCompletionSourceWithCancellation(); + _waitingRequests.Enqueue(waitForAvailableStreamTcs); + } + } + + if (waitForAvailableStreamTcs != null) + { + await waitForAvailableStreamTcs.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); + } + + // Allocate an active request - private Task SendWithoutWaitingAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { QuicStream quicStream = null; - Http3RequestStream stream; + Http3RequestStream requestStream = null; try { - // TODO: do less work in this lock, try to get rid of goto. lock (SyncObj) { - if (_connection == null) + if (_connection != null) { - goto retryRequest; + quicStream = _connection.OpenBidirectionalStream(); + requestStream = new Http3RequestStream(request, this, quicStream); + _activeRequests.Add(quicStream, requestStream); } + } - quicStream = _connection.OpenBidirectionalStream(); - if (_lastProcessedStreamId != -1 && quicStream.StreamId > _lastProcessedStreamId) - { - goto retryRequest; - } + if (quicStream == null) + { + throw new HttpRequestException(SR.net_http_request_aborted, null, RequestRetryType.RetryOnSameOrNextProxy); + } + + // 0-byte write to force QUIC to allocate a stream ID. + await quicStream.WriteAsync(Array.Empty(), cancellationToken).ConfigureAwait(false); + requestStream.StreamId = quicStream.StreamId; - stream = new Http3RequestStream(request, this, quicStream); - _activeRequests.Add(quicStream.StreamId, stream); + bool goAway; + lock (SyncObj) + { + goAway = _lastProcessedStreamId != -1 && requestStream.StreamId > _lastProcessedStreamId; + } + + if (goAway) + { + throw new HttpRequestException(SR.net_http_request_aborted, null, RequestRetryType.RetryOnSameOrNextProxy); } - return stream.SendAsync(cancellationToken); + Task responseTask = requestStream.SendAsync(cancellationToken); + + // null out requestStream to avoid disposing in finally block. It is now in charge of disposing itself. + requestStream = null; + + return await responseTask.ConfigureAwait(false); } catch (QuicConnectionAbortedException ex) { // This will happen if we aborted _connection somewhere. Abort(ex); + throw new HttpRequestException(SR.Format(SR.net_http_http3_connection_error, ex.ErrorCode), ex, RequestRetryType.RetryOnSameOrNextProxy); + } + finally + { + requestStream?.Dispose(); } - - retryRequest: - // We lost a race between GOAWAY/abort and our pool sending the request to this connection. - quicStream?.Dispose(); - return Task.FromException(new HttpRequestException(SR.net_http_request_aborted, null, RequestRetryType.RetryOnSameOrNextProxy)); } /// @@ -292,7 +344,10 @@ namespace System.Net.Http } // Abort the connection. This will cause all of our streams to abort on their next I/O. - _connection?.CloseAsync((long)connectionResetErrorCode).GetAwaiter().GetResult(); // TODO: async... + if (_connection != null && _connectionClosedTask == null) + { + _connectionClosedTask = _connection.CloseAsync((long)connectionResetErrorCode).AsTask(); + } CancelWaiters(); CheckForShutdown(); @@ -324,9 +379,9 @@ namespace System.Net.Http _lastProcessedStreamId = lastProcessedStreamId; - foreach (KeyValuePair request in _activeRequests) + foreach (KeyValuePair request in _activeRequests) { - if (request.Key > lastProcessedStreamId) + if (request.Value.StreamId > lastProcessedStreamId) { streamsToGoAway.Add(request.Value); } @@ -343,11 +398,11 @@ namespace System.Net.Http } } - public void RemoveStream(long streamId) + public void RemoveStream(QuicStream stream) { lock (SyncObj) { - bool removed = _activeRequests.Remove(streamId); + bool removed = _activeRequests.Remove(stream); Debug.Assert(removed == true); if (ShuttingDown) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 8420d40..b0bf40d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -21,12 +21,13 @@ namespace System.Net.Http { private readonly HttpRequestMessage _request; private Http3Connection _connection; - private readonly long _streamId; + private long _streamId = -1; // A stream does not have an ID until the first I/O against it. This gets set almost immediately following construction. private QuicStream _stream; private ArrayBuffer _sendBuffer; private readonly ReadOnlyMemory[] _gatheredSendBuffer = new ReadOnlyMemory[2]; private ArrayBuffer _recvBuffer; private TaskCompletionSource _expect100ContinueCompletionSource; // True indicates we should send content (e.g. received 100 Continue). + private bool _disposed; private CancellationTokenSource _goawayCancellationSource; private CancellationToken _goawayCancellationToken; @@ -52,12 +53,17 @@ namespace System.Net.Http // Keep track of how much is remaining in that frame. private long _requestContentLengthRemaining = 0; + public long StreamId + { + get => Volatile.Read(ref _streamId); + set => Volatile.Write(ref _streamId, value); + } + public Http3RequestStream(HttpRequestMessage request, Http3Connection connection, QuicStream stream) { _request = request; _connection = connection; _stream = stream; - _streamId = stream.StreamId; _sendBuffer = new ArrayBuffer(initialSize: 64, usePool: true); _recvBuffer = new ArrayBuffer(initialSize: 64, usePool: true); @@ -70,30 +76,29 @@ namespace System.Net.Http public void Dispose() { - if (_stream != null) + if (!_disposed) { + _disposed = true; _stream.Dispose(); - _stream = null; - DisposeSyncHelper(); } } public async ValueTask DisposeAsync() { - if (_stream != null) + if (!_disposed) { + _disposed = true; await _stream.DisposeAsync().ConfigureAwait(false); - _stream = null; - DisposeSyncHelper(); } } private void DisposeSyncHelper() { - _connection.RemoveStream(_streamId); + _connection.RemoveStream(_stream); _connection = null; + _stream = null; _sendBuffer.Dispose(); _recvBuffer.Dispose(); @@ -780,13 +785,14 @@ namespace System.Net.Http } else { + if (NetEventSource.IsEnabled) Trace($"Server closed response stream before entire header payload could be read. {headersLength:N0} bytes remaining."); throw new HttpRequestException(SR.net_http_invalid_response_premature_eof); } } int processLength = (int)Math.Min(headersLength, _recvBuffer.ActiveLength); - _headerDecoder.Decode(_recvBuffer.ActiveSpan.Slice(processLength), this); + _headerDecoder.Decode(_recvBuffer.ActiveSpan.Slice(0, processLength), this); _recvBuffer.Discard(processLength); headersLength -= processLength; } @@ -1129,7 +1135,7 @@ namespace System.Net.Http } public void Trace(string message, [CallerMemberName] string memberName = null) => - _connection.Trace(_stream.StreamId, message, memberName); + _connection.Trace(StreamId, message, memberName); // TODO: it may be possible for Http3RequestStream to implement Stream directly and avoid this allocation. private sealed class Http3ReadStream : HttpBaseStream @@ -1164,7 +1170,7 @@ namespace System.Net.Http { // We shouldn't be using a managed instance here, but don't have much choice -- we // need to remove the stream from the connection's GOAWAY collection. - _stream._connection.RemoveStream(_stream._streamId); + _stream._connection.RemoveStream(_stream._stream); _stream._connection = null; } @@ -1209,7 +1215,7 @@ namespace System.Net.Http public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) { - throw new NotImplementedException(); + throw new NotSupportedException(); } } @@ -1235,12 +1241,12 @@ namespace System.Net.Http public override int Read(Span buffer) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) { - throw new NotImplementedException(); + throw new NotSupportedException(); } public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 5f85cd2..03eb94a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -35,7 +35,7 @@ namespace System.Net.Http /// The origin authority used to construct the . private readonly HttpAuthority _originAuthority; - /// Initially set identical to (i.e. what came in the request URL), this can be updated based on Alt-Svc. + /// Initially set to null, this can be set to enable HTTP/3 based on Alt-Svc. private volatile HttpAuthority _http3Authority; /// A timer to expire and return the pool to . Initialized on first use. @@ -109,7 +109,11 @@ namespace System.Net.Http if (host != null) { _originAuthority = new HttpAuthority(host, port); - _http3Authority = _originAuthority; + + if (_poolManager.Settings._assumePrenegotiatedHttp3ForTesting) + { + _http3Authority = _originAuthority; + } } _http2Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version20; @@ -626,7 +630,7 @@ namespace System.Net.Http if (http3Connection != null) { TimeSpan pooledConnectionLifetime = _poolManager.Settings._pooledConnectionLifetime; - if (http3Connection.LifetimeExpired(Environment.TickCount64, pooledConnectionLifetime) && http3Connection.Authority == _http3Authority) + if (http3Connection.LifetimeExpired(Environment.TickCount64, pooledConnectionLifetime) || http3Connection.Authority != authority) { // Connection expired. http3Connection.Dispose(); @@ -675,11 +679,14 @@ namespace System.Net.Http QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(authority.IdnHost, authority.Port, _sslOptionsHttp3, cancellationToken).ConfigureAwait(false); + //TODO: NegotiatedApplicationProtocol not yet implemented. +#if false if (quicConnection.NegotiatedApplicationProtocol != SslApplicationProtocol.Http3) { BlacklistAuthority(authority); throw new HttpRequestException("QUIC connected but no HTTP/3 indicated via ALPN.", null, RequestRetryType.RetryOnSameOrNextProxy); } +#endif http3Connection = new Http3Connection(this, _originAuthority, authority, quicConnection); _http3Connection = http3Connection; @@ -790,7 +797,7 @@ namespace System.Net.Http // 'clear' should be the only value present. if (value == AltSvcHeaderValue.Clear) { - _http3Authority = _originAuthority; + ExpireAltSvcAuthority(); _authorityExpireTimer.Change(Timeout.Infinite, Timeout.Infinite); break; } @@ -849,7 +856,7 @@ namespace System.Net.Http var wr = (WeakReference)o; if (wr.TryGetTarget(out HttpConnectionPool @this)) { - @this._http3Authority = @this._originAuthority; + @this.ExpireAltSvcAuthority(); } }, thisRef, nextAuthorityMaxAge, Timeout.InfiniteTimeSpan); } @@ -865,6 +872,15 @@ namespace System.Net.Http } /// + /// Expires the current Alt-Svc authority, resetting the connection back to origin. + /// + private void ExpireAltSvcAuthority() + { + // If we ever support prenegotiated HTTP/3, this should be set to origin, not nulled out. + _http3Authority = null; + } + + /// /// Blacklists an authority and resets the current authority back to origin. /// If the number of blacklisted authorities exceeds , /// Alt-Svc will be disabled entirely for a period of time. @@ -915,7 +931,7 @@ namespace System.Net.Http { if (_http3Authority == badAuthority) { - _http3Authority = _originAuthority; + ExpireAltSvcAuthority(); _authorityExpireTimer.Change(Timeout.Infinite, Timeout.Infinite); } } @@ -946,9 +962,9 @@ namespace System.Net.Http { lock (SyncObj) { - if (_http3Authority != _originAuthority && _persistAuthority == false) + if (_http3Authority != null && _persistAuthority == false) { - _http3Authority = _originAuthority; + ExpireAltSvcAuthority(); _authorityExpireTimer.Change(Timeout.Infinite, Timeout.Infinite); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index fe579ac..3971c9d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -48,6 +48,9 @@ namespace System.Net.Http internal bool _allowUnencryptedHttp2; + // Used for testing until https://github.com/dotnet/runtime/issues/987 + internal bool _assumePrenegotiatedHttp3ForTesting; + internal SslClientAuthenticationOptions _sslOptions; internal IDictionary _properties; @@ -99,6 +102,7 @@ namespace System.Net.Http _useCookies = _useCookies, _useProxy = _useProxy, _allowUnencryptedHttp2 = _allowUnencryptedHttp2, + _assumePrenegotiatedHttp3ForTesting = _assumePrenegotiatedHttp3ForTesting }; } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs index d6d9db0..399a4c9 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs @@ -16,6 +16,17 @@ namespace System.Net.Http.Functional.Tests { } + /// + /// HTTP/3 tests by default use prenegotiated HTTP/3. To test Alt-Svc upgrades, that must be disabled. + /// + protected override HttpClient CreateHttpClient() + { + HttpClientHandler handler = CreateHttpClientHandler(HttpVersion.Version30); + SetUsePrenegotiatedHttp3(handler, usePrenegotiatedHttp3: false); + + return CreateHttpClient(handler); + } + [Fact] public async Task AltSvc_Header_UpgradeFrom11_Success() { @@ -32,7 +43,7 @@ namespace System.Net.Http.Functional.Tests { using GenericLoopbackServer firstServer = GetFactoryForVersion(fromVersion).CreateServer(); using Http3LoopbackServer secondServer = new Http3LoopbackServer(); - using HttpClient client = CreateHttpClient(CreateHttpClientHandler(HttpVersion.Version30)); + using HttpClient client = CreateHttpClient(); Task firstResponseTask = client.GetAsync(firstServer.Address); @@ -52,7 +63,7 @@ namespace System.Net.Http.Functional.Tests { using Http2LoopbackServer firstServer = Http2LoopbackServer.CreateServer(); using Http3LoopbackServer secondServer = new Http3LoopbackServer(); - using HttpClient client = CreateHttpClient(CreateHttpClientHandler(HttpVersion.Version30)); + using HttpClient client = CreateHttpClient(); Task firstResponseTask = client.GetAsync(firstServer.Address); @@ -74,7 +85,7 @@ namespace System.Net.Http.Functional.Tests { using Http2LoopbackServer firstServer = Http2LoopbackServer.CreateServer(); using Http3LoopbackServer secondServer = new Http3LoopbackServer(); - using HttpClient client = CreateHttpClient(CreateHttpClientHandler(HttpVersion.Version30)); + using HttpClient client = CreateHttpClient(); Task firstResponseTask = client.GetAsync(firstServer.Address); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index 25a09bd..b66becd 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -273,10 +273,10 @@ namespace System.Net.Http.Functional.Tests [Fact] public async Task SendAsync_GetWithInvalidHostHeader_ThrowsException() { - if (LoopbackServerFactory.IsHttp2) + if (LoopbackServerFactory.Version >= HttpVersion.Version20) { // Only SocketsHttpHandler with HTTP/1.x uses the Host header to influence the SSL auth. - // Host header is not used for HTTP2. + // Host header is not used for HTTP2 and later. return; } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs index fbe9511..ae7a249 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs @@ -23,9 +23,24 @@ namespace System.Net.Http.Functional.Tests handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; } + if (useVersion == HttpVersion.Version30) + { + SetUsePrenegotiatedHttp3(handler, usePrenegotiatedHttp3: true); + } + return handler; } + /// + /// Used to bypass Alt-Svc until https://github.com/dotnet/runtime/issues/987 + /// + protected static void SetUsePrenegotiatedHttp3(HttpClientHandler handler, bool usePrenegotiatedHttp3) + { + object socketsHttpHandler = GetUnderlyingSocketsHttpHandler(handler); + object settings = socketsHttpHandler.GetType().GetField("_settings", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(socketsHttpHandler); + settings.GetType().GetField("_assumePrenegotiatedHttp3ForTesting", BindingFlags.Instance | BindingFlags.NonPublic).SetValue(settings, usePrenegotiatedHttp3); + } + protected static object GetUnderlyingSocketsHttpHandler(HttpClientHandler handler) { FieldInfo field = typeof(HttpClientHandler).GetField("_socketsHttpHandler", BindingFlags.Instance | BindingFlags.NonPublic); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientMiniStressTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientMiniStressTest.cs index 01495d3..2f28f98 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientMiniStressTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientMiniStressTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Linq; +using System.Net.Quic; using System.Net.Security; using System.Net.Sockets; using System.Net.Test.Common; @@ -30,8 +31,8 @@ namespace System.Net.Http.Functional.Tests } } - // TODO: public to test HTTP/3. - internal sealed class SocketsHttpHandler_HttpClientMiniStress_Http3 : HttpClientMiniStress + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandler_HttpClientMiniStress_Http3 : HttpClientMiniStress { public SocketsHttpHandler_HttpClientMiniStress_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index d82660f..dd68a69 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Linq; +using System.Net.Quic; using System.Net.Security; using System.Net.Sockets; using System.Net.Test.Common; @@ -151,13 +152,6 @@ namespace System.Net.Http.Functional.Tests protected override Version UseVersion => HttpVersion.Version20; } - // TODO: public to test HTTP/3. - internal sealed class SocketsHttpHandler_HttpClientHandler_Finalization_Http3_Test : HttpClientHandler_Finalization_Test - { - public SocketsHttpHandler_HttpClientHandler_Finalization_Http3_Test(ITestOutputHelper output) : base(output) { } - protected override Version UseVersion => HttpVersion.Version30; - } - public sealed class SocketsHttpHandler_HttpClientHandler_MaxConnectionsPerServer_Test : HttpClientHandler_MaxConnectionsPerServer_Test { public SocketsHttpHandler_HttpClientHandler_MaxConnectionsPerServer_Test(ITestOutputHelper output) : base(output) { } @@ -2148,47 +2142,49 @@ namespace System.Net.Http.Functional.Tests protected override Version UseVersion => HttpVersion.Version20; } - // TODO: public to test HTTP/3. - internal sealed class SocketsHttpHandlerTest_Http3 : HttpClientHandlerTest_Http3 + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandler_HttpClientHandler_Finalization_Http3_Test : HttpClientHandler_Finalization_Test + { + public SocketsHttpHandler_HttpClientHandler_Finalization_Http3_Test(ITestOutputHelper output) : base(output) { } + protected override Version UseVersion => HttpVersion.Version30; + } + + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandlerTest_Http3 : HttpClientHandlerTest_Http3 { public SocketsHttpHandlerTest_Http3(ITestOutputHelper output) : base(output) { } } - // TODO: public to test HTTP/3. - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - internal sealed class SocketsHttpHandlerTest_Cookies_Http3 : HttpClientHandlerTest_Cookies + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandlerTest_Cookies_Http3 : HttpClientHandlerTest_Cookies { public SocketsHttpHandlerTest_Cookies_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; } - // TODO: public to test HTTP/3. - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - internal sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Http3 : HttpClientHandlerTest + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Http3 : HttpClientHandlerTest { public SocketsHttpHandlerTest_HttpClientHandlerTest_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; } - // TODO: public to test HTTP/3. - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - internal sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3 : HttpClientHandlerTest_Headers + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3 : HttpClientHandlerTest_Headers { public SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; } - // TODO: public to test HTTP/3. - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - internal sealed class SocketsHttpHandler_HttpClientHandler_Cancellation_Test_Http3 : HttpClientHandler_Cancellation_Test + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandler_HttpClientHandler_Cancellation_Test_Http3 : HttpClientHandler_Cancellation_Test { public SocketsHttpHandler_HttpClientHandler_Cancellation_Test_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; } - // TODO: public to test HTTP/3. - [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] - internal sealed class SocketsHttpHandler_HttpClientHandler_AltSvc_Test_Http3 : HttpClientHandler_AltSvc_Test + [ConditionalClass(typeof(QuicConnection), nameof(QuicConnection.IsQuicSupported))] + public sealed class SocketsHttpHandler_HttpClientHandler_AltSvc_Test_Http3 : HttpClientHandler_AltSvc_Test { public SocketsHttpHandler_HttpClientHandler_AltSvc_Test_Http3(ITestOutputHelper output) : base(output) { } protected override Version UseVersion => HttpVersion.Version30; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj index 6db1c50..8cb7a61 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj @@ -224,11 +224,11 @@ Common\System\Net\Http\HPackEncoder.cs - - Common\System\Net\Http\QPackDecoder.cs + + Common\System\Net\Http\QPackTestDecoder.cs - - Common\System\Net\Http\QPackEncoder.cs + + Common\System\Net\Http\QPackTestEncoder.cs Common\System\Net\Http\Http2LoopbackServer.cs -- 2.7.4