Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit d4853e2

Browse files
stephentoubstephentoub
authored
Use shared SocketsHttpHandler for some ClientWebSocket options (#27994)
* Use shared SocketsHttpHandler for some ClientWebSocket options Currently we're creating a new SocketsHttpHandler for each ClientWebSocket. This change makes it so that if mostly default options are used (namely no credentials, no proxy, no cookies, and no client certificates), we'll use a shared handler instance. * Make ClientWebSocketOptions.Proxy non-null by default netfx's ClientWebSocketOptions.Proxy defaults to a system-default proxy. It can then be set to null to mean "no proxy" or to another proxy. This changes netcoreapp's Proxy to match as closely as possible. It also then fixes up a few other settings on SocketsHttpHandler: - UseDefaultCredentials wasn't being factored in. On netfx, UseDefaultCredentials takes precedence over Credentials, so this does the same. - UseCookies should be true iff Cookies is non-null. Finally, removed Outerloop from some tests that did not need to be, as they didn't actually involve making external requests, and deleted some dead test code.
1 parent 9665203 commit d4853e2

File tree

8 files changed

+136
-122
lines changed

8 files changed

+136
-122
lines changed

‎src/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public ClientWebSocket()
3131
WebSocketHandle.CheckPlatformSupport();
3232

3333
_state = (int)InternalState.Created;
34-
_options = new ClientWebSocketOptions();
34+
_options = new ClientWebSocketOptions() { Proxy = DefaultWebProxy.Instance };
3535

3636
if (NetEventSource.IsEnabled) NetEventSource.Exit(this);
3737
}
@@ -236,5 +236,14 @@ private void ThrowIfNotConnected()
236236
throw new InvalidOperationException(SR.net_WebSockets_NotConnected);
237237
}
238238
}
239+
240+
/// <summary>Used as a sentinel to indicate that ClientWebSocket should use the system's default proxy.</summary>
241+
internal sealed class DefaultWebProxy : IWebProxy
242+
{
243+
public static DefaultWebProxy Instance { get; } = new DefaultWebProxy();
244+
public ICredentials Credentials { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
245+
public Uri GetProxy(Uri destination) => throw new NotSupportedException();
246+
public bool IsBypassed(Uri host) => throw new NotSupportedException();
247+
}
239248
}
240249
}

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

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ internal sealed class WebSocketHandle
2020
/// <summary>GUID appended by the server as part of the security key response. Defined in the RFC.</summary>
2121
private const string WSServerGuid = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11";
2222

