From f655735fd6f08242813f05559ee712ac0740158c Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Thu, 8 Apr 2021 20:08:46 -0700 Subject: [PATCH 1/3] rework HTTP version handling --- .../SocketsHttpHandler/HttpConnectionPool.cs | 345 +++++++++--------- .../SocketsHttpHandler/SocketsHttpHandler.cs | 9 + 2 files changed, 177 insertions(+), 177 deletions(-) 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 83b92308f79ee..91edaa955c382 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 @@ -361,100 +361,35 @@ public byte[] Http2AltSvcOriginUri /// Object used to synchronize access to state in the pool. private object SyncObj => _idleConnections; - private ValueTask GetConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) + private static HttpRequestException GetVersionException(HttpRequestMessage request, int desiredVersion) { - // Do not even attempt at getting/creating a connection if it's already obvious we cannot provided the one requested. - if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - if (request.Version.Major == 3 && !_http3Enabled) - { - return ValueTask.FromException( - new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3))); - } - if (request.Version.Major == 2 && !_http2Enabled) - { - return ValueTask.FromException( - new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2))); - } - } - - // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished - if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) - { - // Either H3 explicitly requested or secured upgraded allowed. - if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) - { - HttpAuthority? authority = _http3Authority; - // H3 is explicitly requested, assume prenegotiated H3. - if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - authority = authority ?? _originAuthority; - } - if (authority != null) - { - if (IsAltSvcBlocked(authority)) - { - return ValueTask.FromException( - new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); - } + Debug.Assert(desiredVersion == 2 || desiredVersion == 3); - return GetHttp3ConnectionAsync(request, authority, cancellationToken); - } - } - } - - // If we got here, we cannot provide HTTP/3 connection. Do not continue if downgrade is not allowed. - if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - return ValueTask.FromException( - new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); - } - - if (_http2Enabled && (request.Version.Major >= 2 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure)) && - // If the connection is not secured and downgrade is possible, prefer HTTP/1.1. - (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower || IsSecure)) - { - return GetHttp2ConnectionAsync(request, async, cancellationToken); - } - // If we got here, we cannot provide HTTP/2 connection. Do not continue if downgrade is not allowed. - if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - return ValueTask.FromException( - new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2))); - } - - return GetHttp11ConnectionAsync(request, async, cancellationToken); + return new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3)); } private ValueTask GetOrReserveHttp11ConnectionAsync(bool async, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } - - List idleConnections = _idleConnections; long nowTicks = Environment.TickCount64; // Look for a usable idle connection. - // If we can't find one, we will either wait for one to become available (if at the connection limit) - // or just increment the connection count and return null so the caller can create a new connection. TaskCompletionSourceWithCancellation waiter; while (true) { HttpConnection connection; lock (SyncObj) { - if (idleConnections.Count > 0) + int idleConnectionCount = _idleConnections.Count; + if (idleConnectionCount > 0) { - // We have a cached connection that we can attempt to use. - // Test it below outside the lock, to avoid doing expensive validation while holding the lock. - connection = idleConnections[idleConnections.Count - 1]._connection; - idleConnections.RemoveAt(idleConnections.Count - 1); + // We have an idle connection that we can attempt to use. + // Validate it below outside the lock, to avoid doing expensive operations while holding the lock. + connection = _idleConnections[idleConnectionCount - 1]._connection; + _idleConnections.RemoveAt(idleConnectionCount - 1); } else { - // No valid cached connections. + // No available idle connections. if (_associatedConnectionCount < _maxConnections) { // We are under the connection limit, so just increment the count and return null @@ -524,28 +459,8 @@ private ValueTask GetConnectionAsync(HttpRequestMessage requ } } - private async ValueTask GetHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) - { - HttpConnection? connection = await GetOrReserveHttp11ConnectionAsync(async, cancellationToken).ConfigureAwait(false); - if (connection is not null) - { - return connection; - } - - if (NetEventSource.Log.IsEnabled()) Trace("Creating new HTTP/1.1 connection for pool."); - - try - { - return await CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); - } - catch - { - DecrementConnectionCount(); - throw; - } - } - - private async ValueTask GetHttp2ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) + // Returns null if HTTP2 cannot be used + private async ValueTask GetHttp2ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { Debug.Assert(_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel || _kind == HttpConnectionKind.Http); @@ -646,7 +561,8 @@ private async ValueTask GetHttp2ConnectionAsync(HttpRequestM if (sslStream != null) { // We established an SSL connection, but the server denied our request for HTTP2. - // Continue as an HTTP/1.1 connection. + // Try to establish a 1.1 connection instead and add it to the pool. + if (NetEventSource.Log.IsEnabled()) { Trace("Server does not support HTTP2; disabling HTTP2 use and proceeding with HTTP/1.1 connection"); @@ -655,34 +571,25 @@ private async ValueTask GetHttp2ConnectionAsync(HttpRequestM bool canUse = true; lock (SyncObj) { + // Server does not support HTTP2. Disable further HTTP2 attempts. _http2Enabled = false; - if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - sslStream.Close(); - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy)); - } - if (_associatedConnectionCount < _maxConnections) { IncrementConnectionCountNoLock(); } else { - // We are in the weird situation of having established a new HTTP 1.1 connection - // when we were already at the maximum for HTTP 1.1 connections. - // Just discard this connection and get another one from the pool. - // This should be a really rare situation to get into, since it would require - // the user to make multiple HTTP 1.1-only requests first before attempting an - // HTTP2 request, and the server failing to accept HTTP2. + // We are already at the limit for HTTP/1.1 connections, so do not proceed with this connection. canUse = false; } } if (canUse) { - return await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false); - } + HttpConnection http11Connection = await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false); + ReturnConnection(http11Connection); + } else { if (NetEventSource.Log.IsEnabled()) @@ -694,8 +601,8 @@ private async ValueTask GetHttp2ConnectionAsync(HttpRequestM } } - // If we reach this point, it means we need to fall back to a (new or existing) HTTP/1.1 connection. - return await GetHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); + // We are unable to use HTTP2. + return null; } private Http2Connection? GetExistingHttp2Connection() @@ -750,7 +657,7 @@ private void AddHttp2Connection(Http2Connection newConnection) [SupportedOSPlatform("windows")] [SupportedOSPlatform("linux")] [SupportedOSPlatform("macos")] - private async ValueTask GetHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) + private async ValueTask GetHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) { Debug.Assert(_kind == HttpConnectionKind.Https); Debug.Assert(_http3Enabled == true); @@ -845,38 +752,156 @@ private async ValueTask GetHttp3ConnectionAsync(HttpRequestM } } - public async ValueTask SendWithRetryAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + // Returns null if HTTP3 cannot be used. + private async ValueTask TrySendUsingHttp3Async(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - int retryCount = 0; - - while (true) + // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished + if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) { - // Loop on connection failures (or other problems like version downgrade) and retry if possible. - - HttpConnectionBase connection = await GetConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); - - HttpResponseMessage response; - - try + if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { - if (connection is HttpConnection) + // Loop in case we get a 421 and need to send the request to a different authority. + while (true) { - ((HttpConnection)connection).Acquire(); - try + HttpAuthority? authority = _http3Authority; + + // If H3 is explicitly requested, assume prenegotiated H3. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - response = await (doRequestAuth && Settings._credentials != null ? - AuthenticationHelper.SendWithNtConnectionAuthAsync(request, async, Settings._credentials, (HttpConnection)connection, this, cancellationToken) : - SendWithNtProxyAuthAsync((HttpConnection)connection, request, async, cancellationToken)).ConfigureAwait(false); + authority = authority ?? _originAuthority; } - finally + + if (authority != null) { - ((HttpConnection)connection).Release(); + if (IsAltSvcBlocked(authority)) + { + throw GetVersionException(request, 3); + } + + Http3Connection connection = await GetHttp3ConnectionAsync(request, authority, cancellationToken).ConfigureAwait(false); + HttpResponseMessage response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); + + // If an Alt-Svc authority returns 421, it means it can't actually handle the request. + // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug. + // In this case, we blocklist the authority and retry the request at the origin. + if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection.Authority != _originAuthority) + { + response.Dispose(); + BlocklistAuthority(connection.Authority); + continue; + } + + return response; } } - else - { - response = await connection!.SendAsync(request, async, cancellationToken).ConfigureAwait(false); - } + } + } + + return null; + } + + // Returns null if HTTP2 cannot be used. + private async ValueTask TrySendUsingHttp2Async(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + { + // Send using HTTP/2 if we can. + if (_http2Enabled && (request.Version.Major >= 2 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure)) && + // If the connection is not secured and downgrade is possible, prefer HTTP/1.1. + (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower || IsSecure)) + { + Http2Connection? connection = await GetHttp2ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); + if (connection is null) + { + Debug.Assert(!_http2Enabled); + return null; + } + + return await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); + } + + return null; + } + + private async ValueTask SendUsingHttp11Async(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + { + HttpConnection? connection = await GetOrReserveHttp11ConnectionAsync(async, cancellationToken).ConfigureAwait(false); + if (connection is null) + { + if (NetEventSource.Log.IsEnabled()) Trace("Creating new HTTP/1.1 connection for pool."); + + try + { + connection = await CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); + } + catch + { + DecrementConnectionCount(); + throw; + } + } + + // In case we are doing Windows (i.e. connection-based) auth, we need to ensure that we hold on to this specific connection while auth is underway. + connection.Acquire(); + try + { + return await SendWithNtConnectionAuthAsync(connection, request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + } + finally + { + connection.Release(); + } + } + + private async ValueTask DetermineVersionAndSendAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + { + HttpResponseMessage? response = await TrySendUsingHttp3Async(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + if (response is not null) + { + return response; + } + + // We cannot use HTTP/3. Do not continue if downgrade is not allowed. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw GetVersionException(request, 3); + } + + response = await TrySendUsingHttp2Async(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + if (response is not null) + { + return response; + } + + // We cannot use HTTP/2. Do not continue if downgrade is not allowed. + if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw GetVersionException(request, 2); + } + + return await SendUsingHttp11Async(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + } + + private async ValueTask SendAndProcessAltSvcAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + { + HttpResponseMessage response = await DetermineVersionAndSendAsync(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + + // Check for the Alt-Svc header, to upgrade to HTTP/3. + if (_altSvcEnabled && response.Headers.TryGetValues(KnownHeaders.AltSvc.Descriptor, out IEnumerable? altSvcHeaderValues)) + { + HandleAltSvc(altSvcHeaderValues, response.Headers.Age); + } + + return response; + } + + public async ValueTask SendWithRetryAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + { + int retryCount = 0; + while (true) + { + // Loop on connection failures (or other problems like version downgrade) and retry if possible. + try + { + return await SendAndProcessAltSvcAsync(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); } catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnConnectionFailure) { @@ -900,7 +925,6 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag } // Eat exception and try again. - continue; } catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnLowerHttpVersion) { @@ -916,9 +940,7 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag } // Eat exception and try again on a lower protocol version. - Debug.Assert(connection is HttpConnection == false, $"{nameof(RequestRetryType.RetryOnLowerHttpVersion)} should not be thrown by HTTP/1 connections."); request.Version = HttpVersion.Version11; - continue; } catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnStreamLimitReached) { @@ -928,30 +950,7 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag } // Eat exception and try again. - continue; } - - // Check for the Alt-Svc header, to upgrade to HTTP/3. - if (_altSvcEnabled && response.Headers.TryGetValues(KnownHeaders.AltSvc.Descriptor, out IEnumerable? altSvcHeaderValues)) - { - HandleAltSvc(altSvcHeaderValues, response.Headers.Age); - } - - // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished - if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) - { - // If an Alt-Svc authority returns 421, it means it can't actually handle the request. - // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug. - // In this case, we blocklist the authority and retry the request at the origin. - if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection is Http3Connection h3Connection && h3Connection.Authority != _originAuthority) - { - response.Dispose(); - BlocklistAuthority(h3Connection.Authority); - continue; - } - } - - return response; } } @@ -1181,22 +1180,14 @@ public void OnNetworkChanged() } } - public async Task SendWithNtConnectionAuthAsync(HttpConnection connection, HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) + public Task SendWithNtConnectionAuthAsync(HttpConnection connection, HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - connection.Acquire(); - try - { - if (doRequestAuth && Settings._credentials != null) - { - return await AuthenticationHelper.SendWithNtConnectionAuthAsync(request, async, Settings._credentials, connection, this, cancellationToken).ConfigureAwait(false); - } - - return await SendWithNtProxyAuthAsync(connection, request, async, cancellationToken).ConfigureAwait(false); - } - finally + if (doRequestAuth && Settings._credentials != null) { - connection.Release(); + return AuthenticationHelper.SendWithNtConnectionAuthAsync(request, async, Settings._credentials, connection, this, cancellationToken); } + + return SendWithNtProxyAuthAsync(connection, request, async, cancellationToken); } private bool DoProxyAuth => (_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index 003a33a8c66c6..8e0b508e6a485 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -511,6 +511,9 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request, } CheckDisposed(); + + cancellationToken.ThrowIfCancellationRequested(); + HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain(); Exception? error = ValidateAndNormalizeRequest(request); @@ -525,6 +528,12 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request, protected internal override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { CheckDisposed(); + + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + HttpMessageHandler handler = _handler ?? SetupHandlerChain(); Exception? error = ValidateAndNormalizeRequest(request); From 04cbddb429b8d0e51eef6d1797271c9b863ba387 Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Tue, 20 Apr 2021 11:23:08 -0700 Subject: [PATCH 2/3] address review feedback --- .../SocketsHttpHandler/HttpConnectionPool.cs | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) 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 91edaa955c382..8f3355c655ad1 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 @@ -365,7 +365,7 @@ private static HttpRequestException GetVersionException(HttpRequestMessage reque { Debug.Assert(desiredVersion == 2 || desiredVersion == 3); - return new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3)); + return new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, desiredVersion)); } private ValueTask GetOrReserveHttp11ConnectionAsync(bool async, CancellationToken cancellationToken) @@ -753,46 +753,46 @@ private async ValueTask GetHttp3ConnectionAsync(HttpRequestMess } // Returns null if HTTP3 cannot be used. + // TODO: SupportedOSPlatform doesn't work for internal APIs https://github.com/dotnet/runtime/issues/51305 + [SupportedOSPlatform("windows")] + [SupportedOSPlatform("linux")] + [SupportedOSPlatform("macos")] private async ValueTask TrySendUsingHttp3Async(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished - if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) + if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { - if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) + // Loop in case we get a 421 and need to send the request to a different authority. + while (true) { - // Loop in case we get a 421 and need to send the request to a different authority. - while (true) + HttpAuthority? authority = _http3Authority; + + // If H3 is explicitly requested, assume prenegotiated H3. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - HttpAuthority? authority = _http3Authority; + authority = authority ?? _originAuthority; + } - // If H3 is explicitly requested, assume prenegotiated H3. - if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + if (authority != null) + { + if (IsAltSvcBlocked(authority)) { - authority = authority ?? _originAuthority; + throw GetVersionException(request, 3); } - if (authority != null) - { - if (IsAltSvcBlocked(authority)) - { - throw GetVersionException(request, 3); - } - - Http3Connection connection = await GetHttp3ConnectionAsync(request, authority, cancellationToken).ConfigureAwait(false); - HttpResponseMessage response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); + Http3Connection connection = await GetHttp3ConnectionAsync(request, authority, cancellationToken).ConfigureAwait(false); + HttpResponseMessage response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); - // If an Alt-Svc authority returns 421, it means it can't actually handle the request. - // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug. - // In this case, we blocklist the authority and retry the request at the origin. - if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection.Authority != _originAuthority) - { - response.Dispose(); - BlocklistAuthority(connection.Authority); - continue; - } - - return response; + // If an Alt-Svc authority returns 421, it means it can't actually handle the request. + // An authority is supposed to be able to handle ALL requests to the origin, so this is a server bug. + // In this case, we blocklist the authority and retry the request at the origin. + if (response.StatusCode == HttpStatusCode.MisdirectedRequest && connection.Authority != _originAuthority) + { + response.Dispose(); + BlocklistAuthority(connection.Authority); + continue; } + + return response; } } } @@ -853,10 +853,16 @@ private async ValueTask SendUsingHttp11Async(HttpRequestMes private async ValueTask DetermineVersionAndSendAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - HttpResponseMessage? response = await TrySendUsingHttp3Async(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); - if (response is not null) + HttpResponseMessage? response; + + // TODO: Replace with Platform-Guard Assertion Annotations once https://github.com/dotnet/runtime/issues/44922 is finished + if ((OperatingSystem.IsLinux() && !OperatingSystem.IsAndroid()) || OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()) { - return response; + response = await TrySendUsingHttp3Async(request, async, doRequestAuth, cancellationToken).ConfigureAwait(false); + if (response is not null) + { + return response; + } } // We cannot use HTTP/3. Do not continue if downgrade is not allowed. From 0b7efdd2ca99288556003a6f95258b195a64879c Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Tue, 20 Apr 2021 18:08:44 -0700 Subject: [PATCH 3/3] fix version error string --- src/libraries/System.Net.Http/src/Resources/Strings.resx | 3 --- .../System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 501437eb2b0e5..a9b2d967aa9f6 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -579,9 +579,6 @@ HTTP request version upgrade is not enabled for synchronous '{0}'. Do not use '{1}' version policy for synchronous HTTP methods. - - Requesting HTTP version {0} with version policy {1} while HTTP/{2} is not enabled. - Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection. 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 8f3355c655ad1..756bcd4710de8 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 @@ -365,7 +365,7 @@ private static HttpRequestException GetVersionException(HttpRequestMessage reque { Debug.Assert(desiredVersion == 2 || desiredVersion == 3); - return new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, desiredVersion)); + return new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, desiredVersion)); } private ValueTask GetOrReserveHttp11ConnectionAsync(bool async, CancellationToken cancellationToken)