From 31bf248832d6358112b8b66c906da24344da7600 Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Mon, 5 Apr 2021 23:40:25 -0700 Subject: [PATCH 1/6] add tests --- .../Net/Http/HttpClientHandlerTest.Proxy.cs | 100 ++++++++++++++++-- 1 file changed, 93 insertions(+), 7 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs index 2eb655ee7779f..76cb288e765d5 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -322,21 +322,108 @@ public async Task Proxy_SslProxyUnsupported_Throws() } } - [OuterLoop("Uses external server")] [Fact] - public async Task Proxy_SendSecureRequestThruProxy_ConnectTunnelUsed() + public async Task ProxyTunnelRequest_GetAsync_Success() { + const string Content = "Hello world"; + using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) { HttpClientHandler handler = CreateHttpClientHandler(); handler.Proxy = new WebProxy(proxyServer.Uri); + handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; using (HttpClient client = CreateHttpClient(handler)) - using (HttpResponseMessage response = await client.GetAsync(Configuration.Http.SecureRemoteEchoServer)) { - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - _output.WriteLine($"Proxy request line: {proxyServer.Requests[0].RequestLine}"); - Assert.Contains("CONNECT", proxyServer.Requests[0].RequestLine); + var options = new LoopbackServer.Options { UseSsl = true }; + await LoopbackServer.CreateServerAsync(async (server, uri) => + { + Task clientTask = client.GetAsync(uri); + await server.AcceptConnectionSendResponseAndCloseAsync(content: Content); + using (var response = await clientTask) + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(Content, await response.Content.ReadAsStringAsync()); + } + }, options); } + + Assert.Contains("CONNECT", proxyServer.Requests[0].RequestLine); + } + } + + [ActiveIssue("TODO")] + [Fact] + public async Task ProxyTunnelRequest_MaxConnectionsSetButDoesNotApplyToProxyConnect_Success() + { + const string Content = "Hello world"; + + using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) + { + HttpClientHandler handler = CreateHttpClientHandler(); + handler.Proxy = new WebProxy(proxyServer.Uri); + handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; + handler.MaxConnectionsPerServer = 1; + using (HttpClient client = CreateHttpClient(handler)) + { + var options = new LoopbackServer.Options { UseSsl = true }; + await LoopbackServer.CreateServerAsync(async (server1, uri1) => + { + await LoopbackServer.CreateServerAsync(async (server2, uri2) => + { + Task clientTask1 = client.GetAsync(uri1); + Task clientTask2 = client.GetAsync(uri2); + await server1.AcceptConnectionAsync(async connection1 => + { + await server2.AcceptConnectionAsync(async connection2 => + { + await connection1.HandleRequestAsync(content: Content); + await connection2.HandleRequestAsync(content: Content); + }); + }); + + using (var response1 = await clientTask1) + { + Assert.Equal(HttpStatusCode.OK, response1.StatusCode); + Assert.Equal(Content, await response1.Content.ReadAsStringAsync()); + } + + using (var response2 = await clientTask2) + { + Assert.Equal(HttpStatusCode.OK, response2.StatusCode); + Assert.Equal(Content, await response2.Content.ReadAsStringAsync()); + } + }, options); + }, options); + } + + Assert.Contains("CONNECT", proxyServer.Requests[0].RequestLine); + Assert.Contains("CONNECT", proxyServer.Requests[1].RequestLine); + } + } + + [Fact] + public async Task ProxyTunnelRequest_OriginServerSendsProxyAuthChallenge_NoProxyAuthPerformed() + { + using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) + { + HttpClientHandler handler = CreateHttpClientHandler(); + handler.Proxy = new WebProxy(proxyServer.Uri) { Credentials = ConstructCredentials(new NetworkCredential("username", "password"), proxyServer.Uri, BasicAuth, true) }; + handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; + using (HttpClient client = CreateHttpClient(handler)) + { + var options = new LoopbackServer.Options { UseSsl = true }; + await LoopbackServer.CreateServerAsync(async (server, uri) => + { + Task clientTask = client.GetAsync(uri); + await server.AcceptConnectionSendResponseAndCloseAsync(statusCode: HttpStatusCode.ProxyAuthenticationRequired, additionalHeaders: "Proxy-Authenticate: Basic"); + using (var response = await clientTask) + { + Assert.Equal(HttpStatusCode.ProxyAuthenticationRequired, response.StatusCode); + } + }, options); + } + + Assert.Contains("CONNECT", proxyServer.Requests[0].RequestLine); } } @@ -375,7 +462,6 @@ await LoopbackServer.CreateServerAsync(async (proxyServer, proxyUrl) => Assert.Equal(HttpStatusCode.OK, clientTask.Result.StatusCode); } }, options); - } [Fact] From 6c36c695baf066a047259990a393fc076befebbd Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Mon, 5 Apr 2021 23:45:58 -0700 Subject: [PATCH 2/6] add and use DoProxyAuth --- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 159dff77692f0..51f366e4cbf27 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 @@ -290,7 +290,6 @@ private static SslClientAuthenticationOptions ConstructSslOptions(HttpConnection public HttpConnectionSettings Settings => _poolManager.Settings; public HttpConnectionKind Kind => _kind; public bool IsSecure => _kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel; - public bool AnyProxyKind => (_proxyUri != null); public Uri? ProxyUri => _proxyUri; public ICredentials? ProxyCredentials => _poolManager.ProxyCredentials; public byte[]? HostHeaderValueBytes => _hostHeaderValueBytes; @@ -1169,9 +1168,11 @@ public async Task SendWithNtConnectionAuthAsync(HttpConnect } } + public bool DoProxyAuth() => (_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect); + public Task SendWithNtProxyAuthAsync(HttpConnection connection, HttpRequestMessage request, bool async, CancellationToken cancellationToken) { - if (AnyProxyKind && ProxyCredentials != null) + if (DoProxyAuth() && ProxyCredentials is not null) { return AuthenticationHelper.SendWithNtProxyAuthAsync(request, ProxyUri!, async, ProxyCredentials, connection, this, cancellationToken); } @@ -1182,10 +1183,9 @@ public Task SendWithNtProxyAuthAsync(HttpConnection connect public ValueTask SendWithProxyAuthAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - if ((_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect) && - _poolManager.ProxyCredentials != null) + if (DoProxyAuth() && ProxyCredentials is not null) { - return AuthenticationHelper.SendWithProxyAuthAsync(request, _proxyUri!, async, _poolManager.ProxyCredentials, doRequestAuth, this, cancellationToken); + return AuthenticationHelper.SendWithProxyAuthAsync(request, _proxyUri!, async, ProxyCredentials, doRequestAuth, this, cancellationToken); } return SendWithRetryAsync(request, async, doRequestAuth, cancellationToken); From c746cfd921def90709d518b7ed1dcffc50e3b5f2 Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Mon, 5 Apr 2021 23:58:34 -0700 Subject: [PATCH 3/6] don't apply max connections to ProxyConnect connections --- .../System/Net/Http/HttpClientHandlerTest.Proxy.cs | 1 - .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 11 +++++++---- .../SocketsHttpHandler/HttpConnectionPoolManager.cs | 7 ++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs index 76cb288e765d5..4e05aae741068 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -351,7 +351,6 @@ await LoopbackServer.CreateServerAsync(async (server, uri) => } } - [ActiveIssue("TODO")] [Fact] public async Task ProxyTunnelRequest_MaxConnectionsSetButDoesNotApplyToProxyConnect_Success() { 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 51f366e4cbf27..8e9a35ea9f04d 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 @@ -105,13 +105,12 @@ internal sealed class HttpConnectionPool : IDisposable /// The port with which this pool is associated. /// The SSL host with which this pool is associated. /// The proxy this pool targets (optional). - /// The maximum number of connections allowed to be associated with the pool at any given time. - public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri, int maxConnections) + public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionKind kind, string? host, int port, string? sslHostName, Uri? proxyUri) { _poolManager = poolManager; _kind = kind; _proxyUri = proxyUri; - _maxConnections = maxConnections; + _maxConnections = Settings._maxConnectionsPerServer; if (host != null) { @@ -174,6 +173,11 @@ public HttpConnectionPool(HttpConnectionPoolManager poolManager, HttpConnectionK Debug.Assert(sslHostName == null); Debug.Assert(proxyUri != null); + // Don't enforce the max connections limit on proxy tunnels; this would mean that connections to different origin servers + // would compete for the same limited number of connections. + // We will still enforce this limit on the user of the tunnel (i.e. ProxyTunnel or SslProxyTunnel). + _maxConnections = int.MaxValue; + _http2Enabled = false; _http3Enabled = false; break; @@ -1180,7 +1184,6 @@ public Task SendWithNtProxyAuthAsync(HttpConnection connect return connection.SendAsync(request, async, cancellationToken); } - public ValueTask SendWithProxyAuthAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { if (DoProxyAuth() && ProxyCredentials is not null) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs index c7f70725895ec..b8fafcca13614 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs @@ -39,9 +39,7 @@ internal sealed class HttpConnectionPoolManager : IDisposable private readonly Timer? _cleaningTimer; /// Heart beat timer currently used for Http2 ping only. private readonly Timer? _heartBeatTimer; - /// The maximum number of connections allowed per pool. indicates unlimited. - private readonly int _maxConnectionsPerServer; - // Temporary + private readonly HttpConnectionSettings _settings; private readonly IWebProxy? _proxy; private readonly ICredentials? _proxyCredentials; @@ -60,7 +58,6 @@ internal sealed class HttpConnectionPoolManager : IDisposable public HttpConnectionPoolManager(HttpConnectionSettings settings) { _settings = settings; - _maxConnectionsPerServer = settings._maxConnectionsPerServer; _pools = new ConcurrentDictionary(); // As an optimization, we can sometimes avoid the overheads associated with @@ -321,7 +318,7 @@ public ValueTask SendAsyncCore(HttpRequestMessage request, HttpConnectionPool? pool; while (!_pools.TryGetValue(key, out pool)) { - pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri, _maxConnectionsPerServer); + pool = new HttpConnectionPool(this, key.Kind, key.Host, key.Port, key.SslHostName, key.ProxyUri); if (_cleaningTimer == null) { From 886d1b0d1bf12262c8d5bbde11417235558d77be Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Tue, 6 Apr 2021 09:20:29 -0700 Subject: [PATCH 4/6] make DoProxyAuth a property --- .../Net/Http/SocketsHttpHandler/HttpConnectionPool.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 8e9a35ea9f04d..3deaab8007173 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 @@ -1172,11 +1172,11 @@ public async Task SendWithNtConnectionAuthAsync(HttpConnect } } - public bool DoProxyAuth() => (_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect); + private bool DoProxyAuth => (_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect); public Task SendWithNtProxyAuthAsync(HttpConnection connection, HttpRequestMessage request, bool async, CancellationToken cancellationToken) { - if (DoProxyAuth() && ProxyCredentials is not null) + if (DoProxyAuth && ProxyCredentials is not null) { return AuthenticationHelper.SendWithNtProxyAuthAsync(request, ProxyUri!, async, ProxyCredentials, connection, this, cancellationToken); } @@ -1186,7 +1186,7 @@ public Task SendWithNtProxyAuthAsync(HttpConnection connect public ValueTask SendWithProxyAuthAsync(HttpRequestMessage request, bool async, bool doRequestAuth, CancellationToken cancellationToken) { - if (DoProxyAuth() && ProxyCredentials is not null) + if (DoProxyAuth && ProxyCredentials is not null) { return AuthenticationHelper.SendWithProxyAuthAsync(request, _proxyUri!, async, ProxyCredentials, doRequestAuth, this, cancellationToken); } From 4661b4dbacdef8fd34955d5adcdbf547955a0a67 Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Tue, 6 Apr 2021 20:59:47 -0700 Subject: [PATCH 5/6] disable WinHttpHandler for new tests --- .../Net/Http/HttpClientHandlerTest.Proxy.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs index 4e05aae741068..b36c3306aae63 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -325,6 +325,11 @@ public async Task Proxy_SslProxyUnsupported_Throws() [Fact] public async Task ProxyTunnelRequest_GetAsync_Success() { + if (IsWinHttpHandler) + { + return; + } + const string Content = "Hello world"; using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) @@ -354,6 +359,11 @@ await LoopbackServer.CreateServerAsync(async (server, uri) => [Fact] public async Task ProxyTunnelRequest_MaxConnectionsSetButDoesNotApplyToProxyConnect_Success() { + if (IsWinHttpHandler) + { + return; + } + const string Content = "Hello world"; using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) @@ -403,6 +413,11 @@ await server2.AcceptConnectionAsync(async connection2 => [Fact] public async Task ProxyTunnelRequest_OriginServerSendsProxyAuthChallenge_NoProxyAuthPerformed() { + if (IsWinHttpHandler) + { + return; + } + using (LoopbackProxyServer proxyServer = LoopbackProxyServer.Create()) { HttpClientHandler handler = CreateHttpClientHandler(); From 6ac8a3c15207bff107091a1c95794d22413fa646 Mon Sep 17 00:00:00 2001 From: Geoffrey Kizer Date: Tue, 6 Apr 2021 21:10:56 -0700 Subject: [PATCH 6/6] improve test robustness --- .../tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs | 7 +++++++ .../Common/tests/System/Net/Http/LoopbackProxyServer.cs | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs index b36c3306aae63..0ba85110c6765 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -342,6 +342,8 @@ public async Task ProxyTunnelRequest_GetAsync_Success() var options = new LoopbackServer.Options { UseSsl = true }; await LoopbackServer.CreateServerAsync(async (server, uri) => { + Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri)); + Task clientTask = client.GetAsync(uri); await server.AcceptConnectionSendResponseAndCloseAsync(content: Content); using (var response = await clientTask) @@ -379,6 +381,9 @@ await LoopbackServer.CreateServerAsync(async (server1, uri1) => { await LoopbackServer.CreateServerAsync(async (server2, uri2) => { + Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri1)); + Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri2)); + Task clientTask1 = client.GetAsync(uri1); Task clientTask2 = client.GetAsync(uri2); await server1.AcceptConnectionAsync(async connection1 => @@ -428,6 +433,8 @@ public async Task ProxyTunnelRequest_OriginServerSendsProxyAuthChallenge_NoProxy var options = new LoopbackServer.Options { UseSsl = true }; await LoopbackServer.CreateServerAsync(async (server, uri) => { + Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri)); + Task clientTask = client.GetAsync(uri); await server.AcceptConnectionSendResponseAndCloseAsync(statusCode: HttpStatusCode.ProxyAuthenticationRequired, additionalHeaders: "Proxy-Authenticate: Basic"); using (var response = await clientTask) diff --git a/src/libraries/Common/tests/System/Net/Http/LoopbackProxyServer.cs b/src/libraries/Common/tests/System/Net/Http/LoopbackProxyServer.cs index 2429e0302d9ff..0f4999b703497 100644 --- a/src/libraries/Common/tests/System/Net/Http/LoopbackProxyServer.cs +++ b/src/libraries/Common/tests/System/Net/Http/LoopbackProxyServer.cs @@ -143,7 +143,12 @@ private async Task ProcessRequest(Socket clientSocket, StreamReader reader } var request = new ReceivedRequest(); - _requests.Add(request); + + // Avoid concurrent writes to the request list + lock (_requests) + { + _requests.Add(request); + } request.RequestLine = line; string[] requestTokens = request.RequestLine.Split(' ');