23+
/// <summary>Shared, lazily-initialized handler for when using default options.</summary>
24+
private static SocketsHttpHandler s_defaultHandler;
25+
2326
private readonly CancellationTokenSource _abortSource = new CancellationTokenSource();
2427
private WebSocketState _state = WebSocketState.Connecting;
2528
private WebSocket _webSocket;
@@ -72,6 +75,7 @@ public async Task ConnectAsyncCore(Uri uri, CancellationToken cancellationToken,
7275
{
7376
HttpResponseMessage response = null;
7477
SocketsHttpHandler handler = null;
78+
bool disposeHandler = true;
7579
try
7680
{
7781
// Create the request message, including a uri with ws{s} switched to http{s}.
@@ -90,26 +94,72 @@ public async Task ConnectAsyncCore(Uri uri, CancellationToken cancellationToken,
9094
AddWebSocketHeaders(request, secKeyAndSecWebSocketAccept.Key, options);
9195

9296
// Create the handler for this request and populate it with all of the options.
93-
handler = new SocketsHttpHandler();
94-
handler.Credentials = options.Credentials;
95-
handler.Proxy = options.Proxy;
96-
handler.CookieContainer = options.Cookies;
97-
handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback;
98-
if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection
97+
// Try to use a shared handler rather than creating a new one just for this request, if
98+
// the options are compatible.
99+
if (options.Credentials == null &&
100+
!options.UseDefaultCredentials &&
101+
options.Proxy == null &&
102+
options.Cookies == null &&
103+
options.RemoteCertificateValidationCallback == null &&
104+
options._clientCertificates?.Count == 0)
99105
{
100-
if (handler.SslOptions.ClientCertificates == null)
106+
disposeHandler = false;
107+
handler = s_defaultHandler;
108+
if (handler == null)
109+
{
110+
handler = new SocketsHttpHandler()
111+
{
112+
PooledConnectionLifetime = TimeSpan.Zero,
113+
UseProxy = false,
114+
UseCookies = false,
115+
};
116+
if (Interlocked.CompareExchange(ref s_defaultHandler, handler, null) != null)
117+
{
118+
handler.Dispose();
119+
handler = s_defaultHandler;
120+
}
121+
}
122+
}
123+
else
124+
{
125+
handler = new SocketsHttpHandler();
126+
handler.PooledConnectionLifetime = TimeSpan.Zero;
127+
handler.CookieContainer = options.Cookies;
128+
handler.UseCookies = options.Cookies != null;
129+
handler.SslOptions.RemoteCertificateValidationCallback = options.RemoteCertificateValidationCallback;
130+
131+
if (options.UseDefaultCredentials)
132+
{
133+
handler.Credentials = CredentialCache.DefaultCredentials;
134+
}
135+
else
136+
{
137+
handler.Credentials = options.Credentials;
138+
}
139+
140+
if (options.Proxy == null)
141+
{
142+
handler.UseProxy = false;
143+
}
144+
else if (options.Proxy != ClientWebSocket.DefaultWebProxy.Instance)
101145
{
146+
handler.Proxy = options.Proxy;
147+
}
148+
149+
if (options._clientCertificates?.Count > 0) // use field to avoid lazily initializing the collection
150+
{
151+
Debug.Assert(handler.SslOptions.ClientCertificates == null);
102152
handler.SslOptions.ClientCertificates = new X509Certificate2Collection();
153+
handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates);
103154
}
104-
handler.SslOptions.ClientCertificates.AddRange(options.ClientCertificates);
105155
}
106156

107157
// Issue the request. The response must be status code 101.
108158
CancellationTokenSource linkedCancellation, externalAndAbortCancellation;
109159
if (cancellationToken.CanBeCanceled) // avoid allocating linked source if external token is not cancelable
110160
{
111161
linkedCancellation =
112-
externalAndAbortCancellation =
162+
externalAndAbortCancellation =
113163
CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _abortSource.Token);
114164
}
115165
else
@@ -193,7 +243,10 @@ public async Task ConnectAsyncCore(Uri uri, CancellationToken cancellationToken,
193243
finally
194244
{
195245
// Disposing the handler will not affect any active stream wrapped in the WebSocket.
196-
handler?.Dispose();
246+
if (disposeHandler)
247+
{
248+
handler?.Dispose();
249+
}
197250
}
198251
}
199252

‎src/System.Net.WebSockets.Client/tests/ClientWebSocketOptionsTests.Unix.cs‎

Lines changed: 0 additions & 36 deletions
This file was deleted.

‎src/System.Net.WebSockets.Client/tests/ClientWebSocketOptionsTests.Windows.cs‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎src/System.Net.WebSockets.Client/tests/ClientWebSocketOptionsTests.cs‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System.Collections.Generic;
65
using System.Net.Security;
7-
using System.Net.Sockets;
86
using System.Net.Test.Common;
97
using System.Reflection;
10-
using System.Security.Authentication;
118
using System.Security.Cryptography.X509Certificates;
129
using System.Threading;
1310
using System.Threading.Tasks;
@@ -19,10 +16,6 @@ namespace System.Net.WebSockets.Client.Tests
1916
{
2017
public partial class ClientWebSocketOptionsTests : ClientWebSocketTestBase
2118
{
22-
public static bool CanTestCertificates =>
23-
Capability.IsTrustedRootCertificateInstalled() &&
24-
(BackendSupportsCustomCertificateHandling || Capability.AreHostsFileNamesInstalled());
25-
2619
// Windows 10 Version 1709 introduced the necessary APIs for the UAP version of
2720
// ClientWebSocket.ConnectAsync to carry out mutual TLS authentication.
2821
public static bool ClientCertificatesSupported => !PlatformDetection.IsUap;
@@ -40,6 +33,40 @@ public static void UseDefaultCredentials_Roundtrips()
4033
Assert.False(cws.Options.UseDefaultCredentials);
4134
}
4235

36+
[ConditionalFact(nameof(WebSocketsSupported))]
37+
public static void Proxy_Roundtrips()
38+
{
39+
var cws = new ClientWebSocket();
40+
41+
Assert.NotNull(cws.Options.Proxy);
42+
Assert.Same(cws.Options.Proxy, cws.Options.Proxy);
43+
44+
IWebProxy p = new WebProxy();
45+
cws.Options.Proxy = p;
46+
Assert.Same(p, cws.Options.Proxy);
47+
48+
cws.Options.Proxy = null;
49+
Assert.Null(cws.Options.Proxy);
50+
}
51+
52+
[OuterLoop] // TODO: Issue #11345
53+
[ConditionalTheory(nameof(WebSocketsSupported)), MemberData(nameof(EchoServers))]
54+
public async Task NullProxySet_ConnectsSuccessfully(Uri server)
55+
{
56+
for (int i = 0; i < 3; i++) // Connect and disconnect multiple times to exercise shared handler on netcoreapp
57+
{
58+
var ws = await WebSocketHelper.Retry(_output, async () =>
59+
{
60+
var cws = new ClientWebSocket();
61+
cws.Options.Proxy = null;
62+
await cws.ConnectAsync(server, default);
63+
return cws;
64+
});
65+
await ws.CloseAsync(WebSocketCloseStatus.NormalClosure, string.Empty, default);
66+
ws.Dispose();
67+
}
68+
}
69+
4370
[ConditionalFact(nameof(WebSocketsSupported))]
4471
public static void SetBuffer_InvalidArgs_Throws()
4572
{

‎src/System.Net.WebSockets.Client/tests/ClientWebSocketUnitTest.cs‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
using System.Threading.Tasks;
77

88
using Xunit;
9-
using Xunit.Abstractions;
109

1110
namespace System.Net.WebSockets.Client.Tests
1211
{
@@ -15,24 +14,15 @@ namespace System.Net.WebSockets.Client.Tests
1514
/// </summary>
1615
public class ClientWebSocketUnitTest
1716
{
18-
private readonly ITestOutputHelper _output;
19-
20-
public ClientWebSocketUnitTest(ITestOutputHelper output)
21-
{
22-
_output = output;
23-
}
24-
2517
private static bool WebSocketsSupported { get { return WebSocketHelper.WebSocketsSupported; } }
2618

27-
[OuterLoop] // TODO: Issue #11345
2819
[ConditionalFact(nameof(WebSocketsSupported))]
2920
public void Ctor_Success()
3021
{
3122
var cws = new ClientWebSocket();
3223
cws.Dispose();
3324
}
3425

35-
[OuterLoop] // TODO: Issue #11345
3626
[ConditionalFact(nameof(WebSocketsSupported))]
3727
public void Abort_CreateAndAbort_StateIsClosed()
3828
{
@@ -44,7 +34,6 @@ public void Abort_CreateAndAbort_StateIsClosed()
4434
}
4535
}
4636

47-
[OuterLoop] // TODO: Issue #11345
4837
[ConditionalFact(nameof(WebSocketsSupported))]
4938
public void CloseAsync_CreateAndClose_ThrowsInvalidOperationException()
5039
{
@@ -57,7 +46,6 @@ public void CloseAsync_CreateAndClose_ThrowsInvalidOperationException()
5746
}
5847
}
5948

60-
[OuterLoop] // TODO: Issue #11345
6149
[ConditionalFact(nameof(WebSocketsSupported))]
6250
public async Task CloseAsync_CreateAndCloseOutput_ThrowsInvalidOperationExceptionWithMessage()
6351
{
@@ -77,7 +65,6 @@ public async Task CloseAsync_CreateAndCloseOutput_ThrowsInvalidOperationExceptio
7765
}
7866
}
7967

80-
[OuterLoop] // TODO: Issue #11345
8168
[ConditionalFact(nameof(WebSocketsSupported))]
8269
public void CloseAsync_CreateAndReceive_ThrowsInvalidOperationException()
8370
{
@@ -94,7 +81,6 @@ public void CloseAsync_CreateAndReceive_ThrowsInvalidOperationException()
9481
}
9582
}
9683

97-
[OuterLoop] // TODO: Issue #11345
9884
[ConditionalFact(nameof(WebSocketsSupported))]
9985
public async Task CloseAsync_CreateAndReceive_ThrowsInvalidOperationExceptionWithMessage()
10086
{
@@ -118,7 +104,6 @@ public async Task CloseAsync_CreateAndReceive_ThrowsInvalidOperationExceptionWit
118104
}
119105
}
120106

121-
[OuterLoop] // TODO: Issue #11345
122107
[ConditionalFact(nameof(WebSocketsSupported))]
123108
public void CloseAsync_CreateAndSend_ThrowsInvalidOperationException()
124109
{
@@ -135,7 +120,6 @@ public void CloseAsync_CreateAndSend_ThrowsInvalidOperationException()
135120
}
136121
}
137122

