Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

some small proxy-related fixes #50770

Merged
merged 6 commits into from
Apr 11, 2021
Merged

some small proxy-related fixes #50770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
31bf248
add tests
Apr 6, 2021
6c36c69
add and use DoProxyAuth
Apr 6, 2021
c746cfd
don't apply max connections to ProxyConnect connections
Apr 6, 2021
886d1b0
make DoProxyAuth a property
Apr 6, 2021
4661b4d
disable WinHttpHandler for new tests
Apr 7, 2021
6ac8a3c
improve test robustness
Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 114 additions & 7 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,129 @@ public async Task Proxy_SslProxyUnsupported_Throws()
}
}

[OuterLoop("Uses external server")]
[Fact]
public async Task Proxy_SendSecureRequestThruProxy_ConnectTunnelUsed()
public async Task ProxyTunnelRequest_GetAsync_Success()
{
if (IsWinHttpHandler)
{
return;
}

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) =>
{
Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri));

Task<HttpResponseMessage> 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);
}
}

[Fact]
public async Task ProxyTunnelRequest_MaxConnectionsSetButDoesNotApplyToProxyConnect_Success()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we set the pool idle timeout to infinite here? I assume the failure condition for this test relies on

runtime/src/libraries/Common/src/System/Net/Http/HttpHandlerDefaults.cs

Line 28 in 9089931

public static readonly TimeSpan DefaultPooledConnectionIdleTimeout = TimeSpan.FromMinutes(2);

being larger than

runtime/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs

Line 19 in 9089931

private static readonly TimeSpan s_defaultTimeout = TimeSpan.FromSeconds(100);

otherwise it could pass even without this change (after the first pool expired)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We accept both connections before sending the responses. So it shouldn't matter.

{
if (IsWinHttpHandler)
{
return;
}

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) =>
{
Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri1));
Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri2));

Task<HttpResponseMessage> clientTask1 = client.GetAsync(uri1);
Task<HttpResponseMessage> 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()
{
if (IsWinHttpHandler)
{
return;
}

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) =>
{
Assert.Equal(proxyServer.Uri, handler.Proxy.GetProxy(uri));

Task<HttpResponseMessage> 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);
}
}

Expand Down Expand Up @@ -375,7 +483,6 @@ public async Task ProxyAuth_Digest_Succeeds()
Assert.Equal(HttpStatusCode.OK, clientTask.Result.StatusCode);
}
}, options);

}

[Fact]
Expand Down
7 changes: 6 additions & 1 deletion src/libraries/Common/tests/System/Net/Http/LoopbackProxyServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,12 @@ private async Task<bool> 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(' ');
Expand Down
21 changes: 12 additions & 9 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,12 @@ internal sealed class HttpConnectionPool : IDisposable
/// <param name="port">The port with which this pool is associated.</param>
/// <param name="sslHostName">The SSL host with which this pool is associated.</param>
/// <param name="proxyUri">The proxy this pool targets (optional).</param>
/// <param name="maxConnections">The maximum number of connections allowed to be associated with the pool at any given time.</param>
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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -290,7 +294,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;
Expand Down Expand Up @@ -1169,23 +1172,23 @@ public async Task<HttpResponseMessage> SendWithNtConnectionAuthAsync(HttpConnect
}
}

private bool DoProxyAuth => (_kind == HttpConnectionKind.Proxy || _kind == HttpConnectionKind.ProxyConnect);

public Task<HttpResponseMessage> 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);
}

return connection.SendAsync(request, async, cancellationToken);
}


public ValueTask<HttpResponseMessage> 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);
Expand Down
7 changes: 2 additions & 5 deletions ...aries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ internal sealed class HttpConnectionPoolManager : IDisposable
private readonly Timer? _cleaningTimer;
/// <summary>Heart beat timer currently used for Http2 ping only.</summary>
private readonly Timer? _heartBeatTimer;
/// <summary>The maximum number of connections allowed per pool. <see cref="int.MaxValue"/> indicates unlimited.</summary>
private readonly int _maxConnectionsPerServer;
// Temporary

private readonly HttpConnectionSettings _settings;
private readonly IWebProxy? _proxy;
private readonly ICredentials? _proxyCredentials;
Expand All @@ -60,7 +58,6 @@ internal sealed class HttpConnectionPoolManager : IDisposable
public HttpConnectionPoolManager(HttpConnectionSettings settings)
{
_settings = settings;
_maxConnectionsPerServer = settings._maxConnectionsPerServer;
_pools = new ConcurrentDictionary<HttpConnectionKey, HttpConnectionPool>();

// As an optimization, we can sometimes avoid the overheads associated with
Expand Down Expand Up @@ -321,7 +318,7 @@ public ValueTask<HttpResponseMessage> 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)
{
Expand Down