From 30da832eea85a668cd9c01970048b78eb88bef1b Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 24 May 2022 21:15:52 +0200 Subject: [PATCH 01/32] downgrade proposal for HttpClient --- .../SocketsHttpHandler/Http2Connection.cs | 12 +- .../Http/SocketsHttpHandler/Http2Stream.cs | 137 ++++++++++++++++++ .../SocketsHttpHandler/HttpConnectionPool.cs | 3 +- 3 files changed, 150 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 9dc706680c9b2d..55fcff66ab3e4d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -826,6 +826,13 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f // We don't actually store this value; we always send frames of the minimum size (16K). break; + case SettingId.EnableConnect: + if (settingValue == 1) + { + IsWebsocketEnabled = true; + } + break; + default: // All others are ignored because we don't care about them. // Note, per RFC, unknown settings IDs should be ignored. @@ -1888,9 +1895,12 @@ private enum SettingId : ushort MaxConcurrentStreams = 0x3, InitialWindowSize = 0x4, MaxFrameSize = 0x5, - MaxHeaderListSize = 0x6 + MaxHeaderListSize = 0x6, + EnableConnect = 0x7 } + internal bool IsWebsocketEnabled { get; private set; } = false; + // Note that this is safe to be called concurrently by multiple threads. public async Task SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index e522822c6caf5c..a76102f81a22c7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -44,6 +44,7 @@ private sealed class Http2Stream : IValueTaskSource, IHttpStreamHeadersHandler, private StreamCompletionState _requestCompletionState; private StreamCompletionState _responseCompletionState; private ResponseProtocolState _responseProtocolState; + private bool _webSocketEstablished; // If this is not null, then we have received a reset from the server // (i.e. RST_STREAM or general IO error processing the connection) @@ -531,6 +532,7 @@ void IHttpStreamHeadersHandler.OnStaticIndexedHeader(int index) if (index <= LastHPackRequestPseudoHeaderId) { + // add protocol and others pseudoheaders if (NetEventSource.Log.IsEnabled()) Trace($"Invalid request pseudo-header ID {index}."); throw new HttpRequestException(SR.net_http_invalid_response); } @@ -629,6 +631,12 @@ private void OnStatus(int statusCode) } else { + if (_response.RequestMessage != null && _response.RequestMessage.IsWebSocketRequest() && statusCode == 200) + { + _webSocketEstablished = true; + return; + } + _responseProtocolState = ResponseProtocolState.ExpectingHeaders; // If we are waiting for a 100-continue response, signal the waiter now. @@ -1036,6 +1044,10 @@ public async Task ReadResponseHeadersAsync(CancellationToken cancellationToken) MoveTrailersToResponseMessage(_response); responseContent.SetStream(EmptyReadStream.Instance); } + else if (_webSocketEstablished) + { + responseContent.SetStream(new Http2ReadWriteStream(this)); + } else { responseContent.SetStream(new Http2ReadStream(this)); @@ -1584,6 +1596,131 @@ public override Task FlushAsync(CancellationToken cancellationToken) return http2Stream._connection.FlushAsync(cancellationToken); } } + + public sealed class Http2ReadWriteStream : HttpBaseStream + { + private Http2Stream? _http2Stream; + // should be removed + private readonly HttpResponseMessage _responseMessage; + // need to handle data flow + + public Http2ReadWriteStream(Http2Stream http2Stream) + { + Debug.Assert(http2Stream != null); + Debug.Assert(http2Stream._response != null); + _http2Stream = http2Stream; + _responseMessage = _http2Stream._response; + } + + ~Http2ReadWriteStream() + { + if (NetEventSource.Log.IsEnabled()) _http2Stream?.Trace(""); + try + { + Dispose(disposing: false); + } + catch (Exception e) + { + if (NetEventSource.Log.IsEnabled()) _http2Stream?.Trace($"Error: {e}"); + } + } + + protected override void Dispose(bool disposing) + { + Http2Stream? http2Stream = Interlocked.Exchange(ref _http2Stream, null); + if (http2Stream == null) + { + return; + } + + // Technically we shouldn't be doing the following work when disposing == false, + // as the following work relies on other finalizable objects. But given the HTTP/2 + // protocol, we have little choice: if someone drops the Http2ReadStream without + // disposing of it, we need to a) signal to the server that the stream is being + // canceled, and b) clean up the associated state in the Http2Connection. + + http2Stream.CloseResponseBody(); + + base.Dispose(disposing); + } + + public override bool CanRead => _http2Stream != null; + public override bool CanWrite => _http2Stream != null; + + public override int Read(Span destination) + { + Http2Stream http2Stream = _http2Stream ?? throw new ObjectDisposedException(nameof(Http2ReadStream)); + + return http2Stream.ReadData(destination, _responseMessage); + } + + public override ValueTask ReadAsync(Memory destination, CancellationToken cancellationToken) + { + Http2Stream? http2Stream = _http2Stream; + + if (http2Stream == null) + { + return ValueTask.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream)))); + } + + if (cancellationToken.IsCancellationRequested) + { + return ValueTask.FromCanceled(cancellationToken); + } + + return http2Stream.ReadDataAsync(destination, _responseMessage, cancellationToken); + } + + public override void CopyTo(Stream destination, int bufferSize) + { + ValidateCopyToArguments(destination, bufferSize); + Http2Stream http2Stream = _http2Stream ?? throw ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream))); + http2Stream.CopyTo(_responseMessage, destination, bufferSize); + } + + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) + { + ValidateCopyToArguments(destination, bufferSize); + Http2Stream? http2Stream = _http2Stream; + return + http2Stream is null ? Task.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream)))) : + cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : + http2Stream.CopyToAsync(_responseMessage, destination, bufferSize, cancellationToken); + } + + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) + { + + Http2Stream? http2Stream = _http2Stream; + + if (http2Stream == null) + { + return ValueTask.FromException(new ObjectDisposedException(nameof(Http2WriteStream))); + } + + return http2Stream.SendDataAsync(buffer, cancellationToken); + } + + public override Task FlushAsync(CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + + Http2Stream? http2Stream = _http2Stream; + + if (http2Stream == null) + { + return Task.CompletedTask; + } + + // In order to flush this stream's previous writes, we need to flush the connection. We + // really only need to do any work here if the connection's buffer has any pending writes + // from this stream, but we currently lack a good/efficient/safe way of doing that. + return http2Stream._connection.FlushAsync(cancellationToken); + } + } } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 22306fd911c9bd..bbfea5e202a4de 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 @@ -998,6 +998,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn // Use HTTP/3 if possible. if (IsHttp3Supported() && // guard to enable trimming HTTP/3 support _http3Enabled && + !request.IsWebSocketRequest() && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { Debug.Assert(async); @@ -1019,7 +1020,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { Http2Connection? connection = await GetHttp2ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); Debug.Assert(connection is not null || !_http2Enabled); - if (connection is not null) + if (connection is not null && (!request.IsWebSocketRequest() || connection.IsWebsocketEnabled)) { response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); } From 7b03bc8f3319eb1d38e7d8ff538175b543d28683 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 7 Jun 2022 15:57:13 +0200 Subject: [PATCH 02/32] HttpClient to handle ws over h2 --- .../Net/HttpKnownHeaderNames.TryGetHeaderName.cs | 3 +++ .../src/System/Net/HttpKnownHeaderNames.cs | 1 + .../System.Net.Http/ref/System.Net.Http.cs | 1 + .../src/System/Net/Http/Headers/KnownHeaders.cs | 8 +++++++- .../src/System/Net/Http/HttpMethod.cs | 2 +- .../src/System/Net/Http/HttpRequestMessage.cs | 16 ++++++++++++++++ .../Http/SocketsHttpHandler/Http2Connection.cs | 2 +- .../Net/Http/SocketsHttpHandler/Http2Stream.cs | 1 - 8 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs index 6604f483a631c9..63e57ab8a63f96 100644 --- a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs +++ b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs @@ -181,6 +181,9 @@ private static bool TryGetHeaderName( } break; + case 9: + potentialHeader = Protocol; goto TryMatch; // :protocol + case 10: switch (charAt(key, startIndex)) { diff --git a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs index c88ba19b81251d..6101a85165e7d2 100644 --- a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs +++ b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs @@ -55,6 +55,7 @@ internal static partial class HttpKnownHeaderNames public const string Origin = "Origin"; public const string P3P = "P3P"; public const string Pragma = "Pragma"; + public const string Protocol = ":protocol"; public const string ProxyAuthenticate = "Proxy-Authenticate"; public const string ProxyAuthorization = "Proxy-Authorization"; public const string ProxyConnection = "Proxy-Connection"; 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 debcc28d0119a5..8a692bff4b411c 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -231,6 +231,7 @@ public HttpMethod(string method) { } public static System.Net.Http.HttpMethod Post { get { throw null; } } public static System.Net.Http.HttpMethod Put { get { throw null; } } public static System.Net.Http.HttpMethod Trace { get { throw null; } } + public static System.Net.Http.HttpMethod Connect { get { throw null; } } public bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] System.Net.Http.HttpMethod? other) { throw null; } public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] object? obj) { throw null; } public override int GetHashCode() { throw null; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs index d1b624d06260ea..69c89a58062791 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs @@ -12,6 +12,7 @@ internal static class KnownHeaders // If you add a new entry here, you need to add it to TryGetKnownHeader below as well. public static readonly KnownHeader PseudoStatus = new KnownHeader(":status", HttpHeaderType.Response, parser: null); + public static readonly KnownHeader PseudoProtocol = new KnownHeader(":protocol", HttpHeaderType.Request, parser: null); public static readonly KnownHeader Accept = new KnownHeader("Accept", HttpHeaderType.Request, MediaTypeHeaderParser.MultipleValuesParser, null, H2StaticTable.Accept, H3StaticTable.AcceptAny); public static readonly KnownHeader AcceptCharset = new KnownHeader("Accept-Charset", HttpHeaderType.Request, GenericHeaderParser.MultipleValueStringWithQualityParser, null, H2StaticTable.AcceptCharset); public static readonly KnownHeader AcceptEncoding = new KnownHeader("Accept-Encoding", HttpHeaderType.Request, GenericHeaderParser.MultipleValueStringWithQualityParser, null, H2StaticTable.AcceptEncoding, H3StaticTable.AcceptEncodingGzipDeflateBr); @@ -244,7 +245,12 @@ public BytePtrAccessor(byte* p, int length) break; case 9: - return ExpectCT; // Expect-CT + switch (key[0] | 0x20) + { + case ':': return PseudoProtocol; // [:]protocol + case 'e': return ExpectCT; // [E]xpect-CT + } + break; case 10: switch (key[0] | 0x20) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs index 6fddc9dcb964fc..5743ff3ec8bef0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpMethod.cs @@ -67,7 +67,7 @@ public static HttpMethod Patch // Don't expose CONNECT as static property, since it's used by the transport to connect to a proxy. // CONNECT is not used by users directly. - internal static HttpMethod Connect + public static HttpMethod Connect { get { return s_connectMethod; } } 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 479ce771ad343b..aa544e1c03e0e6 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 @@ -168,6 +168,22 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; + internal bool IsWebSocketRequest() + { + if (Headers.TryGetValues(":protocol", out IEnumerable? values)) + { + var valuesArray = (string[])values; + return valuesArray.Length > 0 && valuesArray[0] == "websocket"; + } + + if (Headers.TryGetValues("Upgrade", out values)) + { + var valuesArray = (string[])values; + return valuesArray.Length > 0 && valuesArray[0] == "websocket"; + } + return false; + } + #region IDisposable Members protected virtual void Dispose(bool disposing) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 55fcff66ab3e4d..54d196f1b59a36 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1899,7 +1899,7 @@ private enum SettingId : ushort EnableConnect = 0x7 } - internal bool IsWebsocketEnabled { get; private set; } = false; + internal bool IsWebsocketEnabled { get; private set; } = true; // Note that this is safe to be called concurrently by multiple threads. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index a76102f81a22c7..8e6060fda20de2 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -634,7 +634,6 @@ private void OnStatus(int statusCode) if (_response.RequestMessage != null && _response.RequestMessage.IsWebSocketRequest() && statusCode == 200) { _webSocketEstablished = true; - return; } _responseProtocolState = ResponseProtocolState.ExpectingHeaders; From e7b83322d0d5db52f3a6797e0afa1ce80c133a35 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 7 Jun 2022 16:33:08 +0200 Subject: [PATCH 03/32] ClientWebSocket to handle ws over h2 --- .../ref/System.Net.WebSockets.Client.cs | 6 +- .../ClientWebSocketOptions.cs | 14 + .../System/Net/WebSockets/ClientWebSocket.cs | 24 +- .../Net/WebSockets/ClientWebSocketOptions.cs | 24 ++ .../Net/WebSockets/WebSocketHandle.Managed.cs | 265 ++++++++++++------ 5 files changed, 236 insertions(+), 97 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs index 96cecd9e30f471..2ee9c313158460 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs @@ -3,7 +3,6 @@ // ------------------------------------------------------------------------------ // Changes to this file must follow the https://aka.ms/api-review process. // ------------------------------------------------------------------------------ - namespace System.Net.WebSockets { public sealed partial class ClientWebSocket : System.Net.WebSockets.WebSocket @@ -18,6 +17,7 @@ public override void Abort() { } public override System.Threading.Tasks.Task CloseAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.Task CloseOutputAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Threading.CancellationToken cancellationToken) { throw null; } + public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Net.Http.HttpMessageInvoker sharedHandler, System.Threading.CancellationToken cancellationToken) { throw null; } public override void Dispose() { } public override System.Threading.Tasks.Task ReceiveAsync(System.ArraySegment buffer, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.ValueTask ReceiveAsync(System.Memory buffer, System.Threading.CancellationToken cancellationToken) { throw null; } @@ -43,6 +43,10 @@ internal ClientWebSocketOptions() { } public System.Net.Security.RemoteCertificateValidationCallback? RemoteCertificateValidationCallback { get { throw null; } set { } } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public bool UseDefaultCredentials { get { throw null; } set { } } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] + public System.Version Version { get { throw null; } set { } } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] + public System.Net.Http.HttpVersionPolicy VersionPolicy { get { throw null; } set { } } public void AddSubProtocol(string subProtocol) { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public void SetBuffer(int receiveBufferSize, int sendBufferSize) { } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs index 79dd04229b9c33..c0636fce6e6c33 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs @@ -32,6 +32,20 @@ public bool UseDefaultCredentials set => throw new PlatformNotSupportedException(); } + [UnsupportedOSPlatform("browser")] + public Version Version + { + get => throw new PlatformNotSupportedException(); + set => throw new PlatformNotSupportedException(); + } + + [UnsupportedOSPlatform("browser")] + public System.Net.Http.HttpVersionPolicy VersionPolicy + { + get => throw new PlatformNotSupportedException(); + set => throw new PlatformNotSupportedException(); + } + [UnsupportedOSPlatform("browser")] public System.Net.ICredentials Credentials { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index 5905497a6f911f..27e6b03dbfce63 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -51,6 +52,16 @@ public override WebSocketState State } public Task ConnectAsync(Uri uri, CancellationToken cancellationToken) + { + return ConnectAsyncHelper(uri, null, cancellationToken); + } + + public Task ConnectAsync(Uri uri, HttpMessageInvoker sharedHandler, CancellationToken cancellationToken) + { + return ConnectAsyncHelper(uri, sharedHandler, cancellationToken); + } + + public Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? sharedHandler, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(uri); @@ -77,16 +88,23 @@ public Task ConnectAsync(Uri uri, CancellationToken cancellationToken) } Options.SetToReadOnly(); - return ConnectAsyncCore(uri, cancellationToken); + return ConnectAsyncCore(uri, sharedHandler, cancellationToken); } - private async Task ConnectAsyncCore(Uri uri, CancellationToken cancellationToken) + private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? sharedHandler, CancellationToken cancellationToken) { _innerWebSocket = new WebSocketHandle(); try { - await _innerWebSocket.ConnectAsync(uri, cancellationToken, Options).ConfigureAwait(false); + if (sharedHandler == null) + { + await _innerWebSocket.ConnectAsync(uri, cancellationToken, Options).ConfigureAwait(false); + } + else + { + await _innerWebSocket.ConnectAsync(uri, sharedHandler, cancellationToken, Options).ConfigureAwait(false); + } } catch { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index d58cda99112eec..dbeda69166fdff 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Net.Http; using System.Net.Security; using System.Runtime.Versioning; using System.Security.Cryptography.X509Certificates; @@ -25,6 +26,29 @@ public sealed class ClientWebSocketOptions internal X509CertificateCollection? _clientCertificates; internal WebHeaderCollection? _requestHeaders; internal List? _requestedSubProtocols; + private Version _version = HttpVersion.Version11; + private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; + + [UnsupportedOSPlatform("browser")] + public Version Version + { + get { return _version; } + set + { + ArgumentNullException.ThrowIfNull(value); + _version = value; + } + } + + [UnsupportedOSPlatform("browser")] + public HttpVersionPolicy VersionPolicy + { + get { return _versionPolicy; } + set + { + _versionPolicy = value; + } + } internal ClientWebSocketOptions() { } // prevent external instantiation diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index f19f9a572772f0..189ac2e735584d 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -44,115 +44,154 @@ public void Abort() public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, ClientWebSocketOptions options) { - HttpResponseMessage? response = null; - SocketsHttpHandler? handler = null; - bool disposeHandler = true; - try + SocketsHttpHandler handler = SetupHandler(options); + await ConnectAsyncHelper(uri, new HttpMessageInvoker(handler), cancellationToken, options).ConfigureAwait(false); + } + + public async Task ConnectAsync(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + { + await ConnectAsyncHelper(uri, handler, cancellationToken, options).ConfigureAwait(false); + } + + private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) + { + SocketsHttpHandler? handler; + // Create the handler for this request and populate it with all of the options. + // Try to use a shared handler rather than creating a new one just for this request, if + // the options are compatible. + if (options.Credentials == null && + !options.UseDefaultCredentials && + options.Proxy == null && + options.Cookies == null && + options.RemoteCertificateValidationCallback == null && + options._clientCertificates?.Count == 0) { - var request = new HttpRequestMessage(HttpMethod.Get, uri); - if (options._requestHeaders?.Count > 0) // use field to avoid lazily initializing the collection + handler = s_defaultHandler; + if (handler == null) { - foreach (string key in options.RequestHeaders) + handler = new SocketsHttpHandler() + { + PooledConnectionLifetime = TimeSpan.Zero, + UseProxy = false, + UseCookies = false, + }; + if (Interlocked.CompareExchange(ref s_defaultHandler, handler, null) != null) { - request.Headers.TryAddWithoutValidation(key, options.RequestHeaders[key]); + handler.Dispose(); + handler = s_defaultHandler; } } + } + else + { + handler = new SocketsHttpHandler(); + handler.PooledConnectionLifetime = TimeSpan.Zero; + handler.CookieContainer = options.Cookies; + handler.UseCookies = options.Cookies != null; + handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback; - // Create the security key and expected response, then build all of the request headers - KeyValuePair secKeyAndSecWebSocketAccept = CreateSecKeyAndSecWebSocketAccept(); - AddWebSocketHeaders(request, secKeyAndSecWebSocketAccept.Key, options); - - // Create the handler for this request and populate it with all of the options. - // Try to use a shared handler rather than creating a new one just for this request, if - // the options are compatible. - if (options.Credentials == null && - !options.UseDefaultCredentials && - options.Proxy == null && - options.Cookies == null && - options.RemoteCertificateValidationCallback == null && - options._clientCertificates?.Count == 0) + if (options.UseDefaultCredentials) { - disposeHandler = false; - handler = s_defaultHandler; - if (handler == null) - { - handler = new SocketsHttpHandler() - { - PooledConnectionLifetime = TimeSpan.Zero, - UseProxy = false, - UseCookies = false, - }; - if (Interlocked.CompareExchange(ref s_defaultHandler, handler, null) != null) - { - handler.Dispose(); - handler = s_defaultHandler; - } - } + handler.Credentials = CredentialCache.DefaultCredentials; } else { - handler = new SocketsHttpHandler(); - handler.PooledConnectionLifetime = TimeSpan.Zero; - handler.CookieContainer = options.Cookies; - handler.UseCookies = options.Cookies != null; - handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback; + handler.Credentials = options.Credentials; + } - if (options.UseDefaultCredentials) - { - handler.Credentials = CredentialCache.DefaultCredentials; - } - else - { - handler.Credentials = options.Credentials; - } + if (options.Proxy == null) + { + handler.UseProxy = false; + } + else if (options.Proxy != DefaultWebProxy.Instance) + { + handler.Proxy = options.Proxy; + } - if (options.Proxy == null) - { - handler.UseProxy = false; - } - else if (options.Proxy != DefaultWebProxy.Instance) - { - handler.Proxy = options.Proxy; - } + if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection + { + Debug.Assert(handler.SslOptions.ClientCertificates == null); + handler.SslOptions.ClientCertificates = new X509Certificate2Collection(); + handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates); + } + } + return handler; + } - if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection - { - Debug.Assert(handler.SslOptions.ClientCertificates == null); - handler.SslOptions.ClientCertificates = new X509Certificate2Collection(); - handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates); - } + private async Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + { + HttpResponseMessage? response = null; + + // TODO setup to false + bool disposeHandler = true; + try + { + if (options.Version.Major >= 3 && options.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw new Exception(); } - // Issue the request. The response must be status code 101. - CancellationTokenSource? linkedCancellation; - CancellationTokenSource externalAndAbortCancellation; - if (cancellationToken.CanBeCanceled) // avoid allocating linked source if external token is not cancelable + var request = new HttpRequestMessage(HttpMethod.Get, uri); + if (options.Version.Major >= 2 || (options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { - linkedCancellation = - externalAndAbortCancellation = - CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _abortSource.Token); + request.Version = new Version(2, 0); } else { - linkedCancellation = null; - externalAndAbortCancellation = _abortSource; + request.Version = new Version(1, 1); } - using (linkedCancellation) + while (true) { - response = await new HttpMessageInvoker(handler).SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); - externalAndAbortCancellation.Token.ThrowIfCancellationRequested(); // poll in case sends/receives in request/response didn't observe cancellation - } + try + { + if (options._requestHeaders?.Count > 0) // use field to avoid lazily initializing the collection + { + foreach (string key in options.RequestHeaders) + { + request.Headers.TryAddWithoutValidation(key, options.RequestHeaders[key]); + } + } - if (response.StatusCode != HttpStatusCode.SwitchingProtocols) - { - throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); - } + // Create the security key and expected response, then build all of the request headers + KeyValuePair secKeyAndSecWebSocketAccept = CreateSecKeyAndSecWebSocketAccept(); + AddWebSocketHeaders(request, secKeyAndSecWebSocketAccept.Key, options); - // The Connection, Upgrade, and SecWebSocketAccept headers are required and with specific values. - ValidateHeader(response.Headers, HttpKnownHeaderNames.Connection, "Upgrade"); - ValidateHeader(response.Headers, HttpKnownHeaderNames.Upgrade, "websocket"); - ValidateHeader(response.Headers, HttpKnownHeaderNames.SecWebSocketAccept, secKeyAndSecWebSocketAccept.Value); + // Issue the request. The response must be status code 101. + CancellationTokenSource? linkedCancellation; + CancellationTokenSource externalAndAbortCancellation; + if (cancellationToken.CanBeCanceled) // avoid allocating linked source if external token is not cancelable + { + linkedCancellation = + externalAndAbortCancellation = + CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _abortSource.Token); + } + else + { + linkedCancellation = null; + externalAndAbortCancellation = _abortSource; + } + + using (linkedCancellation) + { + response = await handler.SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); + externalAndAbortCancellation.Token.ThrowIfCancellationRequested(); // poll in case sends/receives in request/response didn't observe cancellation + } + ValidateResponse(response, secKeyAndSecWebSocketAccept.Value, options); + break; + } + catch (HttpRequestException ex) + { + if (request.Version.Major == 2 + && (options.Version.Major == 2 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower + || options.Version.Major == 1 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + { + request.Version = new Version(1, 1); + } + else { throw ex; } + } + + } // The SecWebSocketProtocol header is optional. We should only get it with a non-empty value if we requested subprotocols, // and then it must only be one of the ones we requested. If we got a subprotocol other than one we requested (or if we @@ -200,11 +239,6 @@ public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, Cli } } - if (response.Content is null) - { - throw new WebSocketException(WebSocketError.ConnectionClosedPrematurely); - } - // Get the response stream and wrap it in a web socket. Stream connectedStream = response.Content.ReadAsStream(); Debug.Assert(connectedStream.CanWrite); @@ -319,10 +353,27 @@ static int ParseWindowBits(ReadOnlySpan value) /// The options controlling the request. private static void AddWebSocketHeaders(HttpRequestMessage request, string secKey, ClientWebSocketOptions options) { - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Connection, HttpKnownHeaderNames.Upgrade); - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Upgrade, "websocket"); + request.Version = options.Version; + // always exact because we handle downgrade here + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + + if (options.Version == HttpVersion.Version11) + { + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Connection, HttpKnownHeaderNames.Upgrade); + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Upgrade, "websocket"); + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKey); + } + else if (options.Version == HttpVersion.Version20) + { + request.Headers.TryAddWithoutValidation(":method", "CONNECT"); + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Protocol, "websocket"); + request.Headers.TryAddWithoutValidation(":scheme", "https"); + request.Headers.TryAddWithoutValidation(":path", "/chat"); + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Origin, request.Headers.Host); + } + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketVersion, "13"); - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKey); + if (options._requestedSubProtocols?.Count > 0) { request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketProtocol, string.Join(", ", options.RequestedSubProtocols)); @@ -367,6 +418,34 @@ static string GetDeflateOptions(WebSocketDeflateOptions options) } } + private static void ValidateResponse(HttpResponseMessage response, string secValue, ClientWebSocketOptions options) + { + if (options.Version == HttpVersion.Version11) + { + if (response.StatusCode != HttpStatusCode.SwitchingProtocols) + { + throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); + } + + // The Connection, Upgrade, and SecWebSocketAccept headers are required and with specific values. + ValidateHeader(response.Headers, HttpKnownHeaderNames.Connection, "Upgrade"); + ValidateHeader(response.Headers, HttpKnownHeaderNames.Upgrade, "websocket"); + ValidateHeader(response.Headers, HttpKnownHeaderNames.SecWebSocketAccept, secValue); + } + else if (options.Version == HttpVersion.Version20) + { + if (response.StatusCode != HttpStatusCode.OK) + { + throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); + } + } + + if (response.Content is null) + { + throw new WebSocketException(WebSocketError.ConnectionClosedPrematurely); + } + } + /// /// Creates a pair of a security key for sending in the Sec-WebSocket-Key header and /// the associated response we expect to receive as the Sec-WebSocket-Accept header value. From a5e991a0afac420567f79234ab2659f341a4d15e Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 17 Jun 2022 16:51:27 +0200 Subject: [PATCH 04/32] Add property for protocol header and remove it from known headers --- .../Net/HttpKnownHeaderNames.TryGetHeaderName.cs | 3 --- .../Common/src/System/Net/HttpKnownHeaderNames.cs | 1 - .../System.Net.Http/ref/System.Net.Http.cs | 1 + .../System/Net/Http/Headers/HttpRequestHeaders.cs | 13 +++++++++++++ .../src/System/Net/Http/Headers/KnownHeaders.cs | 8 +------- .../src/System/Net/Http/HttpRequestMessage.cs | 13 +++---------- .../Http/SocketsHttpHandler/Http2Connection.cs | 15 ++++++++++++--- .../Net/Http/SocketsHttpHandler/Http2Stream.cs | 2 +- .../Http/SocketsHttpHandler/HttpConnectionPool.cs | 15 ++++++++++++--- .../ref/System.Net.WebSockets.Client.cs | 2 +- .../src/System/Net/WebSockets/ClientWebSocket.cs | 8 ++++---- .../Net/WebSockets/WebSocketHandle.Browser.cs | 6 ++++++ .../Net/WebSockets/WebSocketHandle.Managed.cs | 8 +++----- 13 files changed, 57 insertions(+), 38 deletions(-) diff --git a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs index 63e57ab8a63f96..6604f483a631c9 100644 --- a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs +++ b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.TryGetHeaderName.cs @@ -181,9 +181,6 @@ private static bool TryGetHeaderName( } break; - case 9: - potentialHeader = Protocol; goto TryMatch; // :protocol - case 10: switch (charAt(key, startIndex)) { diff --git a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs index 6101a85165e7d2..c88ba19b81251d 100644 --- a/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs +++ b/src/libraries/Common/src/System/Net/HttpKnownHeaderNames.cs @@ -55,7 +55,6 @@ internal static partial class HttpKnownHeaderNames public const string Origin = "Origin"; public const string P3P = "P3P"; public const string Pragma = "Pragma"; - public const string Protocol = ":protocol"; public const string ProxyAuthenticate = "Proxy-Authenticate"; public const string ProxyAuthorization = "Proxy-Authorization"; public const string ProxyConnection = "Proxy-Connection"; 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 8a692bff4b411c..5488a4036f7528 100644 --- a/src/libraries/System.Net.Http/ref/System.Net.Http.cs +++ b/src/libraries/System.Net.Http/ref/System.Net.Http.cs @@ -657,6 +657,7 @@ internal HttpRequestHeaders() { } public System.DateTimeOffset? IfUnmodifiedSince { get { throw null; } set { } } public int? MaxForwards { get { throw null; } set { } } public System.Net.Http.Headers.HttpHeaderValueCollection Pragma { get { throw null; } } + public string? Protocol { get { throw null; } set { } } public System.Net.Http.Headers.AuthenticationHeaderValue? ProxyAuthorization { get { throw null; } set { } } public System.Net.Http.Headers.RangeHeaderValue? Range { get { throw null; } set { } } public System.Uri? Referrer { get { throw null; } set { } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index 5da7cfb9844eda..482dbc174e9e45 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -159,6 +159,19 @@ public int? MaxForwards set { SetOrRemoveParsedValue(KnownHeaders.MaxForwards.Descriptor, value); } } + public string? Protocol + { + get { return (string?)GetSingleParsedValue(new HeaderDescriptor(":protocol")); } + set + { + if (value == null) + { + throw new FormatException(SR.net_http_headers_invalid_host_header); + } + SetParsedValue(new HeaderDescriptor(":protocol"), value); + } + } + public AuthenticationHeaderValue? ProxyAuthorization { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs index 69c89a58062791..d1b624d06260ea 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs @@ -12,7 +12,6 @@ internal static class KnownHeaders // If you add a new entry here, you need to add it to TryGetKnownHeader below as well. public static readonly KnownHeader PseudoStatus = new KnownHeader(":status", HttpHeaderType.Response, parser: null); - public static readonly KnownHeader PseudoProtocol = new KnownHeader(":protocol", HttpHeaderType.Request, parser: null); public static readonly KnownHeader Accept = new KnownHeader("Accept", HttpHeaderType.Request, MediaTypeHeaderParser.MultipleValuesParser, null, H2StaticTable.Accept, H3StaticTable.AcceptAny); public static readonly KnownHeader AcceptCharset = new KnownHeader("Accept-Charset", HttpHeaderType.Request, GenericHeaderParser.MultipleValueStringWithQualityParser, null, H2StaticTable.AcceptCharset); public static readonly KnownHeader AcceptEncoding = new KnownHeader("Accept-Encoding", HttpHeaderType.Request, GenericHeaderParser.MultipleValueStringWithQualityParser, null, H2StaticTable.AcceptEncoding, H3StaticTable.AcceptEncodingGzipDeflateBr); @@ -245,12 +244,7 @@ public BytePtrAccessor(byte* p, int length) break; case 9: - switch (key[0] | 0x20) - { - case ':': return PseudoProtocol; // [:]protocol - case 'e': return ExpectCT; // [E]xpect-CT - } - break; + return ExpectCT; // Expect-CT case 10: switch (key[0] | 0x20) 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 aa544e1c03e0e6..b82e88f3a55029 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 @@ -168,18 +168,11 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; - internal bool IsWebSocketRequest() + internal bool IsWebSocketH2Request() { - if (Headers.TryGetValues(":protocol", out IEnumerable? values)) + if (_version.Major == 2 && Headers.Protocol == "websocket") { - var valuesArray = (string[])values; - return valuesArray.Length > 0 && valuesArray[0] == "websocket"; - } - - if (Headers.TryGetValues("Upgrade", out values)) - { - var valuesArray = (string[])values; - return valuesArray.Length > 0 && valuesArray[0] == "websocket"; + return true; } return false; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 54d196f1b59a36..219d115570f2ff 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -829,7 +829,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f case SettingId.EnableConnect: if (settingValue == 1) { - IsWebsocketEnabled = true; + IsConnectEnabled = true; } break; @@ -1453,6 +1453,15 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff WriteIndexedHeader(H2StaticTable.PathSlash, pathAndQuery, ref headerBuffer); } + if (request.HasHeaders && request.Headers.Protocol != null) + { + WriteBytes(":protocol", ref headerBuffer); + Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); + WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); + + request.Headers.Protocol = null; + } + if (request.HasHeaders) { WriteHeaderCollection(request, request.Headers, ref headerBuffer); @@ -1896,10 +1905,10 @@ private enum SettingId : ushort InitialWindowSize = 0x4, MaxFrameSize = 0x5, MaxHeaderListSize = 0x6, - EnableConnect = 0x7 + EnableConnect = 0x8 } - internal bool IsWebsocketEnabled { get; private set; } = true; + internal bool IsConnectEnabled { get; private set; } // Note that this is safe to be called concurrently by multiple threads. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 8e6060fda20de2..a8e3cfd43dff71 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -631,7 +631,7 @@ private void OnStatus(int statusCode) } else { - if (_response.RequestMessage != null && _response.RequestMessage.IsWebSocketRequest() && statusCode == 200) + if (_response.RequestMessage != null && _response.RequestMessage.IsWebSocketH2Request() && statusCode == 200) { _webSocketEstablished = true; } 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 bbfea5e202a4de..812c65f15f2135 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 @@ -998,7 +998,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn // Use HTTP/3 if possible. if (IsHttp3Supported() && // guard to enable trimming HTTP/3 support _http3Enabled && - !request.IsWebSocketRequest() && + !request.IsWebSocketH2Request() && (request.Version.Major >= 3 || (request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher && IsSecure))) { Debug.Assert(async); @@ -1020,9 +1020,18 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { Http2Connection? connection = await GetHttp2ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); Debug.Assert(connection is not null || !_http2Enabled); - if (connection is not null && (!request.IsWebSocketRequest() || connection.IsWebsocketEnabled)) + if (connection is not null) { - response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); + if (!request.IsWebSocketH2Request() || connection.IsConnectEnabled) + { + response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); + } + else if (request.IsWebSocketH2Request() && !connection.IsConnectEnabled) + { + HttpRequestException exception = new("Extended CONNECT is not supported"); + exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; + throw exception; + } } } diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs index 2ee9c313158460..1f3e10829f27de 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs @@ -17,7 +17,7 @@ public override void Abort() { } public override System.Threading.Tasks.Task CloseAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.Task CloseOutputAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Threading.CancellationToken cancellationToken) { throw null; } - public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Net.Http.HttpMessageInvoker sharedHandler, System.Threading.CancellationToken cancellationToken) { throw null; } + public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Net.Http.HttpMessageInvoker invoker, System.Threading.CancellationToken cancellationToken) { throw null; } public override void Dispose() { } public override System.Threading.Tasks.Task ReceiveAsync(System.ArraySegment buffer, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.ValueTask ReceiveAsync(System.Memory buffer, System.Threading.CancellationToken cancellationToken) { throw null; } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index 27e6b03dbfce63..a6cb3365a37ce4 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -56,12 +56,12 @@ public Task ConnectAsync(Uri uri, CancellationToken cancellationToken) return ConnectAsyncHelper(uri, null, cancellationToken); } - public Task ConnectAsync(Uri uri, HttpMessageInvoker sharedHandler, CancellationToken cancellationToken) + public Task ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken) { - return ConnectAsyncHelper(uri, sharedHandler, cancellationToken); + return ConnectAsyncHelper(uri, invoker, cancellationToken); } - public Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? sharedHandler, CancellationToken cancellationToken) + private Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(uri); @@ -88,7 +88,7 @@ public Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? sharedHandler, Cance } Options.SetToReadOnly(); - return ConnectAsyncCore(uri, sharedHandler, cancellationToken); + return ConnectAsyncCore(uri, invoker, cancellationToken); } private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? sharedHandler, CancellationToken cancellationToken) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs index 3a948cc64ab34f..99c604a2992449 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -35,5 +36,10 @@ public Task ConnectAsync(Uri uri, CancellationToken cancellationToken, ClientWeb WebSocket = ws; return ws.ConnectAsync(uri, options.RequestedSubProtocols, cancellationToken); } + + public Task ConnectAsync(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + { + throw new NotImplementedException(); + } } } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 189ac2e735584d..aa749bdd1facb4 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -182,7 +182,7 @@ private async Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker handler, Cance } catch (HttpRequestException ex) { - if (request.Version.Major == 2 + if ( ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && request.Version.Major == 2 && (options.Version.Major == 2 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower || options.Version.Major == 1 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { @@ -365,10 +365,8 @@ private static void AddWebSocketHeaders(HttpRequestMessage request, string secKe } else if (options.Version == HttpVersion.Version20) { - request.Headers.TryAddWithoutValidation(":method", "CONNECT"); - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Protocol, "websocket"); - request.Headers.TryAddWithoutValidation(":scheme", "https"); - request.Headers.TryAddWithoutValidation(":path", "/chat"); + request.Method = HttpMethod.Connect; + request.Headers.Protocol = "websocket"; request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Origin, request.Headers.Host); } From 264c8022f4511d500f9b34412bef62be82a82585 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Sat, 18 Jun 2022 15:54:11 +0200 Subject: [PATCH 05/32] Update src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs Co-authored-by: Stephen Toub --- .../src/System/Net/Http/HttpRequestMessage.cs | 9 +-------- 1 file changed, 1 insertion(+), 8 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 b82e88f3a55029..6c9a51d21dd04b 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 @@ -168,14 +168,7 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; - internal bool IsWebSocketH2Request() - { - if (_version.Major == 2 && Headers.Protocol == "websocket") - { - return true; - } - return false; - } + internal bool IsWebSocketH2Request() => _version.Major == 2 && HasHeaders && Headers.Protocol == "websocket"; #region IDisposable Members From 08d1f8c7819687cb4f320462875a26d62bdc42c4 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Sat, 18 Jun 2022 19:16:22 +0200 Subject: [PATCH 06/32] Address review feedback --- .../Net/Http/Headers/HttpRequestHeaders.cs | 9 +- .../SocketsHttpHandler/Http2Connection.cs | 2 +- .../ref/System.Net.WebSockets.Client.cs | 12 +- .../ref/System.Net.WebSockets.Client.csproj | 1 + .../ClientWebSocketOptions.cs | 11 +- .../System/Net/WebSockets/ClientWebSocket.cs | 9 +- .../Net/WebSockets/ClientWebSocketOptions.cs | 8 +- .../Net/WebSockets/WebSocketHandle.Browser.cs | 7 +- .../Net/WebSockets/WebSocketHandle.Managed.cs | 146 ++++++++---------- 9 files changed, 91 insertions(+), 114 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index 482dbc174e9e45..8e2be7874a8e3e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -162,14 +162,7 @@ public int? MaxForwards public string? Protocol { get { return (string?)GetSingleParsedValue(new HeaderDescriptor(":protocol")); } - set - { - if (value == null) - { - throw new FormatException(SR.net_http_headers_invalid_host_header); - } - SetParsedValue(new HeaderDescriptor(":protocol"), value); - } + set { SetOrRemoveParsedValue(new HeaderDescriptor(":protocol"), value); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 219d115570f2ff..5d113be9969eaa 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1455,7 +1455,7 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff if (request.HasHeaders && request.Headers.Protocol != null) { - WriteBytes(":protocol", ref headerBuffer); + WriteBytes(":protocol"u8, ref headerBuffer); Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs index 1f3e10829f27de..7247bfad69dd2d 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs @@ -17,7 +17,7 @@ public override void Abort() { } public override System.Threading.Tasks.Task CloseAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.Task CloseOutputAsync(System.Net.WebSockets.WebSocketCloseStatus closeStatus, string? statusDescription, System.Threading.CancellationToken cancellationToken) { throw null; } public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Threading.CancellationToken cancellationToken) { throw null; } - public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Net.Http.HttpMessageInvoker invoker, System.Threading.CancellationToken cancellationToken) { throw null; } + public System.Threading.Tasks.Task ConnectAsync(System.Uri uri, System.Net.Http.HttpMessageInvoker? invoker, System.Threading.CancellationToken cancellationToken) { throw null; } public override void Dispose() { } public override System.Threading.Tasks.Task ReceiveAsync(System.ArraySegment buffer, System.Threading.CancellationToken cancellationToken) { throw null; } public override System.Threading.Tasks.ValueTask ReceiveAsync(System.Memory buffer, System.Threading.CancellationToken cancellationToken) { throw null; } @@ -43,10 +43,12 @@ internal ClientWebSocketOptions() { } public System.Net.Security.RemoteCertificateValidationCallback? RemoteCertificateValidationCallback { get { throw null; } set { } } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public bool UseDefaultCredentials { get { throw null; } set { } } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] - public System.Version Version { get { throw null; } set { } } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] - public System.Net.Http.HttpVersionPolicy VersionPolicy { get { throw null; } set { } } + public System.Version Version { get { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] + set { } } + public System.Net.Http.HttpVersionPolicy VersionPolicy { get { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] + set { } } public void AddSubProtocol(string subProtocol) { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public void SetBuffer(int receiveBufferSize, int sendBufferSize) { } diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.csproj b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.csproj index 57de4cf647d84c..deca422762934e 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.csproj +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.csproj @@ -8,6 +8,7 @@ + diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs index c0636fce6e6c33..fae2b88163c3e6 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; +using System.Net.Http; using System.Runtime.Versioning; using System.Security.Cryptography.X509Certificates; @@ -12,6 +13,8 @@ public sealed class ClientWebSocketOptions { private bool _isReadOnly; // After ConnectAsync is called the options cannot be modified. private List? _requestedSubProtocols; + private Version _version = HttpVersion.Version11; + private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; internal ClientWebSocketOptions() { } @@ -32,17 +35,17 @@ public bool UseDefaultCredentials set => throw new PlatformNotSupportedException(); } - [UnsupportedOSPlatform("browser")] public Version Version { - get => throw new PlatformNotSupportedException(); + get => _version; + [UnsupportedOSPlatform("browser")] set => throw new PlatformNotSupportedException(); } - [UnsupportedOSPlatform("browser")] public System.Net.Http.HttpVersionPolicy VersionPolicy { - get => throw new PlatformNotSupportedException(); + get => _versionPolicy; + [UnsupportedOSPlatform("browser")] set => throw new PlatformNotSupportedException(); } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index a6cb3365a37ce4..668a8b549b201f 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -97,14 +97,7 @@ private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? sharedHandler, try { - if (sharedHandler == null) - { - await _innerWebSocket.ConnectAsync(uri, cancellationToken, Options).ConfigureAwait(false); - } - else - { - await _innerWebSocket.ConnectAsync(uri, sharedHandler, cancellationToken, Options).ConfigureAwait(false); - } + await _innerWebSocket.ConnectAsync(uri, sharedHandler, cancellationToken, Options).ConfigureAwait(false); } catch { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index dbeda69166fdff..985fcc0180a355 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -29,10 +29,10 @@ public sealed class ClientWebSocketOptions private Version _version = HttpVersion.Version11; private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; - [UnsupportedOSPlatform("browser")] public Version Version { - get { return _version; } + get => _version; + [UnsupportedOSPlatform("browser")] set { ArgumentNullException.ThrowIfNull(value); @@ -40,10 +40,10 @@ public Version Version } } - [UnsupportedOSPlatform("browser")] public HttpVersionPolicy VersionPolicy { - get { return _versionPolicy; } + get => _versionPolicy; + [UnsupportedOSPlatform("browser")] set { _versionPolicy = value; diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs index 99c604a2992449..f5d05412cbc804 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs @@ -28,7 +28,7 @@ public void Abort() WebSocket?.Abort(); } - public Task ConnectAsync(Uri uri, CancellationToken cancellationToken, ClientWebSocketOptions options) + public Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, CancellationToken cancellationToken, ClientWebSocketOptions options) { cancellationToken.ThrowIfCancellationRequested(); @@ -36,10 +36,5 @@ public Task ConnectAsync(Uri uri, CancellationToken cancellationToken, ClientWeb WebSocket = ws; return ws.ConnectAsync(uri, options.RequestedSubProtocols, cancellationToken); } - - public Task ConnectAsync(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) - { - throw new NotImplementedException(); - } } } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index aa749bdd1facb4..9b754cb761ac02 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -42,84 +42,9 @@ public void Abort() WebSocket?.Abort(); } - public async Task ConnectAsync(Uri uri, CancellationToken cancellationToken, ClientWebSocketOptions options) - { - SocketsHttpHandler handler = SetupHandler(options); - await ConnectAsyncHelper(uri, new HttpMessageInvoker(handler), cancellationToken, options).ConfigureAwait(false); - } - - public async Task ConnectAsync(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) - { - await ConnectAsyncHelper(uri, handler, cancellationToken, options).ConfigureAwait(false); - } - - private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) - { - SocketsHttpHandler? handler; - // Create the handler for this request and populate it with all of the options. - // Try to use a shared handler rather than creating a new one just for this request, if - // the options are compatible. - if (options.Credentials == null && - !options.UseDefaultCredentials && - options.Proxy == null && - options.Cookies == null && - options.RemoteCertificateValidationCallback == null && - options._clientCertificates?.Count == 0) - { - handler = s_defaultHandler; - if (handler == null) - { - handler = new SocketsHttpHandler() - { - PooledConnectionLifetime = TimeSpan.Zero, - UseProxy = false, - UseCookies = false, - }; - if (Interlocked.CompareExchange(ref s_defaultHandler, handler, null) != null) - { - handler.Dispose(); - handler = s_defaultHandler; - } - } - } - else - { - handler = new SocketsHttpHandler(); - handler.PooledConnectionLifetime = TimeSpan.Zero; - handler.CookieContainer = options.Cookies; - handler.UseCookies = options.Cookies != null; - handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback; - - if (options.UseDefaultCredentials) - { - handler.Credentials = CredentialCache.DefaultCredentials; - } - else - { - handler.Credentials = options.Credentials; - } - - if (options.Proxy == null) - { - handler.UseProxy = false; - } - else if (options.Proxy != DefaultWebProxy.Instance) - { - handler.Proxy = options.Proxy; - } - - if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection - { - Debug.Assert(handler.SslOptions.ClientCertificates == null); - handler.SslOptions.ClientCertificates = new X509Certificate2Collection(); - handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates); - } - } - return handler; - } - - private async Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + public async Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, CancellationToken cancellationToken, ClientWebSocketOptions options) { + handler ??= new HttpMessageInvoker(SetupHandler(options)); HttpResponseMessage? response = null; // TODO setup to false @@ -182,7 +107,7 @@ private async Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker handler, Cance } catch (HttpRequestException ex) { - if ( ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && request.Version.Major == 2 + if (ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && request.Version.Major == 2 && (options.Version.Major == 2 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower || options.Version.Major == 1 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { @@ -280,6 +205,71 @@ private async Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker handler, Cance } } + private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) + { + SocketsHttpHandler? handler; + // Create the handler for this request and populate it with all of the options. + // Try to use a shared handler rather than creating a new one just for this request, if + // the options are compatible. + if (options.Credentials == null && + !options.UseDefaultCredentials && + options.Proxy == null && + options.Cookies == null && + options.RemoteCertificateValidationCallback == null && + options._clientCertificates?.Count == 0) + { + handler = s_defaultHandler; + if (handler == null) + { + handler = new SocketsHttpHandler() + { + PooledConnectionLifetime = TimeSpan.Zero, + UseProxy = false, + UseCookies = false, + }; + if (Interlocked.CompareExchange(ref s_defaultHandler, handler, null) != null) + { + handler.Dispose(); + handler = s_defaultHandler; + } + } + } + else + { + handler = new SocketsHttpHandler(); + handler.PooledConnectionLifetime = TimeSpan.Zero; + handler.CookieContainer = options.Cookies; + handler.UseCookies = options.Cookies != null; + handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback; + + if (options.UseDefaultCredentials) + { + handler.Credentials = CredentialCache.DefaultCredentials; + } + else + { + handler.Credentials = options.Credentials; + } + + if (options.Proxy == null) + { + handler.UseProxy = false; + } + else if (options.Proxy != DefaultWebProxy.Instance) + { + handler.Proxy = options.Proxy; + } + + if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection + { + Debug.Assert(handler.SslOptions.ClientCertificates == null); + handler.SslOptions.ClientCertificates = new X509Certificate2Collection(); + handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates); + } + } + return handler; + } + private static WebSocketDeflateOptions ParseDeflateOptions(ReadOnlySpan extension, WebSocketDeflateOptions original) { var options = new WebSocketDeflateOptions(); From 71cc88750b91b3aa05823aa887708e708c6b907e Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Mon, 20 Jun 2022 11:33:10 +0200 Subject: [PATCH 07/32] Rename HttpVersion and HttpVersionPolicy --- .../ref/System.Net.WebSockets.Client.cs | 4 ++-- .../ClientWebSocketOptions.cs | 6 +++--- .../Net/WebSockets/ClientWebSocketOptions.cs | 6 +++--- .../Net/WebSockets/WebSocketHandle.Managed.cs | 18 +++++++++--------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs index 7247bfad69dd2d..a331e1e5a1a912 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs @@ -43,10 +43,10 @@ internal ClientWebSocketOptions() { } public System.Net.Security.RemoteCertificateValidationCallback? RemoteCertificateValidationCallback { get { throw null; } set { } } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public bool UseDefaultCredentials { get { throw null; } set { } } - public System.Version Version { get { throw null; } + public System.Version HttpVersion { get { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] set { } } - public System.Net.Http.HttpVersionPolicy VersionPolicy { get { throw null; } + public System.Net.Http.HttpVersionPolicy HttpVersionPolicy { get { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] set { } } public void AddSubProtocol(string subProtocol) { } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs index fae2b88163c3e6..1af88f4197c401 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs @@ -13,7 +13,7 @@ public sealed class ClientWebSocketOptions { private bool _isReadOnly; // After ConnectAsync is called the options cannot be modified. private List? _requestedSubProtocols; - private Version _version = HttpVersion.Version11; + private Version _version = Net.HttpVersion.Version11; private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; internal ClientWebSocketOptions() @@ -35,14 +35,14 @@ public bool UseDefaultCredentials set => throw new PlatformNotSupportedException(); } - public Version Version + public Version HttpVersion { get => _version; [UnsupportedOSPlatform("browser")] set => throw new PlatformNotSupportedException(); } - public System.Net.Http.HttpVersionPolicy VersionPolicy + public System.Net.Http.HttpVersionPolicy HttpVersionPolicy { get => _versionPolicy; [UnsupportedOSPlatform("browser")] diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index 985fcc0180a355..ca3b958954e0a9 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -26,10 +26,10 @@ public sealed class ClientWebSocketOptions internal X509CertificateCollection? _clientCertificates; internal WebHeaderCollection? _requestHeaders; internal List? _requestedSubProtocols; - private Version _version = HttpVersion.Version11; + private Version _version = Net.HttpVersion.Version11; private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; - public Version Version + public Version HttpVersion { get => _version; [UnsupportedOSPlatform("browser")] @@ -40,7 +40,7 @@ public Version Version } } - public HttpVersionPolicy VersionPolicy + public HttpVersionPolicy HttpVersionPolicy { get => _versionPolicy; [UnsupportedOSPlatform("browser")] diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 9b754cb761ac02..e7fa6c86ca18a6 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -51,13 +51,13 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, Cancellatio bool disposeHandler = true; try { - if (options.Version.Major >= 3 && options.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + if (options.HttpVersion.Major >= 3 && options.HttpVersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { throw new Exception(); } var request = new HttpRequestMessage(HttpMethod.Get, uri); - if (options.Version.Major >= 2 || (options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + if (options.HttpVersion.Major >= 2 || (options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { request.Version = new Version(2, 0); } @@ -108,8 +108,8 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, Cancellatio catch (HttpRequestException ex) { if (ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && request.Version.Major == 2 - && (options.Version.Major == 2 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrLower - || options.Version.Major == 1 && options.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + && (options.HttpVersion.Major == 2 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower + || options.HttpVersion.Major == 1 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { request.Version = new Version(1, 1); } @@ -343,17 +343,17 @@ static int ParseWindowBits(ReadOnlySpan value) /// The options controlling the request. private static void AddWebSocketHeaders(HttpRequestMessage request, string secKey, ClientWebSocketOptions options) { - request.Version = options.Version; + request.Version = options.HttpVersion; // always exact because we handle downgrade here request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - if (options.Version == HttpVersion.Version11) + if (options.HttpVersion == HttpVersion.Version11) { request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Connection, HttpKnownHeaderNames.Upgrade); request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Upgrade, "websocket"); request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKey); } - else if (options.Version == HttpVersion.Version20) + else if (options.HttpVersion == HttpVersion.Version20) { request.Method = HttpMethod.Connect; request.Headers.Protocol = "websocket"; @@ -408,7 +408,7 @@ static string GetDeflateOptions(WebSocketDeflateOptions options) private static void ValidateResponse(HttpResponseMessage response, string secValue, ClientWebSocketOptions options) { - if (options.Version == HttpVersion.Version11) + if (options.HttpVersion == HttpVersion.Version11) { if (response.StatusCode != HttpStatusCode.SwitchingProtocols) { @@ -420,7 +420,7 @@ private static void ValidateResponse(HttpResponseMessage response, string secVal ValidateHeader(response.Headers, HttpKnownHeaderNames.Upgrade, "websocket"); ValidateHeader(response.Headers, HttpKnownHeaderNames.SecWebSocketAccept, secValue); } - else if (options.Version == HttpVersion.Version20) + else if (options.HttpVersion == HttpVersion.Version20) { if (response.StatusCode != HttpStatusCode.OK) { From 8019a3cef6b6ff4b6c5dbcfc501e03e3bc14f62c Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 21 Jun 2022 11:21:30 +0200 Subject: [PATCH 08/32] Apply suggestions from code review Co-authored-by: Chris Ross --- .../src/System/Net/WebSockets/ClientWebSocket.cs | 2 +- .../src/System/Net/WebSockets/WebSocketHandle.Managed.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index 668a8b549b201f..666b21e0420058 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -91,7 +91,7 @@ private Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? invoker, Cancellati return ConnectAsyncCore(uri, invoker, cancellationToken); } - private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? sharedHandler, CancellationToken cancellationToken) + private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken) { _innerWebSocket = new WebSocketHandle(); diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index e7fa6c86ca18a6..9ecc3eae88a0fe 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -42,7 +42,7 @@ public void Abort() WebSocket?.Abort(); } - public async Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken, ClientWebSocketOptions options) { handler ??= new HttpMessageInvoker(SetupHandler(options)); HttpResponseMessage? response = null; From 8ef8b4c6263ccbc189fb608f80f22acc9dd6d346 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 21 Jun 2022 11:48:24 +0200 Subject: [PATCH 09/32] address review feedback --- .../src/System/Net/WebSockets/ClientWebSocket.cs | 2 +- .../Net/WebSockets/WebSocketHandle.Managed.cs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index 666b21e0420058..be87195ed46cd3 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -97,7 +97,7 @@ private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? invoker, Cancel try { - await _innerWebSocket.ConnectAsync(uri, sharedHandler, cancellationToken, Options).ConfigureAwait(false); + await _innerWebSocket.ConnectAsync(uri, invoker, cancellationToken, Options).ConfigureAwait(false); } catch { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 9ecc3eae88a0fe..06fbbd4480f772 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -44,7 +44,7 @@ public void Abort() public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken, ClientWebSocketOptions options) { - handler ??= new HttpMessageInvoker(SetupHandler(options)); + invoker ??= new HttpMessageInvoker(SetupHandler(options)); HttpResponseMessage? response = null; // TODO setup to false @@ -99,7 +99,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio using (linkedCancellation) { - response = await handler.SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); + response = await invoker.SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); externalAndAbortCancellation.Token.ThrowIfCancellationRequested(); // poll in case sends/receives in request/response didn't observe cancellation } ValidateResponse(response, secKeyAndSecWebSocketAccept.Value, options); @@ -200,7 +200,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio // Disposing the handler will not affect any active stream wrapped in the WebSocket. if (disposeHandler) { - handler?.Dispose(); + invoker?.Dispose(); } } } @@ -347,13 +347,13 @@ private static void AddWebSocketHeaders(HttpRequestMessage request, string secKe // always exact because we handle downgrade here request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - if (options.HttpVersion == HttpVersion.Version11) + if (request.Version == HttpVersion.Version11) { request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Connection, HttpKnownHeaderNames.Upgrade); request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Upgrade, "websocket"); request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKey); } - else if (options.HttpVersion == HttpVersion.Version20) + else if (request.Version == HttpVersion.Version20) { request.Method = HttpMethod.Connect; request.Headers.Protocol = "websocket"; @@ -408,7 +408,7 @@ static string GetDeflateOptions(WebSocketDeflateOptions options) private static void ValidateResponse(HttpResponseMessage response, string secValue, ClientWebSocketOptions options) { - if (options.HttpVersion == HttpVersion.Version11) + if (response.Version == HttpVersion.Version11) { if (response.StatusCode != HttpStatusCode.SwitchingProtocols) { @@ -420,7 +420,7 @@ private static void ValidateResponse(HttpResponseMessage response, string secVal ValidateHeader(response.Headers, HttpKnownHeaderNames.Upgrade, "websocket"); ValidateHeader(response.Headers, HttpKnownHeaderNames.SecWebSocketAccept, secValue); } - else if (options.HttpVersion == HttpVersion.Version20) + else if (response.Version == HttpVersion.Version20) { if (response.StatusCode != HttpStatusCode.OK) { From f6de72a5d6ca66cf975fa0568dbe60b2e823f4b3 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 24 Jun 2022 21:35:16 +0200 Subject: [PATCH 10/32] address review feedback --- .../Net/Http/Headers/HttpRequestHeaders.cs | 7 +-- .../src/System/Net/Http/HttpRequestMessage.cs | 2 +- .../SocketsHttpHandler/Http2Connection.cs | 2 - .../Http/SocketsHttpHandler/Http2Stream.cs | 4 ++ .../src/Resources/Strings.resx | 5 +- .../Net/WebSockets/WebSocketHandle.Managed.cs | 52 ++++++++++--------- 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index ba7d0f65a6c2f1..961a349be86d33 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -159,12 +159,7 @@ public int? MaxForwards set { SetOrRemoveParsedValue(KnownHeaders.MaxForwards.Descriptor, value); } } - public string? Protocol - { - get { return (string?)GetSingleParsedValue(new HeaderDescriptor(":protocol")); } - set { SetOrRemoveParsedValue(new HeaderDescriptor(":protocol"), value); } - } - + public string? Protocol { get; set; } public AuthenticationHeaderValue? ProxyAuthorization { 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 6c9a51d21dd04b..60f06ebfeeeffe 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 @@ -168,7 +168,7 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; - internal bool IsWebSocketH2Request() => _version.Major == 2 && HasHeaders && Headers.Protocol == "websocket"; + internal bool IsWebSocketH2Request() => _version.Major == 2 && Method == HttpMethod.Connect && HasHeaders && Headers.Protocol == "websocket"; #region IDisposable Members diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index ddff412a91917a..a6eaea33d05a1d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1458,8 +1458,6 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff WriteBytes(":protocol"u8, ref headerBuffer); Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); - - request.Headers.Protocol = null; } if (request.HasHeaders) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index fb3c6cfd11cff8..6baf3e70110b2d 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -107,6 +107,10 @@ public Http2Stream(HttpRequestMessage request, Http2Connection connection) if (_request.Content == null) { _requestCompletionState = StreamCompletionState.Completed; + if (_request.IsWebSocketH2Request()) + { + _requestBodyCancellationSource = new CancellationTokenSource(); + } } else { diff --git a/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx b/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx index 360d9f9a337e12..28dca248629b6e 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx +++ b/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx @@ -78,6 +78,9 @@ The server returned status code '{0}' when status code '101' was expected. + + The server returned status code '{0}' when status code '200' was expected. + The server's response was missing the required header '{0}'. @@ -129,4 +132,4 @@ The WebSocket failed to negotiate max client window bits. The client requested {0} but the server responded with {1}. - \ No newline at end of file + diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 52de0881b87d24..12a50a0955c660 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -44,32 +44,36 @@ public void Abort() public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken, ClientWebSocketOptions options) { - invoker ??= new HttpMessageInvoker(SetupHandler(options)); + bool disposeHandler = false; + invoker ??= new HttpMessageInvoker(SetupHandler(options, out disposeHandler)); HttpResponseMessage? response = null; - // TODO setup to false - bool disposeHandler = true; + bool tryDowngrade = false; try { - if (options.HttpVersion.Major >= 3 && options.HttpVersionPolicy != HttpVersionPolicy.RequestVersionOrLower) - { - throw new Exception(); - } - - var request = new HttpRequestMessage(HttpMethod.Get, uri); - if (options.HttpVersion.Major >= 2 || (options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) - { - request.Version = new Version(2, 0); - } - else - { - request.Version = new Version(1, 1); - } while (true) { try { + HttpRequestMessage request; + if (!tryDowngrade && options.HttpVersion == HttpVersion.Version20 + || (options.HttpVersion == HttpVersion.Version11 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + { + request = new HttpRequestMessage(HttpMethod.Connect, uri); + request.Version = new Version(2, 0); + tryDowngrade = true; + } + else if (tryDowngrade || options.HttpVersion == HttpVersion.Version11) + { + request = new HttpRequestMessage(HttpMethod.Get, uri); + request.Version = new Version(1, 1); + tryDowngrade = false; + } + else + { + throw new WebSocketException(WebSocketError.UnsupportedProtocol); + } if (options._requestHeaders?.Count > 0) // use field to avoid lazily initializing the collection { foreach (string key in options.RequestHeaders) @@ -107,13 +111,11 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio } catch (HttpRequestException ex) { - if (ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && request.Version.Major == 2 - && (options.HttpVersion.Major == 2 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower - || options.HttpVersion.Major == 1 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) + if (!ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || !tryDowngrade + || (options.HttpVersion != HttpVersion.Version11 && options.HttpVersionPolicy != HttpVersionPolicy.RequestVersionOrLower)) { - request.Version = new Version(1, 1); + throw ex; } - else { throw ex; } } } @@ -205,7 +207,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio } } - private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) + private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, out bool disposeHandler) { SocketsHttpHandler? handler; // Create the handler for this request and populate it with all of the options. @@ -218,6 +220,7 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) options.RemoteCertificateValidationCallback == null && options._clientCertificates?.Count == 0) { + disposeHandler = false; handler = s_defaultHandler; if (handler == null) { @@ -236,6 +239,7 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options) } else { + disposeHandler = true; handler = new SocketsHttpHandler(); handler.PooledConnectionLifetime = TimeSpan.Zero; handler.CookieContainer = options.Cookies; @@ -424,7 +428,7 @@ private static void ValidateResponse(HttpResponseMessage response, string secVal { if (response.StatusCode != HttpStatusCode.OK) { - throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); + throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect200Expected, (int)response.StatusCode)); } } From 92214831e45dfda7cd6b1a7096b5cbb98af5c00b Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 28 Jun 2022 15:18:33 +0200 Subject: [PATCH 11/32] address review feedback --- .../SocketsHttpHandler/Http2Connection.cs | 14 ++++++------ .../Net/WebSockets/WebSocketHandle.Managed.cs | 22 ++++++++++++------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index a6eaea33d05a1d..4b52277963151e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1453,15 +1453,15 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff WriteIndexedHeader(H2StaticTable.PathSlash, pathAndQuery, ref headerBuffer); } - if (request.HasHeaders && request.Headers.Protocol != null) - { - WriteBytes(":protocol"u8, ref headerBuffer); - Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); - WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); - } - if (request.HasHeaders) { + if (request.Headers.Protocol != null) + { + WriteBytes(":protocol"u8, ref headerBuffer); + Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); + WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); + } + WriteHeaderCollection(request, request.Headers, ref headerBuffer); } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 12a50a0955c660..70be6234dbca8e 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -82,9 +82,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio } } - // Create the security key and expected response, then build all of the request headers - KeyValuePair secKeyAndSecWebSocketAccept = CreateSecKeyAndSecWebSocketAccept(); - AddWebSocketHeaders(request, secKeyAndSecWebSocketAccept.Key, options); + string? secValue = AddWebSocketHeaders(request, options); // Issue the request. The response must be status code 101. CancellationTokenSource? linkedCancellation; @@ -106,7 +104,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio response = await invoker.SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); externalAndAbortCancellation.Token.ThrowIfCancellationRequested(); // poll in case sends/receives in request/response didn't observe cancellation } - ValidateResponse(response, secKeyAndSecWebSocketAccept.Value, options); + ValidateResponse(response, secValue, options); break; } catch (HttpRequestException ex) @@ -343,19 +341,22 @@ static int ParseWindowBits(ReadOnlySpan value) /// Adds the necessary headers for the web socket request. /// The request to which the headers should be added. - /// The generated security key to send in the Sec-WebSocket-Key header. /// The options controlling the request. - private static void AddWebSocketHeaders(HttpRequestMessage request, string secKey, ClientWebSocketOptions options) + private static string? AddWebSocketHeaders(HttpRequestMessage request, ClientWebSocketOptions options) { request.Version = options.HttpVersion; // always exact because we handle downgrade here request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + string? secValue = null; if (request.Version == HttpVersion.Version11) { + // Create the security key and expected response, then build all of the request headers + KeyValuePair secKeyAndSecWebSocketAccept = CreateSecKeyAndSecWebSocketAccept(); + secValue = secKeyAndSecWebSocketAccept.Value; request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Connection, HttpKnownHeaderNames.Upgrade); request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Upgrade, "websocket"); - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKey); + request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketKey, secKeyAndSecWebSocketAccept.Key); } else if (request.Version == HttpVersion.Version20) { @@ -408,10 +409,13 @@ static string GetDeflateOptions(WebSocketDeflateOptions options) return builder.ToString(); } } + return secValue; } - private static void ValidateResponse(HttpResponseMessage response, string secValue, ClientWebSocketOptions options) + private static void ValidateResponse(HttpResponseMessage response, string? secValue, ClientWebSocketOptions options) { + Debug.Assert(response.Version == HttpVersion.Version11 || response.Version == HttpVersion.Version20); + if (response.Version == HttpVersion.Version11) { if (response.StatusCode != HttpStatusCode.SwitchingProtocols) @@ -419,6 +423,8 @@ private static void ValidateResponse(HttpResponseMessage response, string secVal throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); } + Debug.Assert(secValue != null); + // The Connection, Upgrade, and SecWebSocketAccept headers are required and with specific values. ValidateHeader(response.Headers, HttpKnownHeaderNames.Connection, "Upgrade"); ValidateHeader(response.Headers, HttpKnownHeaderNames.Upgrade, "websocket"); From fafc84b7b432b3940c118378361625fc2417075f Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 28 Jun 2022 15:21:51 +0200 Subject: [PATCH 12/32] Apply suggestions from code review Co-authored-by: Chris Ross --- .../src/System/Net/WebSockets/WebSocketHandle.Browser.cs | 2 +- .../src/System/Net/WebSockets/WebSocketHandle.Managed.cs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs index f5d05412cbc804..2addc85ea5aed1 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Browser.cs @@ -28,7 +28,7 @@ public void Abort() WebSocket?.Abort(); } - public Task ConnectAsync(Uri uri, HttpMessageInvoker? handler, CancellationToken cancellationToken, ClientWebSocketOptions options) + public Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken, ClientWebSocketOptions options) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 70be6234dbca8e..28308e67fb7adf 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -61,13 +61,13 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio || (options.HttpVersion == HttpVersion.Version11 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { request = new HttpRequestMessage(HttpMethod.Connect, uri); - request.Version = new Version(2, 0); + request.Version = HttpVersion.Version20; tryDowngrade = true; } else if (tryDowngrade || options.HttpVersion == HttpVersion.Version11) { request = new HttpRequestMessage(HttpMethod.Get, uri); - request.Version = new Version(1, 1); + request.Version = HttpVersion.Version11; tryDowngrade = false; } else @@ -344,7 +344,6 @@ static int ParseWindowBits(ReadOnlySpan value) /// The options controlling the request. private static string? AddWebSocketHeaders(HttpRequestMessage request, ClientWebSocketOptions options) { - request.Version = options.HttpVersion; // always exact because we handle downgrade here request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; string? secValue = null; @@ -360,7 +359,6 @@ static int ParseWindowBits(ReadOnlySpan value) } else if (request.Version == HttpVersion.Version20) { - request.Method = HttpMethod.Connect; request.Headers.Protocol = "websocket"; request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Origin, request.Headers.Host); } From 0d8f8f7373830e2eb3ddb3bc9ef96955766f6ba3 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Wed, 29 Jun 2022 18:21:38 +0200 Subject: [PATCH 13/32] fix race condition on setting enable connect --- .../SocketsHttpHandler/Http2Connection.cs | 20 ++++++++++++++----- .../SocketsHttpHandler/HttpConnectionPool.cs | 8 +++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 4b52277963151e..f488398d564121 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -792,6 +792,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f // Parse settings and process the ones we care about. ReadOnlySpan settings = _incomingBuffer.ActiveSpan.Slice(0, frameHeader.PayloadLength); bool maxConcurrentStreamsReceived = false; + bool enableConnectReceived = false; while (settings.Length > 0) { Debug.Assert((settings.Length % 6) == 0); @@ -829,8 +830,9 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f case SettingId.EnableConnect: if (settingValue == 1) { - IsConnectEnabled = true; + IsConnectEnabled.SetResult(true); } + enableConnectReceived = true; break; default: @@ -840,10 +842,18 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f } } - if (initialFrame && !maxConcurrentStreamsReceived) + if (initialFrame) { - // Set to 'infinite' because MaxConcurrentStreams was not set on the initial SETTINGS frame. - ChangeMaxConcurrentStreams(int.MaxValue); + if (!maxConcurrentStreamsReceived) + { + // Set to 'infinite' because MaxConcurrentStreams was not set on the initial SETTINGS frame. + ChangeMaxConcurrentStreams(int.MaxValue); + } + + if (!enableConnectReceived) + { + IsConnectEnabled.SetResult(false); + } } _incomingBuffer.Discard(frameHeader.PayloadLength); @@ -1906,7 +1916,7 @@ private enum SettingId : ushort EnableConnect = 0x8 } - internal bool IsConnectEnabled { get; private set; } + internal TaskCompletionSource IsConnectEnabled { get; private set; } = new TaskCompletionSource(); // Note that this is safe to be called concurrently by multiple threads. 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 7056360462c522..ad3916609df6dc 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 @@ -1022,16 +1022,14 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn Debug.Assert(connection is not null || !_http2Enabled); if (connection is not null) { - if (!request.IsWebSocketH2Request() || connection.IsConnectEnabled) - { - response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); - } - else if (request.IsWebSocketH2Request() && !connection.IsConnectEnabled) + if (request.IsWebSocketH2Request() && !await connection.IsConnectEnabled.Task.ConfigureAwait(false)) { HttpRequestException exception = new("Extended CONNECT is not supported"); exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; throw exception; } + + response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); } } From 9be5b959d08131430fec2919c34a7c88b9144200 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Wed, 29 Jun 2022 23:21:58 +0200 Subject: [PATCH 14/32] inherit h2 read and writes streams --- .../Http/SocketsHttpHandler/Http2Stream.cs | 141 ++---------------- 1 file changed, 10 insertions(+), 131 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 6baf3e70110b2d..942663608282f5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -1432,134 +1432,40 @@ private enum StreamCompletionState : byte Failed } - private sealed class Http2ReadStream : HttpBaseStream + private sealed class Http2ReadStream : Http2ReadWriteStream { - private Http2Stream? _http2Stream; - private readonly HttpResponseMessage _responseMessage; - - public Http2ReadStream(Http2Stream http2Stream) - { - Debug.Assert(http2Stream != null); - Debug.Assert(http2Stream._response != null); - _http2Stream = http2Stream; - _responseMessage = _http2Stream._response; - } - - ~Http2ReadStream() - { - if (NetEventSource.Log.IsEnabled()) _http2Stream?.Trace(""); - try - { - Dispose(disposing: false); - } - catch (Exception e) - { - if (NetEventSource.Log.IsEnabled()) _http2Stream?.Trace($"Error: {e}"); - } - } - - protected override void Dispose(bool disposing) - { - Http2Stream? http2Stream = Interlocked.Exchange(ref _http2Stream, null); - if (http2Stream == null) - { - return; - } - - // Technically we shouldn't be doing the following work when disposing == false, - // as the following work relies on other finalizable objects. But given the HTTP/2 - // protocol, we have little choice: if someone drops the Http2ReadStream without - // disposing of it, we need to a) signal to the server that the stream is being - // canceled, and b) clean up the associated state in the Http2Connection. - - http2Stream.CloseResponseBody(); - - base.Dispose(disposing); - } + public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) { } - public override bool CanRead => _http2Stream != null; public override bool CanWrite => false; - public override int Read(Span destination) - { - Http2Stream http2Stream = _http2Stream ?? throw new ObjectDisposedException(nameof(Http2ReadStream)); - - return http2Stream.ReadData(destination, _responseMessage); - } - - public override ValueTask ReadAsync(Memory destination, CancellationToken cancellationToken) - { - Http2Stream? http2Stream = _http2Stream; - - if (http2Stream == null) - { - return ValueTask.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream)))); - } - - if (cancellationToken.IsCancellationRequested) - { - return ValueTask.FromCanceled(cancellationToken); - } - - return http2Stream.ReadDataAsync(destination, _responseMessage, cancellationToken); - } - - public override void CopyTo(Stream destination, int bufferSize) - { - ValidateCopyToArguments(destination, bufferSize); - Http2Stream http2Stream = _http2Stream ?? throw ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream))); - http2Stream.CopyTo(_responseMessage, destination, bufferSize); - } - - public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) - { - ValidateCopyToArguments(destination, bufferSize); - Http2Stream? http2Stream = _http2Stream; - return - http2Stream is null ? Task.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(Http2ReadStream)))) : - cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : - http2Stream.CopyToAsync(_responseMessage, destination, bufferSize, cancellationToken); - } - public override void Write(ReadOnlySpan buffer) => throw new NotSupportedException(SR.net_http_content_readonly_stream); public override ValueTask WriteAsync(ReadOnlyMemory destination, CancellationToken cancellationToken) => throw new NotSupportedException(); } - private sealed class Http2WriteStream : HttpBaseStream + private sealed class Http2WriteStream : Http2ReadWriteStream { - private Http2Stream? _http2Stream; public long BytesWritten { get; private set; } public long ContentLength { get; private set; } - public Http2WriteStream(Http2Stream http2Stream, long contentLength) + public Http2WriteStream(Http2Stream http2Stream, long contentLength) : base(http2Stream) { - Debug.Assert(http2Stream != null); Debug.Assert(contentLength >= -1); - _http2Stream = http2Stream; ContentLength = contentLength; } - protected override void Dispose(bool disposing) - { - Http2Stream? http2Stream = Interlocked.Exchange(ref _http2Stream, null); - if (http2Stream == null) - { - return; - } - - base.Dispose(disposing); - } - public override bool CanRead => false; - public override bool CanWrite => _http2Stream != null; public override int Read(Span buffer) => throw new NotSupportedException(); public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => throw new NotSupportedException(); + public override void CopyTo(Stream destination, int bufferSize) => throw new NotSupportedException(); + + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => throw new NotSupportedException(); + public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) { BytesWritten += buffer.Length; @@ -1569,38 +1475,11 @@ public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationTo return ValueTask.FromException(new HttpRequestException(SR.net_http_content_write_larger_than_content_length)); } - Http2Stream? http2Stream = _http2Stream; - - if (http2Stream == null) - { - return ValueTask.FromException(new ObjectDisposedException(nameof(Http2WriteStream))); - } - - return http2Stream.SendDataAsync(buffer, cancellationToken); - } - - public override Task FlushAsync(CancellationToken cancellationToken) - { - if (cancellationToken.IsCancellationRequested) - { - return Task.FromCanceled(cancellationToken); - } - - Http2Stream? http2Stream = _http2Stream; - - if (http2Stream == null) - { - return Task.CompletedTask; - } - - // In order to flush this stream's previous writes, we need to flush the connection. We - // really only need to do any work here if the connection's buffer has any pending writes - // from this stream, but we currently lack a good/efficient/safe way of doing that. - return http2Stream._connection.FlushAsync(cancellationToken); + return base.WriteAsync(buffer, cancellationToken); } } - public sealed class Http2ReadWriteStream : HttpBaseStream + public class Http2ReadWriteStream : HttpBaseStream { private Http2Stream? _http2Stream; // should be removed From 502b051b2e01716227a84c24af1e0e8762e0ea31 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 30 Jun 2022 16:13:07 +0200 Subject: [PATCH 15/32] Apply suggestions from code review Co-authored-by: Chris Ross --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index f488398d564121..ba3af6d95db9f7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -830,7 +830,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f case SettingId.EnableConnect: if (settingValue == 1) { - IsConnectEnabled.SetResult(true); + IsConnectEnabled.TrySetResult(true); } enableConnectReceived = true; break; @@ -850,10 +850,8 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f ChangeMaxConcurrentStreams(int.MaxValue); } - if (!enableConnectReceived) - { - IsConnectEnabled.SetResult(false); - } + // Set a default value if it wasn't enabled above. + IsConnectEnabled.TrySetResult(false); } _incomingBuffer.Discard(frameHeader.PayloadLength); @@ -1916,7 +1914,7 @@ private enum SettingId : ushort EnableConnect = 0x8 } - internal TaskCompletionSource IsConnectEnabled { get; private set; } = new TaskCompletionSource(); + internal TaskCompletionSource IsConnectEnabled { get; private set; } = new TaskCompletionSource(TaskCompletionOptions.RunContinuationsAsynchronously); // Note that this is safe to be called concurrently by multiple threads. From e63543975fd014ea795799e87536545296bb44a4 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 30 Jun 2022 23:03:28 +0200 Subject: [PATCH 16/32] fix H2PACK encoding issue --- .../System/Net/Http/SocketsHttpHandler/Http2Connection.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index ba3af6d95db9f7..9312fe74d4afb1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -792,7 +792,6 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f // Parse settings and process the ones we care about. ReadOnlySpan settings = _incomingBuffer.ActiveSpan.Slice(0, frameHeader.PayloadLength); bool maxConcurrentStreamsReceived = false; - bool enableConnectReceived = false; while (settings.Length > 0) { Debug.Assert((settings.Length % 6) == 0); @@ -832,7 +831,6 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f { IsConnectEnabled.TrySetResult(true); } - enableConnectReceived = true; break; default: @@ -1465,7 +1463,7 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff { if (request.Headers.Protocol != null) { - WriteBytes(":protocol"u8, ref headerBuffer); + WriteBytes(HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewNameToAllocatedArray(":protocol"), ref headerBuffer); Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); } @@ -1914,7 +1912,7 @@ private enum SettingId : ushort EnableConnect = 0x8 } - internal TaskCompletionSource IsConnectEnabled { get; private set; } = new TaskCompletionSource(TaskCompletionOptions.RunContinuationsAsynchronously); + internal TaskCompletionSourceWithCancellation IsConnectEnabled { get; private set; } = new TaskCompletionSourceWithCancellation(); // Note that this is safe to be called concurrently by multiple threads. From ad6685770d506f20c68672215c3b2bb9f0a81f9e Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 30 Jun 2022 23:04:53 +0200 Subject: [PATCH 17/32] add timeout for waiting settings task --- .../Http/SocketsHttpHandler/HttpConnectionPool.cs | 13 +++++++++---- 1 file changed, 9 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 ad3916609df6dc..8d64bc000ae6d9 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 @@ -1022,11 +1022,16 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn Debug.Assert(connection is not null || !_http2Enabled); if (connection is not null) { - if (request.IsWebSocketH2Request() && !await connection.IsConnectEnabled.Task.ConfigureAwait(false)) + if (request.IsWebSocketH2Request()) { - HttpRequestException exception = new("Extended CONNECT is not supported"); - exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; - throw exception; + int settingsTimeoutInMilliseconds = 5 * 60 * 1000; + CancellationTokenSource cts = new CancellationTokenSource(settingsTimeoutInMilliseconds); + if (!await connection.IsConnectEnabled.WaitWithCancellationAsync(cts.Token).ConfigureAwait(false)) + { + HttpRequestException exception = new("Extended CONNECT is not supported"); + exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; + throw exception; + } } response = await connection.SendAsync(request, async, cancellationToken).ConfigureAwait(false); From c58d99364ac38f5ed74f10c183f8a52943f1e36f Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Thu, 30 Jun 2022 23:20:46 +0200 Subject: [PATCH 18/32] generalized settings received task --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 9 ++++----- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 9312fe74d4afb1..48a1d07f4b01a6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -8,7 +8,6 @@ using System.IO; using System.Net.Http.Headers; using System.Net.Http.HPack; -using System.Net.Security; using System.Runtime.CompilerServices; using System.Text; using System.Threading; @@ -829,7 +828,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f case SettingId.EnableConnect: if (settingValue == 1) { - IsConnectEnabled.TrySetResult(true); + IsConnectEnabled = true; } break; @@ -848,8 +847,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f ChangeMaxConcurrentStreams(int.MaxValue); } - // Set a default value if it wasn't enabled above. - IsConnectEnabled.TrySetResult(false); + InitialSettingsReceived.TrySetResult(true); } _incomingBuffer.Discard(frameHeader.PayloadLength); @@ -1912,7 +1910,8 @@ private enum SettingId : ushort EnableConnect = 0x8 } - internal TaskCompletionSourceWithCancellation IsConnectEnabled { get; private set; } = new TaskCompletionSourceWithCancellation(); + internal TaskCompletionSourceWithCancellation InitialSettingsReceived { get; private set; } = new TaskCompletionSourceWithCancellation(); + internal bool IsConnectEnabled { get; private set; } // Note that this is safe to be called concurrently by multiple threads. 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 8d64bc000ae6d9..45bb2f3e597711 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 @@ -1026,7 +1026,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { int settingsTimeoutInMilliseconds = 5 * 60 * 1000; CancellationTokenSource cts = new CancellationTokenSource(settingsTimeoutInMilliseconds); - if (!await connection.IsConnectEnabled.WaitWithCancellationAsync(cts.Token).ConfigureAwait(false)) + if (await connection.InitialSettingsReceived.WaitWithCancellationAsync(cts.Token).ConfigureAwait(false) && !connection.IsConnectEnabled) { HttpRequestException exception = new("Extended CONNECT is not supported"); exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; From 76c9e20a549aaec0045372f103e21318a6b442a8 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 1 Jul 2022 11:39:25 +0200 Subject: [PATCH 19/32] Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs Co-authored-by: Chris Ross --- .../System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 4 +--- 1 file changed, 1 insertion(+), 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 45bb2f3e597711..91daee19aa0887 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 @@ -1024,9 +1024,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { if (request.IsWebSocketH2Request()) { - int settingsTimeoutInMilliseconds = 5 * 60 * 1000; - CancellationTokenSource cts = new CancellationTokenSource(settingsTimeoutInMilliseconds); - if (await connection.InitialSettingsReceived.WaitWithCancellationAsync(cts.Token).ConfigureAwait(false) && !connection.IsConnectEnabled) + if (await connection.InitialSettingsReceived.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false) && !connection.IsConnectEnabled) { HttpRequestException exception = new("Extended CONNECT is not supported"); exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; From 24674490cfa547c6f40d2d4e2903e4a3f58f0967 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 1 Jul 2022 20:36:45 +0200 Subject: [PATCH 20/32] Fixing HttpStream tests --- .../Net/Http/SocketsHttpHandler/Http2Stream.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 942663608282f5..2d9532c982e08e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -11,6 +11,7 @@ using System.Runtime.ExceptionServices; using System.Text; using System.Threading; +using System.Threading.Channels; using System.Threading.Tasks; using System.Threading.Tasks.Sources; @@ -1434,7 +1435,10 @@ private enum StreamCompletionState : byte private sealed class Http2ReadStream : Http2ReadWriteStream { - public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) { } + public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) + { + base.CloseResponseBody = true; + } public override bool CanWrite => false; @@ -1482,9 +1486,8 @@ public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationTo public class Http2ReadWriteStream : HttpBaseStream { private Http2Stream? _http2Stream; - // should be removed private readonly HttpResponseMessage _responseMessage; - // need to handle data flow + protected bool CloseResponseBody { get; set; } public Http2ReadWriteStream(Http2Stream http2Stream) { @@ -1520,8 +1523,10 @@ protected override void Dispose(bool disposing) // protocol, we have little choice: if someone drops the Http2ReadStream without // disposing of it, we need to a) signal to the server that the stream is being // canceled, and b) clean up the associated state in the Http2Connection. - - http2Stream.CloseResponseBody(); + if (CloseResponseBody) + { + http2Stream.CloseResponseBody(); + } base.Dispose(disposing); } From d9248a6e8f62096e8ff51e23eaa2b02a1edf1031 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Mon, 4 Jul 2022 11:38:33 +0200 Subject: [PATCH 21/32] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs | 7 +++---- .../WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs | 6 ++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 2d9532c982e08e..bbac4fbebabc54 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -636,7 +636,7 @@ private void OnStatus(int statusCode) } else { - if (_response.RequestMessage != null && _response.RequestMessage.IsWebSocketH2Request() && statusCode == 200) + if (statusCode == 200 && _response.RequestMessage!.IsWebSocketH2Request()) { _webSocketEstablished = true; } @@ -1444,15 +1444,14 @@ public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) public override void Write(ReadOnlySpan buffer) => throw new NotSupportedException(SR.net_http_content_readonly_stream); - public override ValueTask WriteAsync(ReadOnlyMemory destination, CancellationToken cancellationToken) => throw new NotSupportedException(); + public override ValueTask WriteAsync(ReadOnlyMemory destination, CancellationToken cancellationToken) => throw new NotSupportedException(SR.net_http_content_readonly_stream); } private sealed class Http2WriteStream : Http2ReadWriteStream { - public long BytesWritten { get; private set; } - public long ContentLength { get; private set; } + public long ContentLength { get; } public Http2WriteStream(Http2Stream http2Stream, long contentLength) : base(http2Stream) { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs index 1af88f4197c401..e01b4fcf46a876 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/BrowserWebSockets/ClientWebSocketOptions.cs @@ -13,8 +13,6 @@ public sealed class ClientWebSocketOptions { private bool _isReadOnly; // After ConnectAsync is called the options cannot be modified. private List? _requestedSubProtocols; - private Version _version = Net.HttpVersion.Version11; - private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; internal ClientWebSocketOptions() { } @@ -37,14 +35,14 @@ public bool UseDefaultCredentials public Version HttpVersion { - get => _version; + get => Net.HttpVersion.Version11; [UnsupportedOSPlatform("browser")] set => throw new PlatformNotSupportedException(); } public System.Net.Http.HttpVersionPolicy HttpVersionPolicy { - get => _versionPolicy; + get => HttpVersionPolicy.RequestVersionOrLower; [UnsupportedOSPlatform("browser")] set => throw new PlatformNotSupportedException(); } From baf52d4fa25772c9dab810f21045d64deb6745dd Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Mon, 4 Jul 2022 21:34:40 +0200 Subject: [PATCH 22/32] address review feedback --- src/libraries/System.Net.Http/src/Resources/Strings.resx | 3 +++ .../src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs | 7 ++++--- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 20a30c23e9a9f8..7040cc9875abc8 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -564,4 +564,7 @@ The HTTP/1.1 response chunk was too large. + + Extended CONNECT is not supported. + \ No newline at end of file diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index bbac4fbebabc54..770bffdda741ca 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -1437,7 +1437,7 @@ private sealed class Http2ReadStream : Http2ReadWriteStream { public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) { - base.CloseResponseBody = true; + base.CloseResponseBodyOnDispose = true; } public override bool CanWrite => false; @@ -1486,7 +1486,6 @@ public class Http2ReadWriteStream : HttpBaseStream { private Http2Stream? _http2Stream; private readonly HttpResponseMessage _responseMessage; - protected bool CloseResponseBody { get; set; } public Http2ReadWriteStream(Http2Stream http2Stream) { @@ -1509,6 +1508,8 @@ public Http2ReadWriteStream(Http2Stream http2Stream) } } + protected bool CloseResponseBodyOnDispose { get; set; } + protected override void Dispose(bool disposing) { Http2Stream? http2Stream = Interlocked.Exchange(ref _http2Stream, null); @@ -1522,7 +1523,7 @@ protected override void Dispose(bool disposing) // protocol, we have little choice: if someone drops the Http2ReadStream without // disposing of it, we need to a) signal to the server that the stream is being // canceled, and b) clean up the associated state in the Http2Connection. - if (CloseResponseBody) + if (CloseResponseBodyOnDispose) { http2Stream.CloseResponseBody(); } 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 91daee19aa0887..43e77b3a7f80b7 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 @@ -1026,7 +1026,7 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { if (await connection.InitialSettingsReceived.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false) && !connection.IsConnectEnabled) { - HttpRequestException exception = new("Extended CONNECT is not supported"); + HttpRequestException exception = new(SR.net_unsupported_extended_connect); exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; throw exception; } From afaf5becf7661355df5c79344094d3b1a086bbac Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 8 Jul 2022 13:47:54 +0200 Subject: [PATCH 23/32] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../SocketsHttpHandler/Http2Connection.cs | 20 +++++++++++++++++- .../Http/SocketsHttpHandler/Http2Stream.cs | 6 +++--- .../Net/WebSockets/ClientWebSocketOptions.cs | 5 +---- .../Net/WebSockets/WebSocketHandle.Managed.cs | 21 ++++++++----------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 48a1d07f4b01a6..c301c3d43ccecd 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -847,6 +847,10 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f ChangeMaxConcurrentStreams(int.MaxValue); } + if (_initialSettingsReceived is null) + { + Interlocked.CompareExchange(_initialSettingsReceived, s_settingsReceivedSingleton, null); + } InitialSettingsReceived.TrySetResult(true); } @@ -1910,7 +1914,21 @@ private enum SettingId : ushort EnableConnect = 0x8 } - internal TaskCompletionSourceWithCancellation InitialSettingsReceived { get; private set; } = new TaskCompletionSourceWithCancellation(); + private static readonly TaskCompletionSourceWithCancellation s_settingsReceivedSingleton = CreateSuccessfullyCompleted(); + + internal TaskCompletionSourceWithCancellation InitialSettingsReceived => + _initialSettingsReceived ?? + Interlocked.CompareExchange(ref _initialSettingsReceived, new(), null) ?? + _initialSettingsReceived; + + private TaskCompletionSourceWithCancellation? _initialSettingsReceived; + + private static TaskCompletionSourceWithCancellation CreateSuccessfullyCompleted() + { + var tcs = new TaskCompletionSourceWithCancellation(); + tcs.TrySetResult(true); + return tcs; + } internal bool IsConnectEnabled { get; private set; } // Note that this is safe to be called concurrently by multiple threads. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 770bffdda741ca..c6e0afb6a8b509 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -1444,7 +1444,7 @@ public Http2ReadStream(Http2Stream http2Stream) : base(http2Stream) public override void Write(ReadOnlySpan buffer) => throw new NotSupportedException(SR.net_http_content_readonly_stream); - public override ValueTask WriteAsync(ReadOnlyMemory destination, CancellationToken cancellationToken) => throw new NotSupportedException(SR.net_http_content_readonly_stream); + public override ValueTask WriteAsync(ReadOnlyMemory destination, CancellationToken cancellationToken) => ValueTask.FromException(new NotSupportedException(SR.net_http_content_readonly_stream)); } private sealed class Http2WriteStream : Http2ReadWriteStream @@ -1463,11 +1463,11 @@ public Http2WriteStream(Http2Stream http2Stream, long contentLength) : base(http public override int Read(Span buffer) => throw new NotSupportedException(); - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => throw new NotSupportedException(); + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => ValueTask.FromException(new NotSupportedException()); public override void CopyTo(Stream destination, int bufferSize) => throw new NotSupportedException(); - public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => throw new NotSupportedException(); + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => Task.FromException(new NotSupportedException()); public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index ca3b958954e0a9..8e02a7fb80ce9c 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -44,10 +44,7 @@ public HttpVersionPolicy HttpVersionPolicy { get => _versionPolicy; [UnsupportedOSPlatform("browser")] - set - { - _versionPolicy = value; - } + set => _versionPolicy = value; } internal ClientWebSocketOptions() { } // prevent external instantiation diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 28308e67fb7adf..3768307a84edb9 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -60,20 +60,19 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio if (!tryDowngrade && options.HttpVersion == HttpVersion.Version20 || (options.HttpVersion == HttpVersion.Version11 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { - request = new HttpRequestMessage(HttpMethod.Connect, uri); - request.Version = HttpVersion.Version20; + request = new HttpRequestMessage(HttpMethod.Connect, uri) { Version = HttpVersion.Version20 }; tryDowngrade = true; } else if (tryDowngrade || options.HttpVersion == HttpVersion.Version11) { - request = new HttpRequestMessage(HttpMethod.Get, uri); - request.Version = HttpVersion.Version11; + request = new HttpRequestMessage(HttpMethod.Get, uri) { Version = HttpVersion.Version11 }; tryDowngrade = false; } else { throw new WebSocketException(WebSocketError.UnsupportedProtocol); } + if (options._requestHeaders?.Count > 0) // use field to avoid lazily initializing the collection { foreach (string key in options.RequestHeaders) @@ -104,6 +103,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio response = await invoker.SendAsync(request, externalAndAbortCancellation.Token).ConfigureAwait(false); externalAndAbortCancellation.Token.ThrowIfCancellationRequested(); // poll in case sends/receives in request/response didn't observe cancellation } + ValidateResponse(response, secValue, options); break; } @@ -208,6 +208,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, out bool disposeHandler) { SocketsHttpHandler? handler; + // Create the handler for this request and populate it with all of the options. // Try to use a shared handler rather than creating a new one just for this request, if // the options are compatible. @@ -244,14 +245,9 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, o handler.UseCookies = options.Cookies != null; handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback; - if (options.UseDefaultCredentials) - { - handler.Credentials = CredentialCache.DefaultCredentials; - } - else - { - handler.Credentials = options.Credentials; - } + handler.Credentials = options.UseDefaultCredentials ? + CredentialCache.DefaultCredentials : + options.Credentials; if (options.Proxy == null) { @@ -269,6 +265,7 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, o handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates); } } + return handler; } From 918c6a8982d1f36faf666d68b2d88cbfc2221927 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 8 Jul 2022 15:04:05 +0200 Subject: [PATCH 24/32] Address review feedback --- .../System.Net.Http/src/Resources/Strings.resx | 5 ++++- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 9 ++++++--- .../System/Net/Http/SocketsHttpHandler/Http2Stream.cs | 9 ++++----- .../System/Net/WebSockets/WebSocketHandle.Managed.cs | 10 ++++------ 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 7040cc9875abc8..d2220b38f6532e 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -168,6 +168,9 @@ The stream does not support writing. + + The stream does not support reading. + The character set provided in ContentType is invalid. Cannot read content as string using an invalid character set. @@ -567,4 +570,4 @@ Extended CONNECT is not supported. - \ No newline at end of file + diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index c301c3d43ccecd..7496d561a8d530 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -18,6 +18,8 @@ namespace System.Net.Http { internal sealed partial class Http2Connection : HttpConnectionBase { + private static ReadOnlySpan ProtocolLiteralHeaderBytes => new byte[] { 0x0, 0x9, 0x3a, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x63, 0x6f, 0x6c }; + private readonly HttpConnectionPool _pool; private readonly Stream _stream; @@ -849,7 +851,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f if (_initialSettingsReceived is null) { - Interlocked.CompareExchange(_initialSettingsReceived, s_settingsReceivedSingleton, null); + Interlocked.CompareExchange(ref _initialSettingsReceived, s_settingsReceivedSingleton, null); } InitialSettingsReceived.TrySetResult(true); } @@ -1465,7 +1467,8 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff { if (request.Headers.Protocol != null) { - WriteBytes(HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewNameToAllocatedArray(":protocol"), ref headerBuffer); + HttpHeaders.CheckContainsNewLine(request.Headers.Protocol); + WriteBytes(ProtocolLiteralHeaderBytes, ref headerBuffer); Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); } @@ -1922,7 +1925,7 @@ private enum SettingId : ushort _initialSettingsReceived; private TaskCompletionSourceWithCancellation? _initialSettingsReceived; - + private static TaskCompletionSourceWithCancellation CreateSuccessfullyCompleted() { var tcs = new TaskCompletionSourceWithCancellation(); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index c6e0afb6a8b509..52bdd07d0ba344 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -537,7 +537,6 @@ void IHttpStreamHeadersHandler.OnStaticIndexedHeader(int index) if (index <= LastHPackRequestPseudoHeaderId) { - // add protocol and others pseudoheaders if (NetEventSource.Log.IsEnabled()) Trace($"Invalid request pseudo-header ID {index}."); throw new HttpRequestException(SR.net_http_invalid_response); } @@ -1461,13 +1460,13 @@ public Http2WriteStream(Http2Stream http2Stream, long contentLength) : base(http public override bool CanRead => false; - public override int Read(Span buffer) => throw new NotSupportedException(); + public override int Read(Span buffer) => throw new NotSupportedException(SR.net_http_content_writeonly_stream); - public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => ValueTask.FromException(new NotSupportedException()); + public override ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken) => ValueTask.FromException(new NotSupportedException(SR.net_http_content_writeonly_stream)); - public override void CopyTo(Stream destination, int bufferSize) => throw new NotSupportedException(); + public override void CopyTo(Stream destination, int bufferSize) => throw new NotSupportedException(SR.net_http_content_writeonly_stream); - public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => Task.FromException(new NotSupportedException()); + public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken) => Task.FromException(new NotSupportedException(SR.net_http_content_writeonly_stream)); public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken) { diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 3768307a84edb9..dbdc6a551c5895 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -107,13 +107,11 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio ValidateResponse(response, secValue, options); break; } - catch (HttpRequestException ex) + catch (HttpRequestException ex) when + (ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && + tryDowngrade && + (options.HttpVersion == HttpVersion.Version11 || options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower)) { - if (!ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || !tryDowngrade - || (options.HttpVersion != HttpVersion.Version11 && options.HttpVersionPolicy != HttpVersionPolicy.RequestVersionOrLower)) - { - throw ex; - } } } From 97340756a14d574856dde61b39c20205fb1499bd Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Fri, 8 Jul 2022 18:48:45 +0200 Subject: [PATCH 25/32] Adapt test to ValueTask.FromException --- .../Common/tests/System/Net/Http/HttpClientHandlerTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 87fac367760f2d..1592265729f804 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -1031,7 +1031,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => Assert.Equal(PlatformDetection.IsBrowser && !enableWasmStreaming, responseStream.CanSeek); // Not supported operations - Assert.Throws(() => responseStream.BeginWrite(new byte[1], 0, 1, null, null)); + await Assert.ThrowsAsync(async () => await Task.Factory.FromAsync(responseStream.BeginWrite, responseStream.EndWrite, new byte[1], 0, 1, null)); if (!responseStream.CanSeek) { Assert.Throws(() => responseStream.Length); From efdcdba48a7402c1d2432978d52a159a85c8f437 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Sun, 10 Jul 2022 11:55:19 +0200 Subject: [PATCH 26/32] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 1 + .../src/System/Net/WebSockets/ClientWebSocket.cs | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 7496d561a8d530..5bcfcf1a3d10e0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -18,6 +18,7 @@ namespace System.Net.Http { internal sealed partial class Http2Connection : HttpConnectionBase { + // Equivalent to the bytes returned from HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewNameToAllocatedArray(":protocol") private static ReadOnlySpan ProtocolLiteralHeaderBytes => new byte[] { 0x0, 0x9, 0x3a, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x63, 0x6f, 0x6c }; private readonly HttpConnectionPool _pool; diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index be87195ed46cd3..4013c358428eca 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -53,15 +53,10 @@ public override WebSocketState State public Task ConnectAsync(Uri uri, CancellationToken cancellationToken) { - return ConnectAsyncHelper(uri, null, cancellationToken); + return ConnectAsync(uri, null, cancellationToken); } - public Task ConnectAsync(Uri uri, HttpMessageInvoker invoker, CancellationToken cancellationToken) - { - return ConnectAsyncHelper(uri, invoker, cancellationToken); - } - - private Task ConnectAsyncHelper(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken) + public Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(uri); From 6f7b10165b29dc31e207b691b8a01a15d0024562 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Sun, 10 Jul 2022 18:11:56 +0200 Subject: [PATCH 27/32] Adding connect tests --- .../tests/System/Net/Http/Http2Frames.cs | 3 +- .../System/Net/Http/Http2LoopbackServer.cs | 5 ++ .../tests/ConnectTest.Http2.cs | 69 +++++++++++++++++++ .../System.Net.WebSockets.Client.Tests.csproj | 52 +++++++------- 4 files changed, 100 insertions(+), 29 deletions(-) create mode 100644 src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs diff --git a/src/libraries/Common/tests/System/Net/Http/Http2Frames.cs b/src/libraries/Common/tests/System/Net/Http/Http2Frames.cs index 214dab635061a6..5f0ef36311049a 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2Frames.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2Frames.cs @@ -44,7 +44,8 @@ public enum SettingId : ushort MaxConcurrentStreams = 0x3, InitialWindowSize = 0x4, MaxFrameSize = 0x5, - MaxHeaderListSize = 0x6 + MaxHeaderListSize = 0x6, + EnableConnect = 0x8 } public class Frame diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs index edbefefb6ac1f8..9d17c93f0d0cb5 100644 --- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs @@ -44,6 +44,10 @@ public override Uri Address localEndPoint.Address.ToString(); string scheme = _options.UseSsl ? "https" : "http"; + if (_options.WebSocketEndpoint) + { + scheme = _options.UseSsl ? "wss" : "ws"; + } _uri = new Uri($"{scheme}://{host}:{localEndPoint.Port}/"); @@ -177,6 +181,7 @@ public static async Task CreateClientAndServerAsync(Func clientFunc, public class Http2Options : GenericLoopbackOptions { + public bool WebSocketEndpoint { get; set; } = false; public bool ClientCertificateRequired { get; set; } public bool EnableTransparentPingResponse { get; set; } = true; diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs new file mode 100644 index 00000000000000..718ee3d500f467 --- /dev/null +++ b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs @@ -0,0 +1,69 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Net.Http; +using System.Net.Test.Common; +using System.Threading; +using System.Threading.Tasks; + +using Xunit; +using Xunit.Abstractions; + +namespace System.Net.WebSockets.Client.Tests +{ + public class ConnectTest_Http2 : ClientWebSocketTestBase + { + public ConnectTest_Http2(ITestOutputHelper output) : base(output) { } + + [Fact] + public async Task ConnectAsync_VersionNotSupported_Throws() + { + await Http2LoopbackServer.CreateClientAndServerAsync(async uri => + { + using (var clientSocket = new ClientWebSocket()) + using (var cts = new CancellationTokenSource(TimeOutMilliseconds)) + { + clientSocket.Options.HttpVersion = HttpVersion.Version20; + clientSocket.Options.HttpVersionPolicy = Http.HttpVersionPolicy.RequestVersionExact; + using var handler = new SocketsHttpHandler(); + handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; }; + Task t = clientSocket.ConnectAsync(uri, new HttpMessageInvoker(handler), cts.Token); + var ex = await Assert.ThrowsAnyAsync(() => t); + Assert.IsType(ex.InnerException); + Assert.True(ex.InnerException.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL")); + } + }, + async server => + { + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 0 }); + }, new Http2Options() { WebSocketEndpoint = true } + ); + } + + [Fact] + public async Task ConnectAsync_VersionSupported_Success() + { + await Http2LoopbackServer.CreateClientAndServerAsync(async uri => + { + using (var clientSocket = new ClientWebSocket()) + using (var cts = new CancellationTokenSource(TimeOutMilliseconds)) + { + clientSocket.Options.HttpVersion = HttpVersion.Version20; + clientSocket.Options.HttpVersionPolicy = Http.HttpVersionPolicy.RequestVersionExact; + using var handler = new SocketsHttpHandler(); + handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; }; + await clientSocket.ConnectAsync(uri, new HttpMessageInvoker(handler), cts.Token); + } + }, + async server => + { + Http2LoopbackConnection connection = await server.EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 }); + (int streamId, HttpRequestData requestData) = await connection.ReadAndParseRequestHeaderAsync(readBody : false); + await connection.SendResponseHeadersAsync(streamId, endStream: false, HttpStatusCode.OK); + }, new Http2Options() { WebSocketEndpoint = true } + ); + } + } +} diff --git a/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj b/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj index d7cba47b1ace9e..9ead74786e5ba9 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj +++ b/src/libraries/System.Net.WebSockets.Client/tests/System.Net.WebSockets.Client.Tests.csproj @@ -24,10 +24,9 @@ - - - + + + @@ -35,36 +34,33 @@ - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + From 595642cbe16e059e1e1963111cdc7ac55fd3b77e Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Sun, 10 Jul 2022 18:36:16 +0200 Subject: [PATCH 28/32] Apply suggestions from code review Co-authored-by: Miha Zupan --- .../src/System/Net/Http/HttpRequestMessage.cs | 2 +- .../src/System/Net/WebSockets/ClientWebSocketOptions.cs | 7 ++++++- .../src/System/Net/WebSockets/WebSocketHandle.Managed.cs | 2 +- 3 files changed, 8 insertions(+), 3 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 60f06ebfeeeffe..44c42e7e0e2f13 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 @@ -168,7 +168,7 @@ public override string ToString() internal bool WasRedirected() => (_sendStatus & MessageIsRedirect) != 0; - internal bool IsWebSocketH2Request() => _version.Major == 2 && Method == HttpMethod.Connect && HasHeaders && Headers.Protocol == "websocket"; + internal bool IsWebSocketH2Request() => _version.Major == 2 && Method == HttpMethod.Connect && HasHeaders && string.Equals(Headers.Protocol, "websocket", StringComparison.OrdinalIgnoreCase); #region IDisposable Members diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index 8e02a7fb80ce9c..62d2703868b1d1 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -35,6 +35,7 @@ public Version HttpVersion [UnsupportedOSPlatform("browser")] set { + ThrowIfReadOnly(); ArgumentNullException.ThrowIfNull(value); _version = value; } @@ -44,7 +45,11 @@ public HttpVersionPolicy HttpVersionPolicy { get => _versionPolicy; [UnsupportedOSPlatform("browser")] - set => _versionPolicy = value; + set + { + ThrowIfReadOnly(); + _versionPolicy = value; + } } internal ClientWebSocketOptions() { } // prevent external instantiation diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index dbdc6a551c5895..e2bc69e8e4567a 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -215,7 +215,7 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, o options.Proxy == null && options.Cookies == null && options.RemoteCertificateValidationCallback == null && - options._clientCertificates?.Count == 0) + (options._clientCertificates?.Count ?? 0) == 0) { disposeHandler = false; handler = s_defaultHandler; From e05db11d235350755d042fc8cd3612d148d68de7 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Mon, 11 Jul 2022 01:09:03 +0200 Subject: [PATCH 29/32] feedback + skip tests on browser --- .../src/Resources/Strings.resx | 2 +- .../Net/Http/Headers/HttpRequestHeaders.cs | 11 ++++++++- .../SocketsHttpHandler/Http2Connection.cs | 24 +++++++++---------- .../SocketsHttpHandler/HttpConnectionPool.cs | 3 ++- .../src/Resources/Strings.resx | 7 ++---- .../Net/WebSockets/WebSocketHandle.Managed.cs | 7 +++--- .../tests/ClientWebSocketTestBase.cs | 2 +- .../tests/ConnectTest.Http2.cs | 2 ++ 8 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index d2220b38f6532e..f20cf84919190d 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -568,6 +568,6 @@ The HTTP/1.1 response chunk was too large. - Extended CONNECT is not supported. + Failed to establish web socket connection over HTTP/2 because extended CONNECT is not supported. Try to downgrade the request version to HTTP/1.1 diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index 961a349be86d33..ed287aaec6d5a1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -21,6 +21,7 @@ public sealed class HttpRequestHeaders : HttpHeaders private HttpGeneralHeaders? _generalHeaders; private HttpHeaderValueCollection? _expect; private bool _expectContinueSet; + private string? _protocol; #region Request Headers @@ -159,7 +160,15 @@ public int? MaxForwards set { SetOrRemoveParsedValue(KnownHeaders.MaxForwards.Descriptor, value); } } - public string? Protocol { get; set; } + public string? Protocol + { + get => _protocol; + set + { + CheckContainsNewLine(value); + _protocol = value; + } + } public AuthenticationHeaderValue? ProxyAuthorization { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 5bcfcf1a3d10e0..f1bbb5c88c3f7c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -21,6 +21,10 @@ internal sealed partial class Http2Connection : HttpConnectionBase // Equivalent to the bytes returned from HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewNameToAllocatedArray(":protocol") private static ReadOnlySpan ProtocolLiteralHeaderBytes => new byte[] { 0x0, 0x9, 0x3a, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x63, 0x6f, 0x6c }; + private static readonly TaskCompletionSourceWithCancellation s_settingsReceivedSingleton = CreateSuccessfullyCompletedTcs(); + + private TaskCompletionSourceWithCancellation? _initialSettingsReceived; + private readonly HttpConnectionPool _pool; private readonly Stream _stream; @@ -176,6 +180,13 @@ static long TimeSpanToMs(TimeSpan value) { private object SyncObject => _httpStreams; + internal TaskCompletionSourceWithCancellation InitialSettingsReceived => + _initialSettingsReceived ?? + Interlocked.CompareExchange(ref _initialSettingsReceived, new(), null) ?? + _initialSettingsReceived; + + internal bool IsConnectEnabled { get; private set; } + public async ValueTask SetupAsync() { try @@ -1468,7 +1479,6 @@ private void WriteHeaders(HttpRequestMessage request, ref ArrayBuffer headerBuff { if (request.Headers.Protocol != null) { - HttpHeaders.CheckContainsNewLine(request.Headers.Protocol); WriteBytes(ProtocolLiteralHeaderBytes, ref headerBuffer); Encoding? protocolEncoding = _pool.Settings._requestHeaderEncodingSelector?.Invoke(":protocol", request); WriteLiteralHeaderValue(request.Headers.Protocol, protocolEncoding, ref headerBuffer); @@ -1918,22 +1928,12 @@ private enum SettingId : ushort EnableConnect = 0x8 } - private static readonly TaskCompletionSourceWithCancellation s_settingsReceivedSingleton = CreateSuccessfullyCompleted(); - - internal TaskCompletionSourceWithCancellation InitialSettingsReceived => - _initialSettingsReceived ?? - Interlocked.CompareExchange(ref _initialSettingsReceived, new(), null) ?? - _initialSettingsReceived; - - private TaskCompletionSourceWithCancellation? _initialSettingsReceived; - - private static TaskCompletionSourceWithCancellation CreateSuccessfullyCompleted() + private static TaskCompletionSourceWithCancellation CreateSuccessfullyCompletedTcs() { var tcs = new TaskCompletionSourceWithCancellation(); tcs.TrySetResult(true); return tcs; } - internal bool IsConnectEnabled { get; private set; } // Note that this is safe to be called concurrently by multiple threads. 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 43e77b3a7f80b7..66d139a2f9a0bb 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 @@ -1024,7 +1024,8 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn { if (request.IsWebSocketH2Request()) { - if (await connection.InitialSettingsReceived.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false) && !connection.IsConnectEnabled) + await connection.InitialSettingsReceived.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); + if (!connection.IsConnectEnabled) { HttpRequestException exception = new(SR.net_unsupported_extended_connect); exception.Data["SETTINGS_ENABLE_CONNECT_PROTOCOL"] = false; diff --git a/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx b/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx index 28dca248629b6e..401be8dc707616 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx +++ b/src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx @@ -75,11 +75,8 @@ The argument must be a value greater than {0}. - - The server returned status code '{0}' when status code '101' was expected. - - - The server returned status code '{0}' when status code '200' was expected. + + The server returned status code '{0}' when status code '{1}' was expected. The server's response was missing the required header '{0}'. diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index e2bc69e8e4567a..34bda193949e31 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -83,7 +83,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio string? secValue = AddWebSocketHeaders(request, options); - // Issue the request. The response must be status code 101. + // Issue the request. CancellationTokenSource? linkedCancellation; CancellationTokenSource externalAndAbortCancellation; if (cancellationToken.CanBeCanceled) // avoid allocating linked source if external token is not cancelable @@ -355,7 +355,6 @@ static int ParseWindowBits(ReadOnlySpan value) else if (request.Version == HttpVersion.Version20) { request.Headers.Protocol = "websocket"; - request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Origin, request.Headers.Host); } request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.SecWebSocketVersion, "13"); @@ -413,7 +412,7 @@ private static void ValidateResponse(HttpResponseMessage response, string? secVa { if (response.StatusCode != HttpStatusCode.SwitchingProtocols) { - throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect101Expected, (int)response.StatusCode)); + throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_ConnectStatusExpected, (int)response.StatusCode, (int)HttpStatusCode.SwitchingProtocols)); } Debug.Assert(secValue != null); @@ -427,7 +426,7 @@ private static void ValidateResponse(HttpResponseMessage response, string? secVa { if (response.StatusCode != HttpStatusCode.OK) { - throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_Connect200Expected, (int)response.StatusCode)); + throw new WebSocketException(WebSocketError.NotAWebSocket, SR.Format(SR.net_WebSockets_ConnectStatusExpected, (int)response.StatusCode, (int)HttpStatusCode.OK)); } } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs b/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs index 91e5f8cc681e26..a03eff4c18c899 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs @@ -53,7 +53,7 @@ public static IEnumerable UnavailableWebSocketServers { server = System.Net.Test.Common.Configuration.Http.RemoteEchoServer; var ub = new UriBuilder("ws", server.Host, server.Port, server.PathAndQuery); - exceptionMessage = ResourceHelper.GetExceptionMessage("net_WebSockets_Connect101Expected", (int) HttpStatusCode.OK); + exceptionMessage = ResourceHelper.GetExceptionMessage("net_WebSockets_ConnectStatusExpected", (int) HttpStatusCode.OK, (int) HttpStatusCode.SwitchingProtocols); yield return new object[] { ub.Uri, exceptionMessage, WebSocketError.NotAWebSocket }; } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs index 718ee3d500f467..bfbdd9e2941c01 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs @@ -18,6 +18,7 @@ public class ConnectTest_Http2 : ClientWebSocketTestBase public ConnectTest_Http2(ITestOutputHelper output) : base(output) { } [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "WebSocket over HTTP/2 is not supported on Browser")] public async Task ConnectAsync_VersionNotSupported_Throws() { await Http2LoopbackServer.CreateClientAndServerAsync(async uri => @@ -43,6 +44,7 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => } [Fact] + [SkipOnPlatform(TestPlatforms.Browser, "WebSocket over HTTP/2 is not supported on Browser")] public async Task ConnectAsync_VersionSupported_Success() { await Http2LoopbackServer.CreateClientAndServerAsync(async uri => From b7f543bdf00483fb31ba199a648d6fa5f63e3b6d Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 12 Jul 2022 18:33:14 +0200 Subject: [PATCH 30/32] Feedback + test for websocket stream --- .../SocketsHttpHandler/Http2Connection.cs | 8 +++ .../SocketsHttpHandler/HttpConnectionPool.cs | 7 +- .../HttpClientHandlerTest.Http2.cs | 64 +++++++++++++++++++ .../Net/WebSockets/WebSocketHandle.Managed.cs | 11 ++-- .../tests/ConnectTest.Http2.cs | 12 ++-- 5 files changed, 91 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index f4cca5126c64e5..30c8761de4eeb4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -850,6 +850,13 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f { IsConnectEnabled = true; } + else if (settingValue == 0 && IsConnectEnabled) + { + // Accroding to RFC: a sender MUST NOT send a SETTINGS_ENABLE_CONNECT_PROTOCOL parameter + // with the value of 0 after previously sending a value of 1. + // https://datatracker.ietf.org/doc/html/rfc8441#section-3 + ThrowProtocolError(); + } break; default: @@ -871,6 +878,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f { Interlocked.CompareExchange(ref _initialSettingsReceived, s_settingsReceivedSingleton, null); } + // Set result in case if CompareExchange lost the race InitialSettingsReceived.TrySetResult(true); } 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 f129e39e4a889c..45cc1a6e1e0571 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 @@ -1087,7 +1087,12 @@ public async ValueTask SendWithVersionDetectionAndRetryAsyn // Throw if fallback is not allowed by the version policy. if (request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - throw new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy), e); + HttpRequestException exception = new HttpRequestException(SR.Format(SR.net_http_requested_version_server_refused, request.Version, request.VersionPolicy), e); + if (request.IsWebSocketH2Request()) + { + exception.Data["HTTP2_ENABLED"] = false; + } + throw exception; } if (NetEventSource.Log.IsEnabled()) 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 98537366ec43d8..dfc3625aaa7b12 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -2548,6 +2548,70 @@ public async Task PostAsyncDuplex_ServerSendsEndStream_Success() } } + [Fact] + public async Task ConnectAsync_ReadWriteWebSocketStream() + { + var clientMessage = new byte[] { 1, 2, 3 }; + var serverMessage = new byte[] { 4, 5, 6, 7 }; + + using Http2LoopbackServer server = Http2LoopbackServer.CreateServer(); + Http2LoopbackConnection connection = null; + + Task serverTask = Task.Run(async () => + { + connection = await server.EstablishConnectionAsync(new SettingsEntry { SettingId = SettingId.EnableConnect, Value = 1 }); + + // read request headers + (int streamId, _) = await connection.ReadAndParseRequestHeaderAsync(readBody: false); + + // send response headers + await connection.SendResponseHeadersAsync(streamId, endStream: false).ConfigureAwait(false); + + // send reply + await connection.SendResponseDataAsync(streamId, serverMessage, endStream: false); + + // send server EOS + await connection.SendResponseDataAsync(streamId, Array.Empty(), endStream: true); + }); + + StreamingHttpContent requestContent = new StreamingHttpContent(); + + using var handler = new SocketsHttpHandler(); + handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; }; + + using HttpClient client = new HttpClient(handler); + + HttpRequestMessage request = new(HttpMethod.Connect, server.Address); + request.Version = HttpVersion.Version20; + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + request.Headers.Protocol = "websocket"; + + // initiate request + var responseTask = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); + + using HttpResponseMessage response = await responseTask.WaitAsync(TimeSpan.FromSeconds(10)); + + await serverTask.WaitAsync(TimeSpan.FromSeconds(60)); + + var responseStream = await response.Content.ReadAsStreamAsync(); + + // receive data + var readBuffer = new byte[10]; + int bytesRead = await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)); + Assert.Equal(bytesRead, serverMessage.Length); + Assert.Equal(serverMessage, readBuffer[..bytesRead]); + + await responseStream.WriteAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)); + + // Send client's EOS + requestContent.CompleteStream(); + // Receive server's EOS + Assert.Equal(0, await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10))); + + Assert.NotNull(connection); + connection.Dispose(); + } + [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/69870", TestPlatforms.Android)] public async Task PostAsyncDuplex_RequestContentException_ResetsStream() diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index 34bda193949e31..fb4d354a83f884 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -57,9 +57,13 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio try { HttpRequestMessage request; - if (!tryDowngrade && options.HttpVersion == HttpVersion.Version20 + if (!tryDowngrade && options.HttpVersion >= HttpVersion.Version20 || (options.HttpVersion == HttpVersion.Version11 && options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)) { + if (options.HttpVersion > HttpVersion.Version20 && options.HttpVersionPolicy != HttpVersionPolicy.RequestVersionOrLower) + { + throw new WebSocketException(WebSocketError.UnsupportedProtocol); + } request = new HttpRequestMessage(HttpMethod.Connect, uri) { Version = HttpVersion.Version20 }; tryDowngrade = true; } @@ -108,9 +112,8 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio break; } catch (HttpRequestException ex) when - (ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") && - tryDowngrade && - (options.HttpVersion == HttpVersion.Version11 || options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower)) + ((ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || ex.Data.Contains("HTTP2_ENABLED") || tryDowngrade) + && (options.HttpVersion == HttpVersion.Version11 || options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower)) { } diff --git a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs index bfbdd9e2941c01..23d260c87e2ffe 100644 --- a/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs +++ b/src/libraries/System.Net.WebSockets.Client/tests/ConnectTest.Http2.cs @@ -18,7 +18,7 @@ public class ConnectTest_Http2 : ClientWebSocketTestBase public ConnectTest_Http2(ITestOutputHelper output) : base(output) { } [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "WebSocket over HTTP/2 is not supported on Browser")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/69870", TestPlatforms.Browser)] public async Task ConnectAsync_VersionNotSupported_Throws() { await Http2LoopbackServer.CreateClientAndServerAsync(async uri => @@ -44,19 +44,19 @@ await Http2LoopbackServer.CreateClientAndServerAsync(async uri => } [Fact] - [SkipOnPlatform(TestPlatforms.Browser, "WebSocket over HTTP/2 is not supported on Browser")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/69870", TestPlatforms.Browser)] public async Task ConnectAsync_VersionSupported_Success() { await Http2LoopbackServer.CreateClientAndServerAsync(async uri => { - using (var clientSocket = new ClientWebSocket()) + using (var cws = new ClientWebSocket()) using (var cts = new CancellationTokenSource(TimeOutMilliseconds)) { - clientSocket.Options.HttpVersion = HttpVersion.Version20; - clientSocket.Options.HttpVersionPolicy = Http.HttpVersionPolicy.RequestVersionExact; + cws.Options.HttpVersion = HttpVersion.Version20; + cws.Options.HttpVersionPolicy = Http.HttpVersionPolicy.RequestVersionExact; using var handler = new SocketsHttpHandler(); handler.SslOptions.RemoteCertificateValidationCallback = delegate { return true; }; - await clientSocket.ConnectAsync(uri, new HttpMessageInvoker(handler), cts.Token); + await cws.ConnectAsync(uri, new HttpMessageInvoker(handler), cts.Token); } }, async server => From 73968ec5fa74b60c323390c02931c441d1128755 Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 12 Jul 2022 18:34:34 +0200 Subject: [PATCH 31/32] Update src/libraries/System.Net.Http/src/Resources/Strings.resx Co-authored-by: Stephen Toub --- src/libraries/System.Net.Http/src/Resources/Strings.resx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index f20cf84919190d..3ac957f5d8107c 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -568,6 +568,6 @@ The HTTP/1.1 response chunk was too large. - Failed to establish web socket connection over HTTP/2 because extended CONNECT is not supported. Try to downgrade the request version to HTTP/1.1 + Failed to establish web socket connection over HTTP/2 because extended CONNECT is not supported. Try to downgrade the request version to HTTP/1.1. From 4dbe1c290c184e88bc128951b416ea9703ce614d Mon Sep 17 00:00:00 2001 From: Katya Sokolova Date: Tue, 12 Jul 2022 18:50:05 +0200 Subject: [PATCH 32/32] Address review feedback --- .../ref/System.Net.WebSockets.Client.cs | 8 ++------ .../src/System/Net/WebSockets/ClientWebSocketOptions.cs | 8 ++++---- .../src/System/Net/WebSockets/WebSocketHandle.Managed.cs | 3 ++- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs index a331e1e5a1a912..15cc579688698f 100644 --- a/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs +++ b/src/libraries/System.Net.WebSockets.Client/ref/System.Net.WebSockets.Client.cs @@ -43,12 +43,8 @@ internal ClientWebSocketOptions() { } public System.Net.Security.RemoteCertificateValidationCallback? RemoteCertificateValidationCallback { get { throw null; } set { } } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public bool UseDefaultCredentials { get { throw null; } set { } } - public System.Version HttpVersion { get { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] - set { } } - public System.Net.Http.HttpVersionPolicy HttpVersionPolicy { get { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] - set { } } + public System.Version HttpVersion { get { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] set { } } + public System.Net.Http.HttpVersionPolicy HttpVersionPolicy { get { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] set { } } public void AddSubProtocol(string subProtocol) { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public void SetBuffer(int receiveBufferSize, int sendBufferSize) { } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs index 62d2703868b1d1..5f8027abda7bb2 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs @@ -29,6 +29,10 @@ public sealed class ClientWebSocketOptions private Version _version = Net.HttpVersion.Version11; private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower; + internal ClientWebSocketOptions() { } // prevent external instantiation + + #region HTTP Settings + public Version HttpVersion { get => _version; @@ -52,10 +56,6 @@ public HttpVersionPolicy HttpVersionPolicy } } - internal ClientWebSocketOptions() { } // prevent external instantiation - - #region HTTP Settings - [UnsupportedOSPlatform("browser")] // Note that some headers are restricted like Host. public void SetRequestHeader(string headerName, string? headerValue) diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs index fb4d354a83f884..480ea91ce1e3e3 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs @@ -112,7 +112,8 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio break; } catch (HttpRequestException ex) when - ((ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || ex.Data.Contains("HTTP2_ENABLED") || tryDowngrade) + ((ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || ex.Data.Contains("HTTP2_ENABLED")) + && tryDowngrade && (options.HttpVersion == HttpVersion.Version11 || options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrLower)) { }