138-
[OuterLoop] // TODO: Issue #11345
139123
[ConditionalFact(nameof(WebSocketsSupported))]
140124
public async Task CloseAsync_CreateAndSend_ThrowsInvalidOperationExceptionWithMessage()
141125
{
@@ -159,7 +143,6 @@ public async Task CloseAsync_CreateAndSend_ThrowsInvalidOperationExceptionWithMe
159143
}
160144
}
161145

162-
[OuterLoop] // TODO: Issue #11345
163146
[ConditionalFact(nameof(WebSocketsSupported))]
164147
public void Ctor_ExpectedPropertyValues()
165148
{
@@ -174,7 +157,6 @@ public void Ctor_ExpectedPropertyValues()
174157
}
175158
}
176159

177-
[OuterLoop] // TODO: Issue #11345
178160
[ConditionalFact(nameof(WebSocketsSupported))]
179161
public void Abort_CreateAndDisposeAndAbort_StateIsClosedSuccess()
180162
{
@@ -185,7 +167,6 @@ public void Abort_CreateAndDisposeAndAbort_StateIsClosedSuccess()
185167
Assert.Equal(WebSocketState.Closed, cws.State);
186168
}
187169

188-
[OuterLoop] // TODO: Issue #11345
189170
[ConditionalFact(nameof(WebSocketsSupported))]
190171
public void CloseAsync_DisposeAndClose_ThrowsObjectDisposedException()
191172
{
@@ -198,7 +179,6 @@ public void CloseAsync_DisposeAndClose_ThrowsObjectDisposedException()
198179
Assert.Equal(WebSocketState.Closed, cws.State);
199180
}
200181

