Skip to content

Commit 9ea5d2d

Browse files
MihaZupanMihaZupan
authored
Throw on incompatible WebSocket options (#74473)
* Throw on incompatible WebSocket options * Don't set ServerCertificateCustomValidationCallback on Browser * Update exception messages * Skip new H2 test on Browser
1 parent 941ec70 commit 9ea5d2d

12 files changed

+268
-135
lines changed

‎src/libraries/System.Net.WebSockets.Client/src/Resources/Strings.resx

+6
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,10 @@
129129
<data name="net_WebSockets_ClientWindowBitsNegotiationFailure" xml:space="preserve">
130130
<value>The WebSocket failed to negotiate max client window bits. The client requested {0} but the server responded with {1}.</value>
131131
</data>
132+
<data name="net_WebSockets_OptionsIncompatibleWithCustomInvoker" xml:space="preserve">
133+
<value>UseDefaultCredentials, Credentials, Proxy, ClientCertificates, RemoteCertificateValidationCallback and Cookies must not be set on ClientWebSocketOptions when an HttpMessageInvoker instance is also specified. These options should be set on the HttpMessageInvoker's underlying HttpMessageHandler instead.</value>
134+
</data>
135+
<data name="net_WebSockets_CustomInvokerRequiredForHttp2" xml:space="preserve">
136+
<value>An HttpMessageInvoker instance must be passed to ConnectAsync when using HTTP/2.</value>
137+
</data>
132138
</root>

‎src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocketOptions.cs

+8
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ public sealed class ClientWebSocketOptions
3030
private HttpVersionPolicy _versionPolicy = HttpVersionPolicy.RequestVersionOrLower;
3131
private bool _collectHttpResponseDetails;
3232

33+
internal bool AreCompatibleWithCustomInvoker() =>
34+
!UseDefaultCredentials &&
35+
Credentials is null &&
36+
(_clientCertificates?.Count ?? 0) == 0 &&
37+
RemoteCertificateValidationCallback is null &&
38+
Cookies is null &&
39+
(Proxy is null || Proxy == WebSocketHandle.DefaultWebProxy.Instance);
40+
3341
internal ClientWebSocketOptions() { } // prevent external instantiation
3442

3543
#region HTTP Settings

‎src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs

+17-9
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,22 @@ public void Abort()
4848
public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken cancellationToken, ClientWebSocketOptions options)
4949
{
5050
bool disposeHandler = false;
51-
invoker ??= new HttpMessageInvoker(SetupHandler(options, out disposeHandler));
52-
HttpResponseMessage? response = null;
51+
if (invoker is null)
52+
{
53+
if (options.HttpVersion.Major >= 2 || options.HttpVersionPolicy == HttpVersionPolicy.RequestVersionOrHigher)
54+
{
55+
throw new ArgumentException(SR.net_WebSockets_CustomInvokerRequiredForHttp2, nameof(options));
56+
}
5357

58+
invoker = new HttpMessageInvoker(SetupHandler(options, out disposeHandler));
59+
}
60+
else if (!options.AreCompatibleWithCustomInvoker())
61+
{
62+
// This will not throw if the Proxy is a DefaultWebProxy.
63+
throw new ArgumentException(SR.net_WebSockets_OptionsIncompatibleWithCustomInvoker, nameof(options));
64+
}
65+
66+
HttpResponseMessage? response = null;
5467
bool disposeResponse = false;
5568

5669
// force non-secure request to 1.1 whenever it is possible as HttpClient does
@@ -237,12 +250,7 @@ private static SocketsHttpHandler SetupHandler(ClientWebSocketOptions options, o
237250
// Create the handler for this request and populate it with all of the options.
238251
// Try to use a shared handler rather than creating a new one just for this request, if
239252
// the options are compatible.
240-
if (options.Credentials == null &&
241-
!options.UseDefaultCredentials &&
242-
options.Proxy == null &&
243-
options.Cookies == null &&
244-
options.RemoteCertificateValidationCallback == null &&
245-
(options._clientCertificates?.Count ?? 0) == 0)
253+
if (options.AreCompatibleWithCustomInvoker() && options.Proxy is null)
246254
{
247255
disposeHandler = false;
248256
handler = s_defaultHandler;
@@ -518,7 +526,7 @@ private static void ValidateHeader(HttpHeaders headers, string name, string expe
518526
}
519527

520528
/// <summary>Used as a sentinel to indicate that ClientWebSocket should use the system's default proxy.</summary>
521-
private sealed class DefaultWebProxy : IWebProxy
529+
internal sealed class DefaultWebProxy : IWebProxy
522530
{
523531
public static DefaultWebProxy Instance { get; } = new DefaultWebProxy();
524532
public ICredentials? Credentials { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }

‎src/libraries/System.Net.WebSockets.Client/tests/AbortTest.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ public sealed class InvokerAbortTest : AbortTest
1616
{
1717
public InvokerAbortTest(ITestOutputHelper output) : base(output) { }
1818

19-
protected override HttpMessageInvoker? GetInvoker() => new HttpMessageInvoker(new SocketsHttpHandler());
19+
protected override bool UseCustomInvoker => true;
2020
}
2121

2222
public sealed class HttpClientAbortTest : AbortTest
2323
{
2424
public HttpClientAbortTest(ITestOutputHelper output) : base(output) { }
2525

26-
protected override HttpMessageInvoker? GetInvoker() => new HttpClient(new HttpClientHandler());
26+
protected override bool UseHttpClient => true;
2727
}
2828

2929
public class AbortTest : ClientWebSocketTestBase

‎src/libraries/System.Net.WebSockets.Client/tests/CancelTest.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ public sealed class InvokerCancelTest : CancelTest
1414
{
1515
public InvokerCancelTest(ITestOutputHelper output) : base(output) { }
1616

17-
protected override HttpMessageInvoker? GetInvoker() => new HttpMessageInvoker(new SocketsHttpHandler());
17+
protected override bool UseCustomInvoker => true;
1818
}
1919

2020
public sealed class HttpClientCancelTest : CancelTest
2121
{
2222
public HttpClientCancelTest(ITestOutputHelper output) : base(output) { }
2323

24-
protected override HttpMessageInvoker? GetInvoker() => new HttpClient(new HttpClientHandler());
24+
protected override bool UseHttpClient => true;
2525
}
2626

2727
public class CancelTest : ClientWebSocketTestBase

‎src/libraries/System.Net.WebSockets.Client/tests/ClientWebSocketTestBase.cs

+33-6
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,17 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Generic;
5-
using System.Net.Test.Common;
65
using System.Linq;
76
using System.Threading;
87
using System.Threading.Tasks;
98

109
using Xunit;
1110
using Xunit.Abstractions;
1211
using System.Net.Http;
13-
using System.Net.WebSockets.Client.Tests;
12+
using System.Diagnostics;
1413

1514
namespace System.Net.WebSockets.Client.Tests
1615
{
17-
/// <summary>
18-
/// ClientWebSocket tests that do require a remote server.
19-
/// </summary>
2016
public class ClientWebSocketTestBase
2117
{
2218
public static readonly object[][] EchoServers = System.Net.Test.Common.Configuration.WebSockets.EchoServers;
@@ -112,7 +108,38 @@ protected static async Task<WebSocketReceiveResult> ReceiveEntireMessageAsync(We
112108
}
113109
}
114110

115-
protected virtual HttpMessageInvoker? GetInvoker() => null;
111+
protected virtual bool UseCustomInvoker => false;
112+
113+
protected virtual bool UseHttpClient => false;
114+
115+
protected bool UseSharedHandler => !UseCustomInvoker && !UseHttpClient;
116+
117+
protected Action<HttpClientHandler>? ConfigureCustomHandler;
118+
119+
internal HttpMessageInvoker? GetInvoker()
120+
{
121+
var handler = new HttpClientHandler();
122+
123+
if (PlatformDetection.IsNotBrowser)
124+
{
125+
handler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator;
126+
}
127+
128+
ConfigureCustomHandler?.Invoke(handler);
129+
130+
if (UseCustomInvoker)
131+
{
132+
Debug.Assert(!UseHttpClient);
133+
return new HttpMessageInvoker(handler);
134+
}
135+
136+
if (UseHttpClient)
137+
{
138+
return new HttpClient(handler);
139+
}
140+
141+
return null;
142+
}
116143

117144
protected Task<ClientWebSocket> GetConnectedWebSocket(Uri uri, int TimeOutMilliseconds, ITestOutputHelper output) =>
118145
WebSocketHelper.GetConnectedWebSocket(uri, TimeOutMilliseconds, output, invoker: GetInvoker());

‎src/libraries/System.Net.WebSockets.Client/tests/CloseTest.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ public sealed class InvokerCloseTest : CloseTest
1818
{
1919
public InvokerCloseTest(ITestOutputHelper output) : base(output) { }
2020

21-
protected override HttpMessageInvoker? GetInvoker() => new HttpMessageInvoker(new SocketsHttpHandler());
21+
protected override bool UseCustomInvoker => true;
2222
}
2323

2424
public sealed class HttpClientCloseTest : CloseTest
2525
{
2626
public HttpClientCloseTest(ITestOutputHelper output) : base(output) { }
2727

28-
protected override HttpMessageInvoker? GetInvoker() => new HttpClient(new HttpClientHandler());
28+
protected override bool UseHttpClient => true;
2929
}
3030

3131
public class CloseTest : ClientWebSocketTestBase

0 commit comments

Comments
 (0)