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..208ac1ad2916c 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -280,9 +280,8 @@ public async Task AuthenticatedProxiedRequest_GetAsyncWithNoCreds_ProxyAuthentic } } - [OuterLoop("Uses external server")] [Fact] - public async Task AuthenticatedProxyTunnelRequest_PostAsyncWithNoCreds_ProxyAuthenticationRequiredStatusCode() + public async Task AuthenticatedProxyTunnelRequest_PostAsyncWithNoCreds_Throws() { if (IsWinHttpHandler) { @@ -300,14 +299,13 @@ public async Task AuthenticatedProxyTunnelRequest_PostAsyncWithNoCreds_ProxyAuth handler.Proxy = new WebProxy(proxyServer.Uri); handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; using (HttpClient client = CreateHttpClient(handler)) - using (HttpResponseMessage response = await client.PostAsync(Configuration.Http.SecureRemoteEchoServer, new StringContent(content))) { - Assert.Equal(HttpStatusCode.ProxyAuthenticationRequired, response.StatusCode); + HttpRequestException e = await Assert.ThrowsAnyAsync(async () => await client.PostAsync("https://nosuchhost.invalid", new StringContent(content))); + Assert.Contains("407", e.Message); } } } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsSubsystemForLinux))] // [ActiveIssue("https://github.com/dotnet/runtime/issues/18258")] public async Task Proxy_SslProxyUnsupported_Throws() { diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index 2c03bfb4e7390..501437eb2b0e5 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -606,4 +606,7 @@ Synchronous reads are not supported, use ReadAsync instead. + + The proxy tunnel request to proxy '{0}' failed with status code '{1}'." + diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs index 9f254a7318a11..c16c2b18b9012 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/AuthenticationHelper.NtAuth.cs @@ -63,15 +63,7 @@ private static async Task SendWithNtAuthAsync(HttpRequestMe if (response.Headers.ConnectionClose.GetValueOrDefault()) { // Server is closing the connection and asking us to authenticate on a new connection. - // expression returns null connection on error, was not able to use '!' for the expression -#pragma warning disable CS8600 - (connection, response) = await connectionPool.CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); -#pragma warning restore CS8600 - if (response != null) - { - return response; - } - + connection = await connectionPool.CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); connectionPool.IncrementConnectionCount(); connection!.Acquire(); isNewConnection = 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 159dff77692f0..aa4f860cc48b9 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 @@ -334,7 +334,7 @@ public byte[] Http2AltSvcOriginUri /// Object used to synchronize access to state in the pool. private object SyncObj => _idleConnections; - private ValueTask<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + private ValueTask<(HttpConnectionBase connection, bool isNewConnection)> GetConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { // Do not even attempt at getting/creating a connection if it's already obvious we cannot provided the one requested. @@ -342,12 +342,12 @@ public byte[] Http2AltSvcOriginUri { if (request.Version.Major == 3 && !_http3Enabled) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>( new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 3))); } if (request.Version.Major == 2 && !_http2Enabled) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>( new HttpRequestException(SR.Format(SR.net_http_requested_version_not_enabled, request.Version, request.VersionPolicy, 2))); } } @@ -365,7 +365,7 @@ public byte[] Http2AltSvcOriginUri { if (IsAltSvcBlocked(authority)) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>( new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); } @@ -376,7 +376,7 @@ public byte[] Http2AltSvcOriginUri // If we got here, we cannot provide HTTP/3 connection. Do not continue if downgrade is not allowed. if (request.Version.Major >= 3 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>( new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 3))); } @@ -389,11 +389,11 @@ public byte[] Http2AltSvcOriginUri // If we got here, we cannot provide HTTP/2 connection. Do not continue if downgrade is not allowed. if (request.Version.Major >= 2 && request.VersionPolicy != HttpVersionPolicy.RequestVersionOrLower) { - return ValueTask.FromException<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)>( + return ValueTask.FromException<(HttpConnectionBase connection, bool isNewConnection)>( new HttpRequestException(SR.Format(SR.net_http_requested_version_cannot_establish, request.Version, request.VersionPolicy, 2))); } - return GetHttpConnectionAsync(request, async, cancellationToken); + return GetHttp11ConnectionAsync(request, async, cancellationToken); } private ValueTask GetOrReserveHttp11ConnectionAsync(bool async, CancellationToken cancellationToken) @@ -494,27 +494,21 @@ public byte[] Http2AltSvcOriginUri } } - private async ValueTask<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> - GetHttpConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) + private async ValueTask<(HttpConnectionBase connection, bool isNewConnection)> + GetHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { HttpConnection? connection = await GetOrReserveHttp11ConnectionAsync(async, cancellationToken).ConfigureAwait(false); if (connection != null) { - return (connection, false, null); + return (connection, false); } if (NetEventSource.Log.IsEnabled()) Trace("Creating new connection for pool."); try { - HttpResponseMessage? failureResponse; - (connection, failureResponse) = await CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); - if (connection == null) - { - Debug.Assert(failureResponse != null); - DecrementConnectionCount(); - } - return (connection, true, failureResponse); + connection = await CreateHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); + return (connection, true); } catch { @@ -523,7 +517,7 @@ public byte[] Http2AltSvcOriginUri } } - private async ValueTask<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + private async ValueTask<(HttpConnectionBase connection, bool isNewConnection)> GetHttp2ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { Debug.Assert(_kind == HttpConnectionKind.Https || _kind == HttpConnectionKind.SslProxyTunnel || _kind == HttpConnectionKind.Http); @@ -536,7 +530,7 @@ public byte[] Http2AltSvcOriginUri // Connection exists and it is still good to use. if (NetEventSource.Log.IsEnabled()) Trace("Using existing HTTP2 connection."); _usedSinceLastCleanup = true; - return (http2Connection, false, null); + return (http2Connection, false); } // Ensure that the connection creation semaphore is created @@ -564,7 +558,7 @@ public byte[] Http2AltSvcOriginUri http2Connection = GetExistingHttp2Connection(); if (http2Connection != null) { - return (http2Connection, false, null); + return (http2Connection, false); } // Recheck if HTTP2 has been disabled by a previous attempt. @@ -575,16 +569,9 @@ public byte[] Http2AltSvcOriginUri Trace("Attempting new HTTP2 connection."); } - HttpResponseMessage? failureResponse; - - (socket, stream, transportContext, failureResponse) = + (socket, stream, transportContext) = await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false); - if (failureResponse != null) - { - return (null, true, failureResponse); - } - Debug.Assert(stream != null); sslStream = stream as SslStream; @@ -598,7 +585,7 @@ public byte[] Http2AltSvcOriginUri Trace("New unencrypted HTTP2 connection established."); } - return (http2Connection, true, null); + return (http2Connection, true); } Debug.Assert(sslStream != null); @@ -620,7 +607,7 @@ public byte[] Http2AltSvcOriginUri Trace("New HTTP2 connection established."); } - return (http2Connection, true, null); + return (http2Connection, true); } } } @@ -667,7 +654,7 @@ public byte[] Http2AltSvcOriginUri if (canUse) { - return (await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true, null); + return (await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true); } else { @@ -681,7 +668,7 @@ public byte[] Http2AltSvcOriginUri } // If we reach this point, it means we need to fall back to a (new or existing) HTTP/1.1 connection. - return await GetHttpConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); + return await GetHttp11ConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); } private Http2Connection? GetExistingHttp2Connection() @@ -732,7 +719,7 @@ private void AddHttp2Connection(Http2Connection newConnection) } } - private async ValueTask<(HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse)> + private async ValueTask<(HttpConnectionBase connection, bool isNewConnection)> GetHttp3ConnectionAsync(HttpRequestMessage request, HttpAuthority authority, CancellationToken cancellationToken) { Debug.Assert(_kind == HttpConnectionKind.Https); @@ -755,7 +742,7 @@ private void AddHttp2Connection(Http2Connection newConnection) // Connection exists and it is still good to use. if (NetEventSource.Log.IsEnabled()) Trace("Using existing HTTP3 connection."); _usedSinceLastCleanup = true; - return (http3Connection, false, null); + return (http3Connection, false); } } @@ -783,7 +770,7 @@ private void AddHttp2Connection(Http2Connection newConnection) Trace("Using existing HTTP3 connection."); } - return (_http3Connection, false, null); + return (_http3Connection, false); } if (NetEventSource.Log.IsEnabled()) @@ -820,7 +807,7 @@ private void AddHttp2Connection(Http2Connection newConnection) Trace("New HTTP3 connection established."); } - return (http3Connection, true, null); + return (http3Connection, true); } finally { @@ -834,14 +821,7 @@ public async ValueTask SendWithRetryAsync(HttpRequestMessag { // Loop on connection failures and retry if possible. - (HttpConnectionBase? connection, bool isNewConnection, HttpResponseMessage? failureResponse) = await GetConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); - if (failureResponse != null) - { - // Proxy tunnel failure; return proxy response - Debug.Assert(isNewConnection); - Debug.Assert(connection == null); - return failureResponse; - } + (HttpConnectionBase connection, bool isNewConnection) = await GetConnectionAsync(request, async, cancellationToken).ConfigureAwait(false); HttpResponseMessage response; @@ -1201,7 +1181,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool return SendWithProxyAuthAsync(request, async, doRequestAuth, cancellationToken); } - private async ValueTask<(Socket?, Stream?, TransportContext?, HttpResponseMessage?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) + private async ValueTask<(Socket?, Stream, TransportContext?)> ConnectAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { // If a non-infinite connect timeout has been set, create and use a new CancellationToken that will be canceled // when either the original token is canceled or a connect timeout occurs. @@ -1232,14 +1212,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool case HttpConnectionKind.ProxyTunnel: case HttpConnectionKind.SslProxyTunnel: - HttpResponseMessage? response; - (stream, response) = await EstablishProxyTunnelAsync(async, request.HasHeaders ? request.Headers : null, cancellationToken).ConfigureAwait(false); - if (response != null) - { - // Return non-success response from proxy. - response.RequestMessage = request; - return (null, null, null, response); - } + stream = await EstablishProxyTunnelAsync(async, request.HasHeaders ? request.Headers : null, cancellationToken).ConfigureAwait(false); break; } @@ -1259,7 +1232,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool stream = sslStream; } - return (socket, stream, transportContext, null); + return (socket, stream, transportContext); } finally { @@ -1323,17 +1296,12 @@ public ValueTask SendAsync(HttpRequestMessage request, bool } } - internal async ValueTask<(HttpConnection?, HttpResponseMessage?)> CreateHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) + internal async ValueTask CreateHttp11ConnectionAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { - (Socket? socket, Stream? stream, TransportContext? transportContext, HttpResponseMessage? failureResponse) = + (Socket? socket, Stream? stream, TransportContext? transportContext) = await ConnectAsync(request, async, cancellationToken).ConfigureAwait(false); - if (failureResponse != null) - { - return (null, failureResponse); - } - - return (await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), null); + return await ConstructHttp11ConnectionAsync(async, socket, stream!, transportContext, request, cancellationToken).ConfigureAwait(false); } private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessage request) @@ -1414,11 +1382,10 @@ private async ValueTask ConstructHttp2ConnectionAsync(Stream st return http2Connection; } - - // Returns the established stream or an HttpResponseMessage from the proxy indicating failure. - private async ValueTask<(Stream?, HttpResponseMessage?)> EstablishProxyTunnelAsync(bool async, HttpRequestHeaders? headers, CancellationToken cancellationToken) + private async ValueTask EstablishProxyTunnelAsync(bool async, HttpRequestHeaders? headers, CancellationToken cancellationToken) { Debug.Assert(_originAuthority != null); + // Send a CONNECT request to the proxy server to establish a tunnel. HttpRequestMessage tunnelRequest = new HttpRequestMessage(HttpMethod.Connect, _proxyUri); tunnelRequest.Headers.Host = $"{_originAuthority.IdnHost}:{_originAuthority.Port}"; // This specifies destination host/port to connect to @@ -1432,12 +1399,19 @@ private async ValueTask ConstructHttp2ConnectionAsync(Stream st if (tunnelResponse.StatusCode != HttpStatusCode.OK) { - return (null, tunnelResponse); + tunnelResponse.Dispose(); + throw new HttpRequestException(SR.Format(SR.net_http_proxy_tunnel_returned_failure_status_code, _proxyUri, (int)tunnelResponse.StatusCode)); } - Stream stream = tunnelResponse.Content.ReadAsStream(cancellationToken); - - return (stream, null); + try + { + return tunnelResponse.Content.ReadAsStream(cancellationToken); + } + catch + { + tunnelResponse.Dispose(); + throw; + } } /// Enqueues a waiter to the waiters list.