201-
[OuterLoop] // TODO: Issue #11345
202182
[ConditionalFact(nameof(WebSocketsSupported))]
203183
public void CloseAsync_DisposeAndCloseOutput_ThrowsObjectDisposedExceptionWithMessage()
204184
{
@@ -215,7 +195,6 @@ public void CloseAsync_DisposeAndCloseOutput_ThrowsObjectDisposedExceptionWithMe
215195
Assert.Equal(WebSocketState.Closed, cws.State);
216196
}
217197

218-
[OuterLoop] // TODO: Issue #11345
219198
[ConditionalFact(nameof(WebSocketsSupported))]
220199
public void ReceiveAsync_CreateAndDisposeAndReceive_ThrowsObjectDisposedExceptionWithMessage()
221200
{
@@ -235,7 +214,6 @@ public void ReceiveAsync_CreateAndDisposeAndReceive_ThrowsObjectDisposedExceptio
235214
Assert.Equal(WebSocketState.Closed, cws.State);
236215
}
237216

238-
[OuterLoop] // TODO: Issue #11345
239217
[ConditionalFact(nameof(WebSocketsSupported))]
240218
public void SendAsync_CreateAndDisposeAndSend_ThrowsObjectDisposedExceptionWithMessage()
241219
{
@@ -255,7 +233,6 @@ public void SendAsync_CreateAndDisposeAndSend_ThrowsObjectDisposedExceptionWithM
255233
Assert.Equal(WebSocketState.Closed, cws.State);
256234
}
257235

258-
[OuterLoop] // TODO: Issue #11345
259236
[ConditionalFact(nameof(WebSocketsSupported))]
260237
public void Dispose_CreateAndDispose_ExpectedPropertyValues()
261238
{

0 commit comments

Comments
 (0)