From f31c191924da6e607d09c42f949daeffc3a81b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Thu, 9 Jul 2020 14:06:24 +0200 Subject: [PATCH 01/18] VersionPolicy API added. --- .../System.Net.Http/ref/System.Net.Http.cs | 8 ++++++++ .../System.Net.Http/src/System.Net.Http.csproj | 1 + .../src/System/Net/Http/HttpClient.cs | 13 ++++++++++++- .../src/System/Net/Http/HttpRequestMessage.cs | 12 ++++++++++++ .../src/System/Net/Http/HttpUtilities.cs | 2 ++ .../src/System/Net/Http/HttpVersionPolicy.cs | 12 ++++++++++++ 6 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs diff --git a/src/libraries/System.Net.Http/ref/System.Net.Http.cs b/src/libraries/System.Net.Http/ref/System.Net.Http.cs index dc1e77f19d7fb2..565d02a05b95b5 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -46,6 +46,7 @@ public HttpClient(System.Net.Http.HttpMessageHandler handler, bool disposeHandle public static System.Net.IWebProxy DefaultProxy { get { throw null; } set { } } public System.Net.Http.Headers.HttpRequestHeaders DefaultRequestHeaders { get { throw null; } } public System.Version DefaultRequestVersion { get { throw null; } set { } } + public System.Net.Http.HttpVersionPolicy DefaultVersionPolicy { get { throw null; } set { } } public long MaxResponseContentBufferSize { get { throw null; } set { } } public System.TimeSpan Timeout { get { throw null; } set { } } public void CancelPendingRequests() { } @@ -215,6 +216,7 @@ public HttpRequestMessage(System.Net.Http.HttpMethod method, System.Uri? request public System.Collections.Generic.IDictionary Properties { get { throw null; } } public System.Uri? RequestUri { get { throw null; } set { } } public System.Version Version { get { throw null; } set { } } + public System.Net.Http.HttpVersionPolicy VersionPolicy { get { throw null; } set { } } public void Dispose() { } protected virtual void Dispose(bool disposing) { } public override string ToString() { throw null; } @@ -237,6 +239,12 @@ protected virtual void Dispose(bool disposing) { } public System.Net.Http.HttpResponseMessage EnsureSuccessStatusCode() { throw null; } public override string ToString() { throw null; } } + public enum HttpVersionPolicy + { + RequestVersionOrLower = 0, + RequestVersionOrHigher = 1, + RequestVersionExact = 2, + } public abstract partial class MessageProcessingHandler : System.Net.Http.DelegatingHandler { protected MessageProcessingHandler() { } diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index e6c23001804d4c..5a990c4f8c4ed1 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -51,6 +51,7 @@ + diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs index a08edaf6c94824..e8fac70f85ca5d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs @@ -26,6 +26,7 @@ public class HttpClient : HttpMessageInvoker private CancellationTokenSource _pendingRequestsCts; private HttpRequestHeaders? _defaultRequestHeaders; private Version _defaultRequestVersion = HttpUtilities.DefaultRequestVersion; + private HttpVersionPolicy _defaultVersionPolicy = HttpUtilities.DefaultVersionPolicy; private Uri? _baseAddress; private TimeSpan _timeout; @@ -57,6 +58,16 @@ public Version DefaultRequestVersion } } + public HttpVersionPolicy DefaultVersionPolicy + { + get => _defaultVersionPolicy; + set + { + CheckDisposedOrStarted(); + _defaultVersionPolicy = value; + } + } + public Uri? BaseAddress { get { return _baseAddress; } @@ -803,7 +814,7 @@ private static void CheckBaseAddress(Uri? baseAddress, string parameterName) string.IsNullOrEmpty(uri) ? null : new Uri(uri, UriKind.RelativeOrAbsolute); private HttpRequestMessage CreateRequestMessage(HttpMethod method, Uri? uri) => - new HttpRequestMessage(method, uri) { Version = _defaultRequestVersion }; + new HttpRequestMessage(method, uri) { Version = _defaultRequestVersion, VersionPolicy = _defaultVersionPolicy }; #endregion Private Helpers } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs index 3cbb1d058c3987..c920f727742e2e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs @@ -22,6 +22,7 @@ public class HttpRequestMessage : IDisposable private Uri? _requestUri; private HttpRequestHeaders? _headers; private Version _version; + private HttpVersionPolicy _versionPolicy; private HttpContent? _content; private bool _disposed; private IDictionary? _properties; @@ -41,6 +42,17 @@ public Version Version } } + public HttpVersionPolicy VersionPolicy + { + get { return _versionPolicy; } + set + { + CheckDisposed(); + + _versionPolicy = value; + } + } + public HttpContent? Content { get { return _content; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs index 189a25058c82a5..53edcaa3f90192 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpUtilities.cs @@ -13,6 +13,8 @@ internal static class HttpUtilities internal static Version DefaultResponseVersion => HttpVersion.Version11; + internal static HttpVersionPolicy DefaultVersionPolicy => HttpVersionPolicy.RequestVersionOrLower; + internal static bool IsHttpUri(Uri uri) { Debug.Assert(uri != null); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs new file mode 100644 index 00000000000000..e149b55a9ed9a0 --- /dev/null +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Net.Http +{ + public enum HttpVersionPolicy + { + RequestVersionOrLower, + RequestVersionOrHigher, + RequestVersionExact + } +} From 4abfab1f3c89f31a95608c9f5af9e97da948897d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Fri, 10 Jul 2020 23:26:15 +0200 Subject: [PATCH 02/18] WIP Version Policy. --- .../Http/SocketsHttpHandler/ConnectHelper.cs | 2 + .../SocketsHttpHandler/HttpConnectionBase.cs | 10 --- .../SocketsHttpHandler/HttpConnectionPool.cs | 81 ++++++++++++++++--- .../HttpConnectionPoolManager.cs | 2 +- .../HttpConnectionSettings.cs | 32 -------- .../System.Net.Http.Unit.Tests.csproj | 2 + 6 files changed, 76 insertions(+), 53 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index 48d530c2a03e54..832b920ff54dca 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -206,6 +206,8 @@ private static async ValueTask EstablishSslConnectionAsyncCore(bool a throw CancellationHelper.CreateOperationCanceledException(e, cancellationToken); } + // ToDo: let this exception buuble up to the caller, or should we somehow recognize that server didn't allow + // particular HTTP version in ALPN (how? Furtik will know) and use NSE instead. throw new HttpRequestException(SR.net_http_ssl_connection_failed, e); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index b4b0aa19fbad59..ecfb5604dbc724 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs @@ -72,16 +72,6 @@ public bool LifetimeExpired(long nowTicks, TimeSpan lifetime) return expired; } - internal static HttpRequestException CreateRetryException() - { - // This is an exception that's thrown during request processing to indicate that the - // attempt to send the request failed in such a manner that the server is guaranteed to not have - // processed the request in any way, and thus the request can be retried. - // This will be caught in HttpConnectionPool.SendWithRetryAsync and the retry logic will kick in. - // The user should never see this exception. - throw new HttpRequestException(null, null, allowRetry: RequestRetryType.RetryOnSameOrNextProxy); - } - internal static bool IsDigit(byte c) => (uint)(c - '0') <= '9' - '0'; internal static int ParseStatusCode(ReadOnlySpan value) 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 a363430d1bf41c..90ed04881d935c 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 @@ -81,6 +81,8 @@ internal sealed class HttpConnectionPool : IDisposable /// Options specialized and cached for this pool and its key. private readonly SslClientAuthenticationOptions? _sslOptionsHttp11; private readonly SslClientAuthenticationOptions? _sslOptionsHttp2; + // ToDo: can we forgo ALPN completely if we now that downgrade is forbiden by VersionPolicy? + private readonly SslClientAuthenticationOptions? _sslOptionsHttp2Only; private readonly SslClientAuthenticationOptions? _sslOptionsHttp3; /// Queue of waiters waiting for a connection. Created on demand. @@ -115,6 +117,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK { _originAuthority = new HttpAuthority(host, port); + // ToDo: what about this? if (_poolManager.Settings._assumePrenegotiatedHttp3ForTesting) { _http3Authority = _originAuthority; @@ -131,7 +134,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(port != 0); Debug.Assert(sslHostName == null); Debug.Assert(proxyUri == null); - _http2Enabled = _poolManager.Settings._allowUnencryptedHttp2; + _http3Enabled = false; break; @@ -148,7 +151,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(sslHostName == null); Debug.Assert(proxyUri != null); - _http2Enabled = false; + _http2Enabled = false; // ToDo: why? _http3Enabled = false; break; @@ -158,7 +161,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(sslHostName == null); Debug.Assert(proxyUri != null); - _http2Enabled = false; + _http2Enabled = false; // ToDo: why? _http3Enabled = false; break; @@ -167,6 +170,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(port != 0); Debug.Assert(sslHostName != null); Debug.Assert(proxyUri != null); + _http3Enabled = false; // TODO: how do we tunnel HTTP3? break; @@ -220,6 +224,8 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK { _sslOptionsHttp2 = ConstructSslOptions(poolManager, sslHostName); _sslOptionsHttp2.ApplicationProtocols = s_http2ApplicationProtocols; + _sslOptionsHttp2Only = ConstructSslOptions(poolManager, sslHostName); + _sslOptionsHttp2Only.ApplicationProtocols = s_http2OnlyApplicationProtocols; // Note: // The HTTP/2 specification states: @@ -238,7 +244,6 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(hostHeader != null); _http2EncodedAuthorityHostHeader = HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingToAllocatedArray(H2StaticTable.Authority, hostHeader); _http3EncodedAuthorityHostHeader = QPackEncoder.EncodeLiteralHeaderFieldWithStaticNameReferenceToArray(H3StaticTable.Authority, hostHeader); - } if (_http3Enabled) @@ -260,6 +265,9 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK private static readonly List s_http3ApplicationProtocols = new List() { SslApplicationProtocol.Http3 }; private static readonly List s_http2ApplicationProtocols = new List() { SslApplicationProtocol.Http2, SslApplicationProtocol.Http11 }; + // ToDo: can we forgo ALPN completely if we now that downgrade is forbiden by VersionPolicy? + private static readonly List s_http2OnlyApplicationProtocols = new List() { SslApplicationProtocol.Http2 }; + private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName) { Debug.Assert(sslHostName != null); @@ -335,8 +343,22 @@ public byte[] Http2AltSvcOriginUri private ValueTask<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> GetConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { + // 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) + { + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while HTTP/{2} is not enabled. See '{3}' AppContext switch."); + } + if (request.Version.Major >= 2 && !_http2Enabled) + { + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while HTTP/{2} is not enabled. See '{3}' AppContext switch."); + } + } + if (_http3Enabled && request.Version.Major >= 3) { + // ToDo: this depends on _assumePrenegotiatedHttp3ForTesting, why is the H3 connection under this condition? HttpAuthority? authority = _http3Authority; if (authority != null) { @@ -344,11 +366,26 @@ public byte[] Http2AltSvcOriginUri } } - if (_http2Enabled && request.Version.Major >= 2) + // What about retry? Should we move this check into SendWithRetry? + // If we got here, we cannot provide HTTP/3 connection so do not continue to attempt at getting/creating a lowered one. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); + } + + if (_http2Enabled && request.Version.Major >= 2 && + (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel || request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower)) { return GetHttp2ConnectionAsync(request, async, cancellationToken); } + // What about retry? Should we move this check into SendWithRetry? + // If we got here, we cannot provide HTTP/2 connection so do not continue to attempt at getting/creating a lowered one. + if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); + } + return GetHttpConnectionAsync(request, async, cancellationToken); } @@ -547,7 +584,7 @@ public byte[] Http2AltSvcOriginUri Stream? stream; HttpResponseMessage? failureResponse; (socket, stream, transportContext, failureResponse) = - await ConnectAsync(request, async, true, cancellationToken).ConfigureAwait(false); + await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false); if (failureResponse != null) { return (null, true, failureResponse); @@ -613,6 +650,12 @@ public byte[] Http2AltSvcOriginUri { _http2Enabled = false; + if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + // ToDo: Could this even happen if we do not allow HTTP/1.1 in ALPN for different version policies? + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while server returned HTTP/1.1 in ALPN."); + } + if (_associatedConnectionCount < _maxConnections) { IncrementConnectionCountNoLock(); @@ -773,13 +816,18 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag } catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnLowerHttpVersion) { + // Throw NSE, since fallback is not allowed by the version policy. + if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while server offers only version fallback.", e); + } + if (NetEventSource.Log.IsEnabled()) { Trace($"Retrying request after exception on existing connection: {e}"); } // 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; @@ -1082,7 +1130,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool return SendWithProxyAuthAsync(request, async, doRequestAuth, cancellationToken); } - private async ValueTask<(Socket?, Stream?, TransportContext?, HttpResponseMessage?)> ConnectAsync(HttpRequestMessage request, bool async, bool allowHttp2, CancellationToken cancellationToken) + private async ValueTask<(Socket?, Stream?, TransportContext?, HttpResponseMessage?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { // If a non-infinite connect timeout has been set, create and use a new CancellationToken that will be canceled // when either the original token is canceled or a connect timeout occurs. @@ -1128,7 +1176,20 @@ public ValueTask SendAsync(HttpRequestMessage request, bool TransportContext? transportContext = null; if (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel) { - SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(allowHttp2 ? _sslOptionsHttp2! : _sslOptionsHttp11!, request, async, stream!, cancellationToken).ConfigureAwait(false); + // SslOptions based on request version and version policy. + SslClientAuthenticationOptions sslOptions = _sslOptionsHttp11!; + if (_http2Enabled && request.Version.Major >= 2) + { + if (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower) + { + sslOptions = _sslOptionsHttp2!; + } + else + { + sslOptions = _sslOptionsHttp2Only!; + } + } + SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(sslOptions, request, async, stream!, cancellationToken).ConfigureAwait(false); stream = sslStream; transportContext = sslStream.TransportContext; } @@ -1144,7 +1205,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool internal async ValueTask<(HttpConnection?, HttpResponseMessage?)> CreateHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { (Socket? socket, Stream? stream, TransportContext? transportContext, HttpResponseMessage? failureResponse) = - await ConnectAsync(request, async, false, cancellationToken).ConfigureAwait(false); + await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false); if (failureResponse != null) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs index 67906139abd503..1bd6f890614faa 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs @@ -252,7 +252,7 @@ private HttpConnectionKey GetConnectionKey(HttpRequestMessage request, Uri? prox } else { - // No explicit Host header. Use host from uri. + // No explicit Host header. Use host from uri. sslHostName = uri.IdnHost; } } 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 959f82ba83f4b4..d8dc038713a1c7 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 @@ -11,8 +11,6 @@ internal sealed class HttpConnectionSettings { private const string Http2SupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2SUPPORT"; private const string Http2SupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2Support"; - private const string Http2UnencryptedSupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2UNENCRYPTEDSUPPORT"; - private const string Http2UnencryptedSupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport"; private const string Http3DraftSupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP3DRAFTSUPPORT"; private const string Http3DraftSupportAppCtxSettingName = "System.Net.SocketsHttpHandler.Http3DraftSupport"; @@ -45,8 +43,6 @@ internal sealed class HttpConnectionSettings internal Version _maxHttpVersion; - internal bool _allowUnencryptedHttp2; - // Used for testing until https://github.com/dotnet/runtime/issues/987 internal bool _assumePrenegotiatedHttp3ForTesting; @@ -61,7 +57,6 @@ public HttpConnectionSettings() AllowDraftHttp3 && allowHttp2 ? HttpVersion.Version30 : allowHttp2 ? HttpVersion.Version20 : HttpVersion.Version11; - _allowUnencryptedHttp2 = allowHttp2 && AllowUnencryptedHttp2; _defaultCredentialsUsedForProxy = _proxy != null && (_proxy.Credentials == CredentialCache.DefaultCredentials || _defaultProxyCredentials == CredentialCache.DefaultCredentials); _defaultCredentialsUsedForServer = _credentials == CredentialCache.DefaultCredentials; } @@ -100,7 +95,6 @@ public HttpConnectionSettings CloneAndNormalize() _sslOptions = _sslOptions?.ShallowClone(), // shallow clone the options for basic prevention of mutation issues while processing _useCookies = _useCookies, _useProxy = _useProxy, - _allowUnencryptedHttp2 = _allowUnencryptedHttp2, _assumePrenegotiatedHttp3ForTesting = _assumePrenegotiatedHttp3ForTesting }; } @@ -131,32 +125,6 @@ private static bool AllowHttp2 } } - private static bool AllowUnencryptedHttp2 - { - get - { - // Default to not allowing unencrypted HTTP/2, but enable that to be overridden - // by an AppContext switch, or by an environment variable being to to true/1. - - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(Http2UnencryptedSupportAppCtxSettingName, out bool allowHttp2)) - { - return allowHttp2; - } - - // AppContext switch wasn't used. Check the environment variable. - string? envVar = Environment.GetEnvironmentVariable(Http2UnencryptedSupportEnvironmentVariableSettingName); - if (envVar != null && (envVar.Equals("true", StringComparison.OrdinalIgnoreCase) || envVar.Equals("1"))) - { - // Allow HTTP/2.0 protocol for HTTP endpoints. - return true; - } - - // Default to a maximum of HTTP/1.1. - return false; - } - } - private static bool AllowDraftHttp3 { get diff --git a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index 9ca98181f81fbb..da1d8d83804b88 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -212,6 +212,8 @@ Link="ProductionCode\System\Net\Http\HttpRuleParser.cs" /> + Date: Tue, 14 Jul 2020 22:33:53 +0200 Subject: [PATCH 03/18] Cleared up some ToDos. --- .../tests/System/Net/Http/TestHelper.cs | 48 ------------------- .../SocketsHttpHandler/HttpConnectionPool.cs | 24 +++------- .../HttpConnectionSettings.cs | 6 +-- .../HttpClientHandlerTest.AltSvc.cs | 1 - .../HttpClientHandlerTest.Http2.cs | 1 - ...lientHandlerTestBase.SocketsHttpHandler.cs | 16 ------- 6 files changed, 8 insertions(+), 88 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/TestHelper.cs b/src/libraries/Common/tests/System/Net/Http/TestHelper.cs index 9cca3a6466d216..041d6efe1e705f 100644 --- a/src/libraries/Common/tests/System/Net/Http/TestHelper.cs +++ b/src/libraries/Common/tests/System/Net/Http/TestHelper.cs @@ -112,54 +112,6 @@ public static IPAddress GetIPv6LinkLocalAddress() => .Where(a => a.IsIPv6LinkLocal) .FirstOrDefault(); - public static void EnableUnencryptedHttp2IfNecessary(HttpClientHandler handler) - { - if (PlatformDetection.SupportsAlpn && !Capability.Http2ForceUnencryptedLoopback()) - { - return; - } - - FieldInfo socketsHttpHandlerField = typeof(HttpClientHandler).GetField("_underlyingHandler", BindingFlags.NonPublic | BindingFlags.Instance); - if (socketsHttpHandlerField == null) - { - // Not using .NET Core implementation, i.e. could be .NET Framework. - return; - } - - object socketsHttpHandler = socketsHttpHandlerField.GetValue(handler); - Assert.NotNull(socketsHttpHandler); - - EnableUncryptedHttp2(socketsHttpHandler); - } - -#if !NETFRAMEWORK - public static void EnableUnencryptedHttp2IfNecessary(SocketsHttpHandler socketsHttpHandler) - { - if (PlatformDetection.SupportsAlpn && !Capability.Http2ForceUnencryptedLoopback()) - { - return; - } - - EnableUncryptedHttp2(socketsHttpHandler); - } -#endif - - private static void EnableUncryptedHttp2(object socketsHttpHandler) - { - // Get HttpConnectionSettings object from SocketsHttpHandler. - Type socketsHttpHandlerType = typeof(HttpClientHandler).Assembly.GetType("System.Net.Http.SocketsHttpHandler"); - FieldInfo settingsField = socketsHttpHandlerType.GetField("_settings", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(settingsField); - object settings = settingsField.GetValue(socketsHttpHandler); - Assert.NotNull(settings); - - // Allow HTTP/2.0 via unencrypted socket if ALPN is not supported on platform. - Type httpConnectionSettingsType = typeof(HttpClientHandler).Assembly.GetType("System.Net.Http.HttpConnectionSettings"); - FieldInfo allowUnencryptedHttp2Field = httpConnectionSettingsType.GetField("_allowUnencryptedHttp2", BindingFlags.NonPublic | BindingFlags.Instance); - Assert.NotNull(allowUnencryptedHttp2Field); - allowUnencryptedHttp2Field.SetValue(settings, true); - } - public static byte[] GenerateRandomContent(int size) { byte[] data = new byte[size]; 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 90ed04881d935c..e975e0359a611e 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 @@ -81,7 +81,6 @@ internal sealed class HttpConnectionPool : IDisposable /// Options specialized and cached for this pool and its key. private readonly SslClientAuthenticationOptions? _sslOptionsHttp11; private readonly SslClientAuthenticationOptions? _sslOptionsHttp2; - // ToDo: can we forgo ALPN completely if we now that downgrade is forbiden by VersionPolicy? private readonly SslClientAuthenticationOptions? _sslOptionsHttp2Only; private readonly SslClientAuthenticationOptions? _sslOptionsHttp3; @@ -116,12 +115,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK if (host != null) { _originAuthority = new HttpAuthority(host, port); - - // ToDo: what about this? - if (_poolManager.Settings._assumePrenegotiatedHttp3ForTesting) - { - _http3Authority = _originAuthority; - } + _http3Authority = _originAuthority; } _http2Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version20; @@ -151,7 +145,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(sslHostName == null); Debug.Assert(proxyUri != null); - _http2Enabled = false; // ToDo: why? + _http2Enabled = false; _http3Enabled = false; break; @@ -161,7 +155,7 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(sslHostName == null); Debug.Assert(proxyUri != null); - _http2Enabled = false; // ToDo: why? + _http2Enabled = false; _http3Enabled = false; break; @@ -264,8 +258,6 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK private static readonly List s_http3ApplicationProtocols = new List() { SslApplicationProtocol.Http3 }; private static readonly List s_http2ApplicationProtocols = new List() { SslApplicationProtocol.Http2, SslApplicationProtocol.Http11 }; - - // ToDo: can we forgo ALPN completely if we now that downgrade is forbiden by VersionPolicy? private static readonly List s_http2OnlyApplicationProtocols = new List() { SslApplicationProtocol.Http2 }; private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnectionPoolManager poolManager, string sslHostName) @@ -356,9 +348,9 @@ public byte[] Http2AltSvcOriginUri } } - if (_http3Enabled && request.Version.Major >= 3) + // Allow HTTP/3 only when user requests exact version. ALPN is not yet supported by HTTP/3 and implmentation is still experimental. + if (_http3Enabled && request.Version.Major >= 3 && request.VersionPolicy == HttpVersionPolicy.RequestVersionExact) { - // ToDo: this depends on _assumePrenegotiatedHttp3ForTesting, why is the H3 connection under this condition? HttpAuthority? authority = _http3Authority; if (authority != null) { @@ -366,20 +358,18 @@ public byte[] Http2AltSvcOriginUri } } - // What about retry? Should we move this check into SendWithRetry? // If we got here, we cannot provide HTTP/3 connection so do not continue to attempt at getting/creating a lowered one. if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); } - if (_http2Enabled && request.Version.Major >= 2 && - (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel || request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower)) + if (_http2Enabled && + (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { return GetHttp2ConnectionAsync(request, async, cancellationToken); } - // What about retry? Should we move this check into SendWithRetry? // If we got here, we cannot provide HTTP/2 connection so do not continue to attempt at getting/creating a lowered one. if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { 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 d8dc038713a1c7..547711bb288fb7 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 @@ -43,9 +43,6 @@ internal sealed class HttpConnectionSettings internal Version _maxHttpVersion; - // Used for testing until https://github.com/dotnet/runtime/issues/987 - internal bool _assumePrenegotiatedHttp3ForTesting; - internal SslClientAuthenticationOptions? _sslOptions; internal IDictionary? _properties; @@ -94,8 +91,7 @@ public HttpConnectionSettings CloneAndNormalize() _proxy = _proxy, _sslOptions = _sslOptions?.ShallowClone(), // shallow clone the options for basic prevention of mutation issues while processing _useCookies = _useCookies, - _useProxy = _useProxy, - _assumePrenegotiatedHttp3ForTesting = _assumePrenegotiatedHttp3ForTesting + _useProxy = _useProxy }; } 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 b09815bcee6b81..e881013d6d4cb1 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AltSvc.cs @@ -21,7 +21,6 @@ public HttpClientHandler_AltSvc_Test(ITestOutputHelper output) : base(output) protected override HttpClient CreateHttpClient() { HttpClientHandler handler = CreateHttpClientHandler(HttpVersion.Version30); - SetUsePrenegotiatedHttp3(handler, usePrenegotiatedHttp3: false); return CreateHttpClient(handler); } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs index a0cfd9a88d0465..56c97fe0cbdd1e 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -1873,7 +1873,6 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async url => using (var handler = new SocketsHttpHandler()) using (HttpClient client = CreateHttpClient(handler)) { - TestHelper.EnableUnencryptedHttp2IfNecessary(handler); handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; }; // Increase default Expect: 100-continue timeout to ensure that we don't accidentally fire the timer and send the request body. handler.Expect100ContinueTimeout = TimeSpan.FromSeconds(300); 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 90fa26e8fe75e8..d74226f3402f29 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs @@ -18,28 +18,12 @@ protected static HttpClientHandler CreateHttpClientHandler(Version useVersion = if (useVersion >= HttpVersion.Version20) { - TestHelper.EnableUnencryptedHttp2IfNecessary(handler); 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("_underlyingHandler", BindingFlags.Instance | BindingFlags.NonPublic); From 83b88442f85072fedd82df17507ff12db31a85c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 15 Jul 2020 17:08:08 +0200 Subject: [PATCH 04/18] Some more version upgrade/downgrade logic. --- .../src/System/Net/Http/HttpRequestMessage.cs | 1 + .../SocketsHttpHandler/HttpConnectionPool.cs | 34 ++++++++++++------- .../SocketsHttpHandler/SocketsHttpHandler.cs | 3 ++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs index c920f727742e2e..ab545c5d9c1b3d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs @@ -198,6 +198,7 @@ private void InitializeValues(HttpMethod method, Uri? requestUri) _method = method; _requestUri = requestUri; _version = HttpUtilities.DefaultRequestVersion; + _versionPolicy = HttpUtilities.DefaultVersionPolicy; } internal bool MarkAsSent() 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 e975e0359a611e..6eb2914688cbe1 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 @@ -288,7 +288,6 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection public HttpAuthority? OriginAuthority => _originAuthority; public HttpConnectionSettings Settings => _poolManager.Settings; - public bool IsSecure => _sslOptionsHttp11 != null; public HttpConnectionKind Kind => _kind; public bool AnyProxyKind => (_proxyUri != null); public Uri? ProxyUri => _proxyUri; @@ -348,8 +347,7 @@ public byte[] Http2AltSvcOriginUri } } - // Allow HTTP/3 only when user requests exact version. ALPN is not yet supported by HTTP/3 and implmentation is still experimental. - if (_http3Enabled && request.Version.Major >= 3 && request.VersionPolicy == HttpVersionPolicy.RequestVersionExact) + if (_http3Enabled && (request.Version.Major >= 3 || request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { HttpAuthority? authority = _http3Authority; if (authority != null) @@ -357,20 +355,19 @@ public byte[] Http2AltSvcOriginUri return GetHttp3ConnectionAsync(request, authority, cancellationToken); } } - - // If we got here, we cannot provide HTTP/3 connection so do not continue to attempt at getting/creating a lowered one. + // 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) { throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); } - if (_http2Enabled && - (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + if (_http2Enabled && (request.Version.Major >= 2 || request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher) && + // If the connection is not secured and downgrade is possible, prefer HTTP/1.1. + (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower || _kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel)) { return GetHttp2ConnectionAsync(request, async, cancellationToken); } - - // If we got here, we cannot provide HTTP/2 connection so do not continue to attempt at getting/creating a lowered one. + // 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) { throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); @@ -640,9 +637,9 @@ public byte[] Http2AltSvcOriginUri { _http2Enabled = false; - if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - // ToDo: Could this even happen if we do not allow HTTP/1.1 in ALPN for different version policies? + // Could this even happen if we do not allow HTTP/1.1 in ALPN for different version policies? throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while server returned HTTP/1.1 in ALPN."); } @@ -739,7 +736,20 @@ public byte[] Http2AltSvcOriginUri Trace("Attempting new HTTP3 connection."); } - QuicConnection quicConnection = await ConnectHelper.ConnectQuicAsync(authority.IdnHost, authority.Port, _sslOptionsHttp3, cancellationToken).ConfigureAwait(false); + QuicConnection quicConnection; + try + { + quicConnection = await ConnectHelper.ConnectQuicAsync(authority.IdnHost, authority.Port, _sslOptionsHttp3, cancellationToken).ConfigureAwait(false); + } + catch + { + // Disables HTTP/3 until server announces it can handle it via Alt-Svc. + lock (SyncObj) + { + ExpireAltSvcAuthority(); + } + throw; + } //TODO: NegotiatedApplicationProtocol not yet implemented. #if false 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 294b0c1734b76e..3444f3f1e45654 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 @@ -341,6 +341,9 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request, throw new NotSupportedException(SR.Format(SR.net_http_http2_sync_not_supported, GetType())); } + // Do not allow upgrades for synchronous requests, that might lead to asynchronous code-paths. + request.VersionPolicy = HttpVersionPolicy.RequestVersionOrLower; + CheckDisposed(); HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain(); From 04a37a1dd498cee4627614fe05c06acccaa9118b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Fri, 17 Jul 2020 13:11:24 +0200 Subject: [PATCH 05/18] Enforce H2C in tests when ALPN is not available. --- .../HttpClientHandlerTest.Http2.cs | 1 + ...lientHandlerTestBase.SocketsHttpHandler.cs | 34 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs index 56c97fe0cbdd1e..4ef5d34c7d6a83 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -1879,6 +1879,7 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async url => var request = new HttpRequestMessage(HttpMethod.Post, url); request.Version = new Version(2,0); + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; request.Content = new StringContent(new string('*', 3000)); request.Headers.ExpectContinue = true; request.Headers.Add("x-test", "PostAsyncExpect100Continue_NonSuccessResponse_RequestBodyNotSent"); 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 d74226f3402f29..7125aa6b2c4d36 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs @@ -3,6 +3,7 @@ using System.IO; using System.Reflection; +using System.Threading.Tasks; namespace System.Net.Http.Functional.Tests { @@ -14,7 +15,7 @@ protected static HttpClientHandler CreateHttpClientHandler(Version useVersion = { useVersion ??= HttpVersion.Version11; - HttpClientHandler handler = new HttpClientHandler(); + HttpClientHandler handler = PlatformDetection.SupportsAlpn ? new HttpClientHandler() : new VersionHttpClientHandler(useVersion); if (useVersion >= HttpVersion.Version20) { @@ -30,4 +31,35 @@ protected static object GetUnderlyingSocketsHttpHandler(HttpClientHandler handle return field?.GetValue(handler); } } + + internal class VersionHttpClientHandler : HttpClientHandler + { + private readonly Version _useVersion; + + public VersionHttpClientHandler(Version useVersion) + { + _useVersion = useVersion; + } + + protected override HttpResponseMessage Send(HttpRequestMessage request, Threading.CancellationToken cancellationToken) + { + if (request.Version == _useVersion) + { + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + } + + return base.Send(request, cancellationToken); + } + + protected override Task SendAsync(HttpRequestMessage request, Threading.CancellationToken cancellationToken) + { + + if (request.Version == _useVersion) + { + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + } + + return base.SendAsync(request, cancellationToken); + } + } } From 08f085ba744d794bab8ddedff0c745df8c7c79b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Sun, 19 Jul 2020 13:06:46 +0200 Subject: [PATCH 06/18] Exception messages into resources. --- .../src/Resources/Strings.resx | 12 +++++ .../SocketsHttpHandler/HttpConnectionPool.cs | 44 +++++++++---------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 8848ecf6794897..428f1f8d061d90 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -564,4 +564,16 @@ The synchronous method is not supported by '{0}' for HTTP/2 or higher. Either use an asynchronous method or downgrade the request version to HTTP/1.1 or lower. + + 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. + + + Requesting HTTP version {0} with version policy {1} while server returned HTTP/1.1 in ALPN. + + + Requesting HTTP version {0} with version policy {1} while server offers only version fallback. + 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 6eb2914688cbe1..630b25a75917f8 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 @@ -339,11 +339,11 @@ public byte[] Http2AltSvcOriginUri { if (request.Version.Major >= 3 && !_http3Enabled) { - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while HTTP/{2} is not enabled. See '{3}' AppContext switch."); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3)); } if (request.Version.Major >= 2 && !_http2Enabled) { - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while HTTP/{2} is not enabled. See '{3}' AppContext switch."); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2)); } } @@ -358,7 +358,7 @@ public byte[] Http2AltSvcOriginUri // 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) { - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); + throw 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) && @@ -370,7 +370,7 @@ public byte[] Http2AltSvcOriginUri // 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) { - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while unable to establish HTTP/{2} connection."); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2)); } return GetHttpConnectionAsync(request, async, cancellationToken); @@ -639,8 +639,7 @@ public byte[] Http2AltSvcOriginUri if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - // Could this even happen if we do not allow HTTP/1.1 in ALPN for different version policies? - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while server returned HTTP/1.1 in ALPN."); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3)); } if (_associatedConnectionCount < _maxConnections) @@ -816,10 +815,10 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag } catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnLowerHttpVersion) { - // Throw NSE, since fallback is not allowed by the version policy. + // Throw since fallback is not allowed by the version policy. if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - throw new NotSupportedException("ToDo: Message=Requesting HTTP version {0} with version policy {1} while server offers only version fallback.", e); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy), e); } if (NetEventSource.Log.IsEnabled()) @@ -1176,20 +1175,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool TransportContext? transportContext = null; if (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel) { - // SslOptions based on request version and version policy. - SslClientAuthenticationOptions sslOptions = _sslOptionsHttp11!; - if (_http2Enabled && request.Version.Major >= 2) - { - if (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower) - { - sslOptions = _sslOptionsHttp2!; - } - else - { - sslOptions = _sslOptionsHttp2Only!; - } - } - SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(sslOptions, request, async, stream!, cancellationToken).ConfigureAwait(false); + SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(GetSslOptionsForRequest(request), request, async, stream!, cancellationToken).ConfigureAwait(false); stream = sslStream; transportContext = sslStream.TransportContext; } @@ -1215,6 +1201,20 @@ public ValueTask SendAsync(HttpRequestMessage request, bool return (ConstructHttp11Connection(socket, stream!, transportContext), null); } + private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessage request) + { + if (_http2Enabled) + { + if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + return _sslOptionsHttp2Only!; + } + + return _sslOptionsHttp2!; + } + return _sslOptionsHttp11!; + } + private HttpConnection ConstructHttp11Connection(Socket? socket, Stream stream, TransportContext? transportContext) { return _maxConnections == int.MaxValue ? From ce5f21e7da7db2e52dbbacf9d534aea95b000348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Sun, 19 Jul 2020 21:50:10 +0200 Subject: [PATCH 07/18] Some fixes. --- .../tests/System/Net/Http/Http2LoopbackServer.cs | 2 +- .../tests/System/Net/Http/HttpClientHandlerTest.cs | 8 ++++---- .../Common/tests/System/Net/Http/LoopbackServer.cs | 2 ++ .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 11 +++++++++-- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs index a3ff123657a680..5d9f388e8028e0 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs @@ -243,7 +243,7 @@ public override async Task CreateServerAsync(Func HttpVersion20.Value; + public override Version Version => HttpVersion20.Value; } public enum ProtocolErrors diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index e243987445c9f0..2d61e3a3d1ae9c 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -232,7 +232,7 @@ public async Task GetAsync_IPv6LinkLocalAddressUri_Success() using (HttpClient client = CreateHttpClient()) { - var options = new GenericLoopbackOptions { Address = TestHelper.GetIPv6LinkLocalAddress() }; + var options = new GenericLoopbackOptions { Address = TestHelper.GetIPv6LinkLocalAddress(), UseSsl = false }; if (options.Address == null) { throw new SkipTestException("Unable to find valid IPv6 LL address."); @@ -243,7 +243,7 @@ await LoopbackServerFactory.CreateServerAsync(async (server, url) => _output.WriteLine(url.ToString()); await TestHelper.WhenAllCompletedOrAnyFailed( server.AcceptConnectionSendResponseAndCloseAsync(), - client.GetAsync(url)); + client.SendAsync(new HttpRequestMessage(HttpMethod.Get, url) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact })); }, options: options); } } @@ -259,13 +259,13 @@ public async Task GetAsync_IPBasedUri_Success(IPAddress address) using (HttpClient client = CreateHttpClient()) { - var options = new GenericLoopbackOptions { Address = address }; + var options = new GenericLoopbackOptions { Address = address, UseSsl = false }; await LoopbackServerFactory.CreateServerAsync(async (server, url) => { _output.WriteLine(url.ToString()); await TestHelper.WhenAllCompletedOrAnyFailed( server.AcceptConnectionSendResponseAndCloseAsync(), - client.GetAsync(url)); + client.SendAsync(new HttpRequestMessage(HttpMethod.Get, url) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact })); }, options: options); } } diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index 208f9d7f29fa49..839439ee74c35e 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -902,6 +902,8 @@ private static LoopbackServer.Options CreateOptions(GenericLoopbackOptions optio if (options != null) { newOptions.Address = options.Address; + newOptions.UseSsl = options.UseSsl; + newOptions.SslProtocols = options.SslProtocols; } return newOptions; } 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 630b25a75917f8..2fb3a23561acee 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 @@ -639,7 +639,8 @@ public byte[] Http2AltSvcOriginUri if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3)); + sslStream.Close(); + throw new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy)); } if (_associatedConnectionCount < _maxConnections) @@ -1207,11 +1208,17 @@ private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessag { if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { + if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp2Only)}"); return _sslOptionsHttp2Only!; } - return _sslOptionsHttp2!; + if (request.Version.Major >= 2 || request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher) + { + if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp2)}"); + return _sslOptionsHttp2!; + } } + if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp11)}"); return _sslOptionsHttp11!; } From 53a08d1f38b0d54c5131e25057b747735f529ebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 21 Jul 2020 16:56:25 +0200 Subject: [PATCH 08/18] Loopback tests. --- .../tests/FunctionalTests/HttpClientTest.cs | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index 5a23a73ef5f246..162d0b1c3b1a05 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Net.Quic; using System.Net.Sockets; using System.Net.Test.Common; using System.Text; @@ -1001,6 +1002,101 @@ await server.AcceptConnectionAsync(async connection => }); } + public static IEnumerable VersionSelectionMemberData() + { + var serverOptions = new GenericLoopbackOptions(); + // Either we support SSL (ALPN), or we're testing only clear text. + foreach (var useSsl in BoolValues.Where(b => serverOptions.UseSsl || !b)) + { + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; + if (QuicConnection.IsQuicSupported) + { + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version30, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, HttpVersion.Version30 }; + } + + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; + if (QuicConnection.IsQuicSupported) + { + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version30, useSsl, HttpVersion.Version20 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, HttpVersion.Version30 }; + } + + if (QuicConnection.IsQuicSupported) + { + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version30 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, useSsl ? HttpVersion.Version30 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version30, useSsl, useSsl ? (object)HttpVersion.Version30 : typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, useSsl ? (object)HttpVersion.Version30 : typeof(HttpRequestException) }; + } + } + } + + [Theory] + [MemberData(nameof(VersionSelectionMemberData))] + public async Task SendAsync_CorrectVersionSelected(Version requestVersion, HttpVersionPolicy versionPolicy, Version serverVersion, bool useSsl, object expectedResult) + { + // Loopback servers can serve only the exact same version as requested. + if (expectedResult is Version expectedVersion && expectedVersion != serverVersion) + { + return; + } + + LoopbackServerFactory factory = GetFactoryForVersion(serverVersion); + await factory.CreateClientAndServerAsync( + async uri => + { + var request = new HttpRequestMessage(HttpMethod.Get, uri) + { + Version = requestVersion, + VersionPolicy = versionPolicy + }; + + using HttpClientHandler handler = CreateHttpClientHandler(); + if (useSsl) + { + handler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator; + } + using HttpClient client = CreateHttpClient(handler); + if (expectedResult is Type type) + { + Exception exception = await Assert.ThrowsAnyAsync(() => client.SendAsync(request)); + Assert.IsType(type, exception); + _output.WriteLine(exception.ToString()); + } + else + { + HttpResponseMessage response = await client.SendAsync(request); + Assert.Equal(expectedResult, response.Version); + } + }, + async server => + { + try + { + await server.AcceptConnectionSendResponseAndCloseAsync(); + } + catch (Exception) { } // Eat errors from client disconnect. + }, options: new GenericLoopbackOptions(){ UseSsl = useSsl }); + } + [Fact] public void DefaultRequestVersion_InitialValueExpected() { From 959f85f78b69e26b949e7e9d87bc0aec86df281e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 21 Jul 2020 18:07:08 +0200 Subject: [PATCH 09/18] RemoteServer tests and fixes. --- .../SocketsHttpHandler/HttpConnectionPool.cs | 9 ++-- .../tests/FunctionalTests/HttpClientTest.cs | 54 +++++++++++++++++-- 2 files changed, 54 insertions(+), 9 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 2fb3a23561acee..81c937799f5eb3 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 @@ -289,6 +289,7 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection public HttpAuthority? OriginAuthority => _originAuthority; public HttpConnectionSettings Settings => _poolManager.Settings; public HttpConnectionKind Kind => _kind; + public bool IsSecure => _kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel; public bool AnyProxyKind => (_proxyUri != null); public Uri? ProxyUri => _proxyUri; public ICredentials? ProxyCredentials => _poolManager.ProxyCredentials; @@ -347,7 +348,7 @@ public byte[] Http2AltSvcOriginUri } } - if (_http3Enabled && (request.Version.Major >= 3 || request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { HttpAuthority? authority = _http3Authority; if (authority != null) @@ -361,9 +362,9 @@ public byte[] Http2AltSvcOriginUri throw 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) && + 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 || _kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel)) + (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower || IsSecure)) { return GetHttp2ConnectionAsync(request, async, cancellationToken); } @@ -1174,7 +1175,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool Socket? socket = (stream as NetworkStream)?.Socket; TransportContext? transportContext = null; - if (_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel) + if (IsSecure) { SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(GetSslOptionsForRequest(request), request, async, stream!, cancellationToken).ConfigureAwait(false); stream = sslStream; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index 162d0b1c3b1a05..dea786a315fca3 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -14,6 +14,7 @@ using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; +using static System.Net.Test.Common.Configuration.Http; namespace System.Net.Http.Functional.Tests { @@ -1013,12 +1014,12 @@ public static IEnumerable VersionSelectionMemberData() yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; - yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; if (QuicConnection.IsQuicSupported) { yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version30, useSsl, HttpVersion.Version11 }; - yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, HttpVersion.Version30 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, useSsl ? HttpVersion.Version30 : HttpVersion.Version11 }; } yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; @@ -1031,12 +1032,12 @@ public static IEnumerable VersionSelectionMemberData() { yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version30, useSsl, HttpVersion.Version20 }; - yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, HttpVersion.Version30 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version30, useSsl, useSsl ? (object)HttpVersion.Version30 : typeof(HttpRequestException) }; } if (QuicConnection.IsQuicSupported) { - yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, useSsl ? HttpVersion.Version30 : HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; yield return new object[] { HttpVersion.Version30, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version30 : HttpVersion.Version11 }; @@ -1051,11 +1052,12 @@ public static IEnumerable VersionSelectionMemberData() [Theory] [MemberData(nameof(VersionSelectionMemberData))] - public async Task SendAsync_CorrectVersionSelected(Version requestVersion, HttpVersionPolicy versionPolicy, Version serverVersion, bool useSsl, object expectedResult) + public async Task SendAsync_CorrectVersionSelected_LoopbackServer(Version requestVersion, HttpVersionPolicy versionPolicy, Version serverVersion, bool useSsl, object expectedResult) { // Loopback servers can serve only the exact same version as requested. if (expectedResult is Version expectedVersion && expectedVersion != serverVersion) { + _output.WriteLine($"Skipping test: Loopback servers can serve only the exact same version as requested."); return; } @@ -1097,6 +1099,48 @@ await factory.CreateClientAndServerAsync( }, options: new GenericLoopbackOptions(){ UseSsl = useSsl }); } + [OuterLoop("Uses external server")] + [Theory] + [MemberData(nameof(VersionSelectionMemberData))] + public async Task SendAsync_CorrectVersionSelected_ExternalServer(Version requestVersion, HttpVersionPolicy versionPolicy, Version serverVersion, bool useSsl, object expectedResult) + { + RemoteServer remoteServer = null; + if (serverVersion == HttpVersion.Version11) + { + remoteServer = useSsl ? RemoteSecureHttp11Server : RemoteHttp11Server; + } + if (serverVersion == HttpVersion.Version30) + { + remoteServer = useSsl ? RemoteHttp2Server : null; + } + // No remote server that could serve the requested version. + if (remoteServer == null) + { + _output.WriteLine($"Skipping test: No remote server that could serve the requested version."); + return; + } + + + var request = new HttpRequestMessage(HttpMethod.Get, remoteServer.EchoUri) + { + Version = requestVersion, + VersionPolicy = versionPolicy + }; + + using HttpClient client = CreateHttpClient(); + if (expectedResult is Type type) + { + Exception exception = await Assert.ThrowsAnyAsync(() => client.SendAsync(request)); + Assert.IsType(type, exception); + _output.WriteLine(exception.ToString()); + } + else + { + HttpResponseMessage response = await client.SendAsync(request); + Assert.Equal(expectedResult, response.Version); + } + } + [Fact] public void DefaultRequestVersion_InitialValueExpected() { From c0a731f0dafa72ad0dec40905ecb0cea46dbd4be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 21 Jul 2020 20:15:38 +0200 Subject: [PATCH 10/18] H3 assumed prenegotiated only when explicitly requested. --- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 81c937799f5eb3..7ff8e5f0c7d646 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 @@ -115,7 +115,6 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK if (host != null) { _originAuthority = new HttpAuthority(host, port); - _http3Authority = _originAuthority; } _http2Enabled = _poolManager.Settings._maxHttpVersion >= HttpVersion.Version20; @@ -348,9 +347,15 @@ public byte[] Http2AltSvcOriginUri } } + // Either H3 explicitely requested or secured upgraded allowed. if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { HttpAuthority? authority = _http3Authority; + // H3 is explicitely requested, assume prenegotiated H3. + if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + authority = authority ?? _originAuthority; + } if (authority != null) { return GetHttp3ConnectionAsync(request, authority, cancellationToken); From e332222e15a4414a544b1bdab87ad7b29f0e3c64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 21 Jul 2020 22:40:15 +0200 Subject: [PATCH 11/18] Netfx compilaris --- .../System/Net/Http/HttpClientHandlerTest.Decompression.cs | 3 --- .../Common/tests/System/Net/Http/HttpClientHandlerTest.cs | 4 ++-- .../HttpClientHandlerTestBase.WinHttpHandler.cs | 6 ++++++ .../HttpClientHandlerTestBase.SocketsHttpHandler.cs | 7 +++++++ 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs index 7ffddeadb45f00..4877f755162504 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Decompression.cs @@ -64,9 +64,6 @@ public static IEnumerable DecompressedResponse_MethodSpecified_Decompr } } - private HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version) => - new HttpRequestMessage(method, uri) { Version = version }; - [Theory] [MemberData(nameof(DecompressedResponse_MethodSpecified_DecompressedContentReturned_MemberData))] public async Task DecompressedResponse_MethodSpecified_DecompressedContentReturned( diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 2d61e3a3d1ae9c..4ae7dc23a22e03 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -243,7 +243,7 @@ await LoopbackServerFactory.CreateServerAsync(async (server, url) => _output.WriteLine(url.ToString()); await TestHelper.WhenAllCompletedOrAnyFailed( server.AcceptConnectionSendResponseAndCloseAsync(), - client.SendAsync(new HttpRequestMessage(HttpMethod.Get, url) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact })); + client.SendAsync(CreateRequest(HttpMethod.Get, url, UseVersion, true))); }, options: options); } } @@ -265,7 +265,7 @@ await LoopbackServerFactory.CreateServerAsync(async (server, url) => _output.WriteLine(url.ToString()); await TestHelper.WhenAllCompletedOrAnyFailed( server.AcceptConnectionSendResponseAndCloseAsync(), - client.SendAsync(new HttpRequestMessage(HttpMethod.Get, url) { Version = UseVersion, VersionPolicy = HttpVersionPolicy.RequestVersionExact })); + client.SendAsync(CreateRequest(HttpMethod.Get, url, UseVersion, true))); }, options: options); } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs index 179cf30e9105e7..0896bbd7b5b08a 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/HttpClientHandlerTestBase.WinHttpHandler.cs @@ -25,5 +25,11 @@ protected static WinHttpClientHandler CreateHttpClientHandler(Version useVersion return handler; } + + protected static HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version, bool exactVersion = false) => + new HttpRequestMessage(method, uri) + { + Version = version + }; } } 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 7125aa6b2c4d36..0823ce945cb810 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTestBase.SocketsHttpHandler.cs @@ -30,6 +30,13 @@ protected static object GetUnderlyingSocketsHttpHandler(HttpClientHandler handle FieldInfo field = typeof(HttpClientHandler).GetField("_underlyingHandler", BindingFlags.Instance | BindingFlags.NonPublic); return field?.GetValue(handler); } + + protected static HttpRequestMessage CreateRequest(HttpMethod method, Uri uri, Version version, bool exactVersion = false) => + new HttpRequestMessage(method, uri) + { + Version = version, + VersionPolicy = exactVersion ? HttpVersionPolicy.RequestVersionExact : HttpVersionPolicy.RequestVersionOrLower + }; } internal class VersionHttpClientHandler : HttpClientHandler From 24df30f0b04545d45f2d2300bcc248484bef6cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 22 Jul 2020 11:53:37 +0200 Subject: [PATCH 12/18] Typo fixes --- .../System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 4 ++-- .../System.Net.Http/tests/FunctionalTests/HttpClientTest.cs | 2 +- 2 files changed, 3 insertions(+), 3 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 7ff8e5f0c7d646..a299783bbe53d5 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 @@ -347,11 +347,11 @@ public byte[] Http2AltSvcOriginUri } } - // Either H3 explicitely requested or secured upgraded allowed. + // Either H3 explicitly requested or secured upgraded allowed. if (_http3Enabled && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { HttpAuthority? authority = _http3Authority; - // H3 is explicitely requested, assume prenegotiated H3. + // H3 is explicitly requested, assume prenegotiated H3. if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { authority = authority ?? _originAuthority; diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index dea786a315fca3..0b949c4fac8e70 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -1109,7 +1109,7 @@ public async Task SendAsync_CorrectVersionSelected_ExternalServer(Version reques { remoteServer = useSsl ? RemoteSecureHttp11Server : RemoteHttp11Server; } - if (serverVersion == HttpVersion.Version30) + if (serverVersion == HttpVersion.Version20) { remoteServer = useSsl ? RemoteHttp2Server : null; } From d716c5ad38854528bf7db122da49c54fc084a358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 22 Jul 2020 11:57:30 +0200 Subject: [PATCH 13/18] ValueTask.FromException instead of throw --- .../Http/SocketsHttpHandler/HttpConnectionPool.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 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 a299783bbe53d5..6ff817f16d3932 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 @@ -339,11 +339,13 @@ public byte[] Http2AltSvcOriginUri { if (request.Version.Major >= 3 && !_http3Enabled) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3)); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + (new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3))); } if (request.Version.Major >= 2 && !_http2Enabled) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2)); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + (new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2))); } } @@ -364,7 +366,8 @@ public byte[] Http2AltSvcOriginUri // 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) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3)); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + (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)) && @@ -376,7 +379,8 @@ public byte[] Http2AltSvcOriginUri // 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) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2)); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + (new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2))); } return GetHttpConnectionAsync(request, async, cancellationToken); From cf25acbbd54816aab947b2b0f50ced54764f9af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 28 Jul 2020 17:38:50 +0200 Subject: [PATCH 14/18] Blocklisting H3 authority after failed connection attempt. --- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 6 +----- 1 file changed, 1 insertion(+), 5 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 1b925178708717..cd7fc1370430e9 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 @@ -798,10 +798,7 @@ private void AddHttp2Connection(Http2Connection newConnection) catch { // Disables HTTP/3 until server announces it can handle it via Alt-Svc. - lock (SyncObj) - { - ExpireAltSvcAuthority(); - } + BlocklistAuthority(authority); throw; } @@ -1067,7 +1064,6 @@ private void ExpireAltSvcAuthority() internal void BlocklistAuthority(HttpAuthority badAuthority) { Debug.Assert(badAuthority != null); - Debug.Assert(badAuthority != _originAuthority); HashSet? altSvcBlocklist = _altSvcBlocklist; From ea57adf6c3985264e0dfa63c636af33852c0b6b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 28 Jul 2020 17:45:28 +0200 Subject: [PATCH 15/18] Fixed merge --- .../System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 5 +---- .../Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs | 2 +- 2 files changed, 2 insertions(+), 5 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 cd7fc1370430e9..2be39bcd73a524 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 @@ -1236,7 +1236,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool TransportContext? transportContext = null; if (IsSecure) { - SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(GetSslOptionsForRequest(request), request, async, stream!, cancellationToken).ConfigureAwait(false); + SslStream sslStream = await ConnectHelper.EstablishSslConnectionAsync(GetSslOptionsForRequest(request), request, async, connection.Stream, cancellationToken).ConfigureAwait(false); connection = Connection.FromStream(sslStream, leaveOpen: false, connection.ConnectionProperties, connection.LocalEndPoint, connection.RemoteEndPoint); transportContext = sslStream.TransportContext; } @@ -1296,17 +1296,14 @@ private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessag { if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp2Only)}"); return _sslOptionsHttp2Only!; } if (request.Version.Major >= 2 || request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher) { - if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp2)}"); return _sslOptionsHttp2!; } } - if (NetEventSource.Log.IsEnabled()) Trace($"GetSslOptionsForRequest({request.Version}, {request.VersionPolicy}) --> {nameof(_sslOptionsHttp11)}"); return _sslOptionsHttp11!; } 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 432d667e72a393..fc01edc786e9ce 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 @@ -99,7 +99,7 @@ public HttpConnectionSettings CloneAndNormalize() _proxy = _proxy, _sslOptions = _sslOptions?.ShallowClone(), // shallow clone the options for basic prevention of mutation issues while processing _useCookies = _useCookies, - _useProxy = _useProxy + _useProxy = _useProxy, _enableMultipleHttp2Connections = _enableMultipleHttp2Connections, _connectionFactory = _connectionFactory, _plaintextFilter = _plaintextFilter From 88343a185980d5b9162df9be24f34b83ee7620b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 28 Jul 2020 22:24:46 +0200 Subject: [PATCH 16/18] H3 blocked alt-svc authority. --- .../SocketsHttpHandler/HttpConnectionPool.cs | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 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 2be39bcd73a524..b238440e60d96c 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 @@ -364,6 +364,12 @@ public byte[] Http2AltSvcOriginUri } if (authority != null) { + if (IsAltSvcBlocked(authority)) + { + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + (new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); + } + return GetHttp3ConnectionAsync(request, authority, cancellationToken); } } @@ -955,16 +961,10 @@ internal void HandleAltSvc(IEnumerable altSvcHeaderValues, TimeSpan? res { var authority = new HttpAuthority(value.Host!, value.Port); - if (_altSvcBlocklist != null) + if (IsAltSvcBlocked(authority)) { - lock (_altSvcBlocklist) - { - if (_altSvcBlocklist.Contains(authority)) - { - // Skip authorities in our blocklist. - continue; - } - } + // Skip authorities in our blocklist. + continue; } TimeSpan authorityMaxAge = value.MaxAge; @@ -1048,6 +1048,22 @@ private void ExpireAltSvcAuthority() _http3Authority = null; } + /// + /// Checks whether the given is on the currext Alt-Svc blocklist. + /// + /// + private bool IsAltSvcBlocked(HttpAuthority authority) + { + if (_altSvcBlocklist != null) + { + lock (_altSvcBlocklist) + { + return _altSvcBlocklist.Contains(authority); + } + } + return false; + } + /// /// Blocklists an authority and resets the current authority back to origin. /// If the number of blocklisted authorities exceeds , From 2ed24f540eec49904188d9fd16dc4c8a19eb55bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Wed, 5 Aug 2020 13:05:58 +0200 Subject: [PATCH 17/18] Adapted version selection tests to HttpAgnosticLoopbackServer --- .../Net/Http/HttpAgnosticLoopbackServer.cs | 130 +++++++++++------- .../System/Net/Http/HttpClientHandlerTest.cs | 3 + .../tests/FunctionalTests/HttpClientTest.cs | 35 ++--- 3 files changed, 102 insertions(+), 66 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs index 6a6657b3adb025..7e4fe975440aef 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs @@ -54,77 +54,104 @@ public override void Dispose() _listenSocket = null; } } + public override async Task EstablishGenericConnectionAsync() { Socket socket = await _listenSocket.AcceptAsync().ConfigureAwait(false); Stream stream = new NetworkStream(socket, ownsSocket: true); - if (_options.UseSsl) + var options = new GenericLoopbackOptions() { - var sslStream = new SslStream(stream, false, delegate { return true; }); - - using (X509Certificate2 cert = Configuration.Certificates.GetServerCertificate()) - { - SslServerAuthenticationOptions options = new SslServerAuthenticationOptions(); - - options.EnabledSslProtocols = _options.SslProtocols; - - var protocols = new List(); - protocols.Add(SslApplicationProtocol.Http11); - protocols.Add(SslApplicationProtocol.Http2); - options.ApplicationProtocols = protocols; + Address = _options.Address, + SslProtocols = _options.SslProtocols, + UseSsl = false, + ListenBacklog = _options.ListenBacklog + }; - options.ServerCertificate = cert; + GenericLoopbackConnection connection = null; - await sslStream.AuthenticateAsServerAsync(options, CancellationToken.None).ConfigureAwait(false); - } - - stream = sslStream; - if (sslStream.NegotiatedApplicationProtocol == SslApplicationProtocol.Http2) + try + { + if (_options.UseSsl) { - // Do not pass original options so the CreateConnectionAsync won't try to do ALPN again. - return await Http2LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream); + var sslStream = new SslStream(stream, false, delegate { return true; }); + + using (X509Certificate2 cert = Configuration.Certificates.GetServerCertificate()) + { + SslServerAuthenticationOptions sslOptions = new SslServerAuthenticationOptions(); + + sslOptions.EnabledSslProtocols = _options.SslProtocols; + sslOptions.ApplicationProtocols = _options.SslApplicationProtocols; + sslOptions.ServerCertificate = cert; + + await sslStream.AuthenticateAsServerAsync(sslOptions, CancellationToken.None).ConfigureAwait(false); + } + + stream = sslStream; + if (sslStream.NegotiatedApplicationProtocol == SslApplicationProtocol.Http2) + { + // Do not pass original options so the CreateConnectionAsync won't try to do ALPN again. + return connection = await Http2LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream, options).ConfigureAwait(false); + } + if (sslStream.NegotiatedApplicationProtocol == SslApplicationProtocol.Http11 || + sslStream.NegotiatedApplicationProtocol == default) + { + // Do not pass original options so the CreateConnectionAsync won't try to do ALPN again. + return connection = await Http11LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream, options).ConfigureAwait(false); + } + else + { + throw new Exception($"Unsupported negotiated protocol {sslStream.NegotiatedApplicationProtocol}"); + } } - if (sslStream.NegotiatedApplicationProtocol == SslApplicationProtocol.Http11) + + var buffer = new byte[24]; + var position = 0; + while (position < buffer.Length) { - // Do not pass original options so the CreateConnectionAsync won't try to do ALPN again. - return await Http11LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream); + var readBytes = await stream.ReadAsync(buffer, position, buffer.Length - position).ConfigureAwait(false); + if (readBytes == 0) + { + break; + } + position += readBytes; } - throw new Exception($"Unsupported negotiated protocol {sslStream.NegotiatedApplicationProtocol}"); - } + + var memory = new Memory(buffer, 0, position); + stream = new ReturnBufferStream(stream, memory); - var buffer = new byte[24]; - var position = 0; - while (position < buffer.Length) - { - var readBytes = await stream.ReadAsync(buffer, position, buffer.Length - position).ConfigureAwait(false); - if (readBytes == 0) + var prefix = Text.Encoding.ASCII.GetString(memory.Span); + if (prefix == Http2LoopbackConnection.Http2Prefix) { - break; + if (_options.ClearTextVersion == HttpVersion.Version20 || _options.ClearTextVersion == HttpVersion.Unknown) + { + return connection = await Http2LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream, options).ConfigureAwait(false); + } } - position += readBytes; - } - - var memory = new Memory(buffer, 0, position); - stream = new ReturnBufferStream(stream, memory); - - var prefix = Text.Encoding.ASCII.GetString(memory.Span); - if (prefix == Http2LoopbackConnection.Http2Prefix) - { - if (_options.ClearTextVersion == HttpVersion.Version20 || _options.ClearTextVersion == HttpVersion.Unknown) + else { - return await Http2LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream); + if (_options.ClearTextVersion == HttpVersion.Version11 || _options.ClearTextVersion == HttpVersion.Unknown) + { + return connection = await Http11LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream, options).ConfigureAwait(false); + } } + + throw new Exception($"HTTP/{_options.ClearTextVersion} server cannot establish connection due to unexpected data: '{prefix}'"); + } + catch + { + connection?.Dispose(); + connection = null; + stream.Dispose(); + throw; } - else + finally { - if (_options.ClearTextVersion == HttpVersion.Version11 || _options.ClearTextVersion == HttpVersion.Unknown) + if (connection != null) { - return await Http11LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream); + await connection.InitializeConnectionAsync().ConfigureAwait(false); } } - - throw new Exception($"HTTP/{_options.ClearTextVersion} server cannot establish connection due to unexpected data: '{prefix}'"); } public override async Task HandleRequestAsync(HttpStatusCode statusCode = HttpStatusCode.OK, IList headers = null, string content = "") @@ -163,6 +190,7 @@ public static async Task CreateClientAndServerAsync(Func clientFunc, public class HttpAgnosticOptions : GenericLoopbackOptions { public Version ClearTextVersion { get; set; } + public List SslApplicationProtocols { get; set; } public HttpAgnosticOptions() { @@ -259,5 +287,9 @@ public override int Read(byte[] buffer, int offset, int count) public override void SetLength(long value) => _stream.SetLength(value); public override void Write(byte[] buffer, int offset, int count) => _stream.Write(buffer, offset, count); + protected override void Dispose(bool disposing) + { + _stream.Dispose(); + } } } diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index d3d94673d70cb3..e889a86a8ea243 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -236,6 +236,7 @@ public async Task GetAsync_IPv6LinkLocalAddressUri_Success() using HttpClient client = CreateHttpClient(handler); + var options = new GenericLoopbackOptions { Address = TestHelper.GetIPv6LinkLocalAddress() }; if (options.Address == null) { throw new SkipTestException("Unable to find valid IPv6 LL address."); @@ -264,6 +265,8 @@ public async Task GetAsync_IPBasedUri_Success(IPAddress address) using HttpClient client = CreateHttpClient(handler); + var options = new GenericLoopbackOptions { Address = address }; + await LoopbackServerFactory.CreateServerAsync(async (server, url) => { _output.WriteLine(url.ToString()); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index 406ec6f5591f38..b41eec644a9ff2 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -7,6 +7,7 @@ using System.IO; using System.Linq; using System.Net.Quic; +using System.Net.Security; using System.Net.Sockets; using System.Net.Test.Common; using System.Text; @@ -903,7 +904,7 @@ await LoopbackServer.CreateClientAndServerAsync( Assert.IsType(ex.InnerException); }, async server => - { + { await server.AcceptConnectionAsync(async connection => { try @@ -1013,9 +1014,9 @@ public static IEnumerable VersionSelectionMemberData() yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; - yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; - yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, HttpVersion.Version11 }; - yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? (object)HttpVersion.Version11 : typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, useSsl ? (object)HttpVersion.Version11 : typeof(HttpRequestException) }; + yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, useSsl ? (object)HttpVersion.Version20 : typeof(HttpRequestException) }; if (QuicConnection.IsQuicSupported) { yield return new object[] { HttpVersion.Version11, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version30, useSsl, HttpVersion.Version11 }; @@ -1026,7 +1027,7 @@ public static IEnumerable VersionSelectionMemberData() yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version11, useSsl, HttpVersion.Version11 }; yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version11, useSsl, typeof(HttpRequestException) }; - yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? HttpVersion.Version20 : HttpVersion.Version11 }; + yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrLower, HttpVersion.Version20, useSsl, useSsl ? (object)HttpVersion.Version20 : typeof(HttpRequestException) }; yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionExact, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; yield return new object[] { HttpVersion.Version20, HttpVersionPolicy.RequestVersionOrHigher, HttpVersion.Version20, useSsl, HttpVersion.Version20 }; if (QuicConnection.IsQuicSupported) @@ -1055,15 +1056,7 @@ public static IEnumerable VersionSelectionMemberData() [MemberData(nameof(VersionSelectionMemberData))] public async Task SendAsync_CorrectVersionSelected_LoopbackServer(Version requestVersion, HttpVersionPolicy versionPolicy, Version serverVersion, bool useSsl, object expectedResult) { - // Loopback servers can serve only the exact same version as requested. - if (expectedResult is Version expectedVersion && expectedVersion != serverVersion) - { - _output.WriteLine($"Skipping test: Loopback servers can serve only the exact same version as requested."); - return; - } - - LoopbackServerFactory factory = GetFactoryForVersion(serverVersion); - await factory.CreateClientAndServerAsync( + await HttpAgnosticLoopbackServer.CreateClientAndServerAsync( async uri => { var request = new HttpRequestMessage(HttpMethod.Get, uri) @@ -1082,7 +1075,7 @@ await factory.CreateClientAndServerAsync( { Exception exception = await Assert.ThrowsAnyAsync(() => client.SendAsync(request)); Assert.IsType(type, exception); - _output.WriteLine(exception.ToString()); + _output.WriteLine("Client expected exception: " + exception.ToString()); } else { @@ -1096,8 +1089,16 @@ await factory.CreateClientAndServerAsync( { await server.AcceptConnectionSendResponseAndCloseAsync(); } - catch (Exception) { } // Eat errors from client disconnect. - }, options: new GenericLoopbackOptions(){ UseSsl = useSsl }); + catch (Exception ex) + { + _output.WriteLine("Server exception: " + ex.ToString()); + } + }, httpOptions: new HttpAgnosticOptions() + { + UseSsl = useSsl, + ClearTextVersion = serverVersion, + SslApplicationProtocols = serverVersion.Major >= 2 ? new List{ SslApplicationProtocol.Http2, SslApplicationProtocol.Http11 } : null + }); } [OuterLoop("Uses external server")] From 63f7b059d915919b50b4e92c4a10c64b29a95928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Mon, 10 Aug 2020 17:44:29 +0200 Subject: [PATCH 18/18] Addressed PR feedback. --- .../System/Net/Http/GenericLoopbackServer.cs | 2 ++ .../Net/Http/Http2LoopbackConnection.cs | 1 + .../System/Net/Http/Http3LoopbackServer.cs | 5 +++ .../System/Net/Http/Http3LoopbackStream.cs | 1 + .../Net/Http/HttpAgnosticLoopbackServer.cs | 14 +++++---- .../tests/System/Net/Http/LoopbackServer.cs | 1 + .../src/Resources/Strings.resx | 3 ++ .../src/System/Net/Http/HttpClient.cs | 8 +++++ .../src/System/Net/Http/HttpRequestMessage.cs | 3 ++ .../src/System/Net/Http/HttpVersionPolicy.cs | 31 +++++++++++++++++++ .../Http/SocketsHttpHandler/ConnectHelper.cs | 2 -- .../SocketsHttpHandler/HttpConnectionPool.cs | 24 +++++++------- .../SocketsHttpHandler/SocketsHttpHandler.cs | 5 ++- .../tests/FunctionalTests/HttpClientTest.cs | 5 +-- 14 files changed, 82 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs index da6e915a237fcc..f4f92c0791c8f5 100644 --- a/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -130,6 +130,7 @@ public class HttpRequestData public byte[] Body; public string Method; public string Path; + public Version Version; public List Headers { get; } public int RequestId; // Generic request ID. Currently only used for HTTP/2 to hold StreamId. @@ -143,6 +144,7 @@ public static async Task FromHttpRequestMessageAsync(System.Net var result = new HttpRequestData(); result.Method = request.Method.ToString(); result.Path = request.RequestUri?.AbsolutePath; + result.Version = request.Version; foreach (var header in request.Headers) { diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index 4701cecd248fba..80705c4dd78777 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -566,6 +566,7 @@ public async Task ReadBodyAsync(bool expectEndOfStream = false) // Extract method and path requestData.Method = requestData.GetSingleHeaderValue(":method"); requestData.Path = requestData.GetSingleHeaderValue(":path"); + requestData.Version = HttpVersion20.Value; if (readBody && (frame.Flags & FrameFlags.EndStream) == 0) { diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs index b290c5a074ab88..06f8c4633268c0 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs @@ -86,4 +86,9 @@ public override Task CreateConnectionAsync(Socket soc throw new NotImplementedException("HTTP/3 does not operate over a Socket."); } } + + public static class HttpVersion30 + { + public static readonly Version Value = new Version(3, 0); + } } diff --git a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs index d0e8d433ec7098..3286aff03a282e 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs @@ -201,6 +201,7 @@ private HttpRequestData ParseHeaders(ReadOnlySpan buffer) break; } } + request.Version = HttpVersion30.Value; return request; } diff --git a/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs index 7e4fe975440aef..7e0f4e43838b5b 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpAgnosticLoopbackServer.cs @@ -105,6 +105,11 @@ public override async Task EstablishGenericConnection } } + if (_options.ClearTextVersion is null) + { + throw new Exception($"HTTP server does not accept clear text connections, either set '{nameof(HttpAgnosticOptions.UseSsl)}' or set up '{nameof(HttpAgnosticOptions.ClearTextVersion)}' in server options."); + } + var buffer = new byte[24]; var position = 0; while (position < buffer.Length) @@ -135,7 +140,7 @@ public override async Task EstablishGenericConnection return connection = await Http11LoopbackServerFactory.Singleton.CreateConnectionAsync(socket, stream, options).ConfigureAwait(false); } } - + throw new Exception($"HTTP/{_options.ClearTextVersion} server cannot establish connection due to unexpected data: '{prefix}'"); } catch @@ -189,13 +194,10 @@ public static async Task CreateClientAndServerAsync(Func clientFunc, public class HttpAgnosticOptions : GenericLoopbackOptions { + // Default null will raise an exception for any clear text protocol version + // Use HttpVersion.Unknown to use protocol version detection for clear text. public Version ClearTextVersion { get; set; } public List SslApplicationProtocols { get; set; } - - public HttpAgnosticOptions() - { - ClearTextVersion = HttpVersion.Version11; - } } public sealed class HttpAgnosticLoopbackServerFactory : LoopbackServerFactory diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs index 2c027ca702cd41..bff8be28d144c5 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackServer.cs @@ -717,6 +717,7 @@ public override async Task ReadRequestDataAsync(bool readBody = string[] splits = Encoding.ASCII.GetString(headerLines[0]).Split(' '); requestData.Method = splits[0]; requestData.Path = splits[1]; + requestData.Version = Version.Parse(splits[2].Substring(splits[2].IndexOf('/') + 1)); // Convert header lines to key/value pairs // Skip first line since it's the status line diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 7520e6f60bc098..61b45be26fdd93 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -567,6 +567,9 @@ The synchronous method is not supported by '{0}' for HTTP/2 or higher. Either use an asynchronous method or downgrade the request version to HTTP/1.1 or lower. + + 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. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs index e8fac70f85ca5d..dc3172c8a87728 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs @@ -58,6 +58,14 @@ public Version DefaultRequestVersion } } + /// + /// Gets or sets the default value of for implicitly created requests in convenience methods, + /// e.g.: , . + /// + /// + /// Note that this property has no effect on any of the and overloads + /// since they accept fully initialized . + /// public HttpVersionPolicy DefaultVersionPolicy { get => _defaultVersionPolicy; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs index d8a36966fcdfee..bad8cdf6424f1c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs @@ -42,6 +42,9 @@ public Version Version } } + /// + /// Gets or sets the policy determining how is interpreted and how is the final HTTP version negotiated with the server. + /// public HttpVersionPolicy VersionPolicy { get { return _versionPolicy; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs index e149b55a9ed9a0..9f3ddb5569f36b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpVersionPolicy.cs @@ -3,10 +3,41 @@ namespace System.Net.Http { + /// + /// Determines behavior when selecting and negotiating HTTP version for a request. + /// public enum HttpVersionPolicy { + /// + /// Default behavior, either uses requested version or downgrades to a lower one. + /// + /// + /// If the server supports the requested version, either negotiated via ALPN (H2) or advertised via Alt-Svc (H3), + /// as well as a secure connection is being requested, the result is the . + /// Otherwise, downgrades to HTTP/1.1. + /// Note that this option does not allow use of prenegotiated clear text connection, e.g. H2C. + /// RequestVersionOrLower, + + /// + /// Tries to uses highest available version, downgrading only to the requested version, not bellow. + /// Throwing if a connection with higher or equal version cannot be established. + /// + /// + /// If the server supports higher than requested version, either negotiated via ALPN (H2) or advertised via Alt-Svc (H3), + /// as well as secure connection is being requested, the result is the highest available one. + /// Otherwise, downgrades to the . + /// Note that this option allows to use prenegotiated clear text connection for the requested version but not for anything higher. + /// RequestVersionOrHigher, + + /// + /// Uses only the requested version. + /// Throwing if a connection with the exact version cannot be established. + /// + /// + /// Note that this option allows to use prenegotiated clear text connection for the requested version. + /// RequestVersionExact } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs index c55b2eb2f1262f..467e813c49ac58 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs @@ -125,8 +125,6 @@ private static async ValueTask EstablishSslConnectionAsyncCore(bool a throw CancellationHelper.CreateOperationCanceledException(e, cancellationToken); } - // ToDo: let this exception buuble up to the caller, or should we somehow recognize that server didn't allow - // particular HTTP version in ALPN (how? Furtik will know) and use NSE instead. throw new HttpRequestException(SR.net_http_ssl_connection_failed, e); } 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 c6198798852206..d83159a951b0bb 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 @@ -341,15 +341,15 @@ public byte[] Http2AltSvcOriginUri // 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) + if (request.Version.Major == 3 && !_http3Enabled) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - (new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3))); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3))); } - if (request.Version.Major >= 2 && !_http2Enabled) + if (request.Version.Major == 2 && !_http2Enabled) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - (new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2))); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2))); } } @@ -366,8 +366,8 @@ public byte[] Http2AltSvcOriginUri { if (IsAltSvcBlocked(authority)) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - (new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); } return GetHttp3ConnectionAsync(request, authority, cancellationToken); @@ -376,8 +376,8 @@ public byte[] Http2AltSvcOriginUri // 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<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - (new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + 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)) && @@ -389,8 +389,8 @@ public byte[] Http2AltSvcOriginUri // 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<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - (new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2))); + return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2))); } return GetHttpConnectionAsync(request, async, cancellationToken); 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 0b0c66043d1c0a..6a8a3d5f03921a 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 @@ -423,7 +423,10 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request, } // Do not allow upgrades for synchronous requests, that might lead to asynchronous code-paths. - request.VersionPolicy = HttpVersionPolicy.RequestVersionOrLower; + if (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher) + { + throw new NotSupportedException(SR.Format(SR.net_http_upgrade_not_enabled_sync, nameof(Send), request.VersionPolicy)); + } CheckDisposed(); HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain(); diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs index 6ae68fbc32578d..840dd74ad7023e 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs @@ -1087,9 +1087,10 @@ await HttpAgnosticLoopbackServer.CreateClientAndServerAsync( { try { - await server.AcceptConnectionSendResponseAndCloseAsync(); + HttpRequestData requestData = await server.AcceptConnectionSendResponseAndCloseAsync(); + Assert.Equal(expectedResult, requestData.Version); } - catch (Exception ex) + catch (Exception ex) when (expectedResult is Type) { _output.WriteLine("Server exception: " + ex.ToString()); }