diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index e97aec5094c73..2c03bfb4e7390 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -591,12 +591,6 @@ Requesting HTTP version {0} with version policy {1} while server offers only version fallback. - - Synchronous operation is not supported when a ConnectCallback is specified on the SocketsHttpHandler instance. - - - Synchronous operation is not supported when a PlaintextStreamFilter is specified on the SocketsHttpHandler instance. - An exception occurred while invoking the PlaintextStreamFilter. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs index 47f260eec04b0..ab88b3578df1a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs @@ -37,13 +37,8 @@ internal static bool IsGloballyEnabled() // SendAsyncCore returns already completed ValueTask for when async: false is passed. // Internally, it calls the synchronous Send method of the base class. - protected internal override HttpResponseMessage Send(HttpRequestMessage request, - CancellationToken cancellationToken) - { - ValueTask sendTask = SendAsyncCore(request, async: false, cancellationToken); - Debug.Assert(sendTask.IsCompleted); - return sendTask.GetAwaiter().GetResult(); - } + protected internal override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) => + SendAsyncCore(request, async: false, cancellationToken).AsTask().GetAwaiter().GetResult(); protected internal override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => SendAsyncCore(request, async: true, cancellationToken).AsTask(); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index e966271b7e586..6f2b2112b9e1a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1835,6 +1835,7 @@ private enum SettingId : ushort public sealed override async Task SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { + Debug.Assert(async); if (NetEventSource.Log.IsEnabled()) Trace($"{request}"); try diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index c149d9bed8494..5f0c3cf885be5 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -158,6 +158,8 @@ private void CheckForShutdown() public override async Task SendAsync(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { + Debug.Assert(async); + // Wait for an available stream (based on QUIC MAX_STREAMS) if there isn't one available yet. TaskCompletionSourceWithCancellation? waitForAvailableStreamTcs = null; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index ca579ec3ae403..485b600e9c5a0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -511,15 +511,26 @@ public async Task SendAsyncCore(HttpRequestMessage request, if (t != null) { // Handle the pre-emptive read. For the async==false case, hopefully the read has - // already completed and this will be a nop, but if it hasn't, we're forced to block + // already completed and this will be a nop, but if it hasn't, the caller will be forced to block // waiting for the async operation to complete. We will only hit this case for proxied HTTPS // requests that use a pooled connection, as in that case we don't have a Socket we // can poll and are forced to issue an async read. ValueTask vt = t.GetValueOrDefault(); - int bytesRead = - vt.IsCompletedSuccessfully ? vt.Result : - async ? await vt.ConfigureAwait(false) : - vt.AsTask().GetAwaiter().GetResult(); + int bytesRead; + if (vt.IsCompleted) + { + bytesRead = vt.Result; + } + else + { + if (!async) + { + Trace($"Pre-emptive read completed asynchronously for a synchronous request."); + } + + bytesRead = await vt.ConfigureAwait(false); + } + if (NetEventSource.Log.IsEnabled()) Trace($"Received {bytesRead} bytes."); if (bytesRead == 0) @@ -635,14 +646,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, { Task sendTask = sendRequestContentTask; sendRequestContentTask = null; - if (async) - { - await sendTask.ConfigureAwait(false); - } - else - { - sendTask.GetAwaiter().GetResult(); - } + await sendTask.ConfigureAwait(false); } // Now we are sure that the request was fully sent. @@ -735,15 +739,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, { try { - if (async) - { - await sendRequestContentTask.ConfigureAwait(false); - } - else - { - // No way around it here if we want to get the exception from the task. - sendRequestContentTask.GetAwaiter().GetResult(); - } + await sendRequestContentTask.ConfigureAwait(false); } // Map the exception the same way as we normally do. catch (Exception ex) when (MapSendException(ex, cancellationToken, out Exception mappedEx)) 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 43fdcd46a9b51..8af9648b25907 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 @@ -486,8 +486,8 @@ public byte[] Http2AltSvcOriginUri conn.Dispose(); } - // We are at the connection limit. Wait for an available connection or connection count (indicated by null). - if (NetEventSource.Log.IsEnabled()) Trace("Connection limit reached, waiting for available connection."); + // We are at the connection limit. Wait for an available connection or connection count. + if (NetEventSource.Log.IsEnabled()) Trace($"{(async ? "As" : "S")}ynchronous request. Connection limit reached, waiting for available connection."); if (HttpTelemetry.Log.IsEnabled()) { @@ -495,24 +495,14 @@ public byte[] Http2AltSvcOriginUri } else { - return async ? - waiter.WaitWithCancellationAsync(cancellationToken) : - new ValueTask(waiter.Task.GetAwaiter().GetResult()); + return waiter.WaitWithCancellationAsync(cancellationToken); } static async ValueTask WaitOnWaiterWithTelemetryAsync(TaskCompletionSourceWithCancellation waiter, bool async, CancellationToken cancellationToken) { ValueStopwatch stopwatch = ValueStopwatch.StartNew(); - HttpConnection? connection; - if (async) - { - connection = await waiter.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); - } - else - { - connection = waiter.Task.GetAwaiter().GetResult(); - } + HttpConnection? connection = await waiter.WaitWithCancellationAsync(cancellationToken).ConfigureAwait(false); HttpTelemetry.Log.Http11RequestLeftQueue(stopwatch.GetElapsedTime().TotalMilliseconds); return connection; @@ -615,7 +605,7 @@ public byte[] Http2AltSvcOriginUri if (_kind == HttpConnectionKind.Http) { - http2Connection = await ConstructHttp2Connection(stream, request, cancellationToken).ConfigureAwait(false); + http2Connection = await ConstructHttp2ConnectionAsync(stream, request, cancellationToken).ConfigureAwait(false); if (NetEventSource.Log.IsEnabled()) { @@ -637,7 +627,7 @@ public byte[] Http2AltSvcOriginUri throw new HttpRequestException(SR.Format(SR.net_ssl_http2_requires_tls12, sslStream.SslProtocol)); } - http2Connection = await ConstructHttp2Connection(stream, request, cancellationToken).ConfigureAwait(false); + http2Connection = await ConstructHttp2ConnectionAsync(stream, request, cancellationToken).ConfigureAwait(false); if (NetEventSource.Log.IsEnabled()) { @@ -691,7 +681,7 @@ public byte[] Http2AltSvcOriginUri if (canUse) { - return (await ConstructHttp11Connection(stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true, null); + return (await ConstructHttp11ConnectionAsync(async, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), true, null); } else { @@ -1254,7 +1244,7 @@ public ValueTask SendAsync(HttpRequestMessage request, bool case HttpConnectionKind.ProxyTunnel: case HttpConnectionKind.SslProxyTunnel: HttpResponseMessage? response; - (stream, response) = await EstablishProxyTunnel(async, request.HasHeaders ? request.Headers : null, cancellationToken).ConfigureAwait(false); + (stream, response) = await EstablishProxyTunnelAsync(async, request.HasHeaders ? request.Headers : null, cancellationToken).ConfigureAwait(false); if (response != null) { // Return non-success response from proxy. @@ -1295,22 +1285,16 @@ private async ValueTask ConnectToTcpHostAsync(string host, int port, Htt { ValueTask streamTask = Settings._connectCallback(new SocketsHttpConnectionContext(endPoint, initialRequest), cancellationToken); - Stream stream; - if (async || streamTask.IsCompleted) - { - stream = await streamTask.ConfigureAwait(false); - } - else + if (!async && !streamTask.IsCompleted) { // User-provided ConnectCallback is completing asynchronously but the user is making a synchronous request; if the user cares, they should // set it up so that synchronous requests are made on a handler with a synchronously-completing ConnectCallback supplied. If in the future, // we could add a Boolean to SocketsHttpConnectionContext (https://github.com/dotnet/runtime/issues/44876) to let the callback know whether - // this request is sync or async. For now, log it and block. + // this request is sync or async. Trace($"{nameof(SocketsHttpHandler.ConnectCallback)} completing asynchronously for a synchronous request."); - stream = streamTask.AsTask().GetAwaiter().GetResult(); } - return stream ?? throw new HttpRequestException(SR.net_http_null_from_connect_callback); + return await streamTask.ConfigureAwait(false) ?? throw new HttpRequestException(SR.net_http_null_from_connect_callback); } else { @@ -1351,7 +1335,7 @@ private async ValueTask ConnectToTcpHostAsync(string host, int port, Htt return (null, failureResponse); } - return (await ConstructHttp11Connection(stream!, transportContext, request, cancellationToken).ConfigureAwait(false), null); + return (await ConstructHttp11ConnectionAsync(async, stream!, transportContext, request, cancellationToken).ConfigureAwait(false), null); } private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessage request) @@ -1371,7 +1355,7 @@ private SslClientAuthenticationOptions GetSslOptionsForRequest(HttpRequestMessag return _sslOptionsHttp11!; } - private async ValueTask ApplyPlaintextFilter(Stream stream, Version httpVersion, HttpRequestMessage request, CancellationToken cancellationToken) + private async ValueTask ApplyPlaintextFilterAsync(bool async, Stream stream, Version httpVersion, HttpRequestMessage request, CancellationToken cancellationToken) { if (Settings._plaintextStreamFilter is null) { @@ -1381,7 +1365,18 @@ private async ValueTask ApplyPlaintextFilter(Stream stream, Version http Stream newStream; try { - newStream = await Settings._plaintextStreamFilter(new SocketsHttpPlaintextStreamFilterContext(stream, httpVersion, request), cancellationToken).ConfigureAwait(false); + ValueTask streamTask = Settings._plaintextStreamFilter(new SocketsHttpPlaintextStreamFilterContext(stream, httpVersion, request), cancellationToken); + + if (!async && !streamTask.IsCompleted) + { + // User-provided PlaintextStreamFilter is completing asynchronously but the user is making a synchronous request; if the user cares, they should + // set it up so that synchronous requests are made on a handler with a synchronously-completing PlaintextStreamFilter supplied. If in the future, + // we could add a Boolean to SocketsHttpPlaintextStreamFilterContext (https://github.com/dotnet/runtime/issues/44876) to let the callback know whether + // this request is sync or async. + Trace($"{nameof(SocketsHttpHandler.PlaintextStreamFilter)} completing asynchronously for a synchronous request."); + } + + newStream = await streamTask.ConfigureAwait(false); } catch (Exception e) { @@ -1398,15 +1393,15 @@ private async ValueTask ApplyPlaintextFilter(Stream stream, Version http return newStream; } - private async ValueTask ConstructHttp11Connection(Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken) + private async ValueTask ConstructHttp11ConnectionAsync(bool async, Stream stream, TransportContext? transportContext, HttpRequestMessage request, CancellationToken cancellationToken) { - stream = await ApplyPlaintextFilter(stream, HttpVersion.Version11, request, cancellationToken).ConfigureAwait(false); + stream = await ApplyPlaintextFilterAsync(async, stream, HttpVersion.Version11, request, cancellationToken).ConfigureAwait(false); return new HttpConnection(this, stream, transportContext); } - private async ValueTask ConstructHttp2Connection(Stream stream, HttpRequestMessage request, CancellationToken cancellationToken) + private async ValueTask ConstructHttp2ConnectionAsync(Stream stream, HttpRequestMessage request, CancellationToken cancellationToken) { - stream = await ApplyPlaintextFilter(stream, HttpVersion.Version20, request, cancellationToken).ConfigureAwait(false); + stream = await ApplyPlaintextFilterAsync(async: true, stream, HttpVersion.Version20, request, cancellationToken).ConfigureAwait(false); Http2Connection http2Connection = new Http2Connection(this, stream); await http2Connection.SetupAsync().ConfigureAwait(false); @@ -1418,7 +1413,7 @@ private async ValueTask ConstructHttp2Connection(Stream stream, // Returns the established stream or an HttpResponseMessage from the proxy indicating failure. - private async ValueTask<(Stream?, HttpResponseMessage?)> EstablishProxyTunnel(bool async, HttpRequestHeaders? headers, CancellationToken cancellationToken) + private async ValueTask<(Stream?, HttpResponseMessage?)> EstablishProxyTunnelAsync(bool async, HttpRequestHeaders? headers, CancellationToken cancellationToken) { Debug.Assert(_originAuthority != null); // Send a CONNECT request to the proxy server to establish a tunnel. diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpMessageHandlerStage.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpMessageHandlerStage.cs index 72b91c9e49f38..abe2eb90ad20c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpMessageHandlerStage.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpMessageHandlerStage.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics; using System.Threading; using System.Threading.Tasks; @@ -12,9 +11,10 @@ internal abstract class HttpMessageHandlerStage : HttpMessageHandler protected internal sealed override HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken) { - ValueTask sendTask = SendAsync(request, async:false, cancellationToken); - Debug.Assert(sendTask.IsCompleted); - return sendTask.GetAwaiter().GetResult(); + ValueTask sendTask = SendAsync(request, async: false, cancellationToken); + return sendTask.IsCompleted ? + sendTask.Result : + sendTask.AsTask().GetAwaiter().GetResult(); } protected internal sealed override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs index 7e2631ec1541e..003a33a8c66c6 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocketsHttpHandler.cs @@ -513,11 +513,6 @@ protected internal override HttpResponseMessage Send(HttpRequestMessage request, CheckDisposed(); HttpMessageHandlerStage handler = _handler ?? SetupHandlerChain(); - if (_settings._plaintextStreamFilter is not null) - { - throw new NotSupportedException(SR.net_http_sync_operations_not_allowed_with_plaintext_filter); - } - Exception? error = ValidateAndNormalizeRequest(request); if (error != null) { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index b019a1e89855b..05eaa63ad0b10 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -2372,6 +2372,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync( else { await s.ConnectAsync(context.DnsEndPoint, token); + await Task.Delay(1); // to increase the chances of the whole operation completing asynchronously, without consuming too much additional time } return new NetworkStream(s, ownsSocket: true); }; @@ -2626,24 +2627,22 @@ public abstract class SocketsHttpHandlerTest_PlaintextStreamFilter : HttpClientH { public SocketsHttpHandlerTest_PlaintextStreamFilter(ITestOutputHelper output) : base(output) { } - [Fact] - public void PlaintextStreamFilter_SyncRequest_Fails() - { - using SocketsHttpHandler handler = new SocketsHttpHandler - { - PlaintextStreamFilter = (context, token) => default, - }; - - using HttpClient client = CreateHttpClient(handler); - - Assert.ThrowsAny(() => client.Send(new HttpRequestMessage(HttpMethod.Get, "http://bing.com"))); - } + public static IEnumerable PlaintextStreamFilter_ContextHasCorrectProperties_Success_MemberData() => + from useSsl in new[] { false, true } + from syncRequest in new[] { false, true } + from syncCallback in new[] { false, true } + select new object[] { useSsl, syncRequest, syncCallback }; [Theory] - [InlineData(true)] - [InlineData(false)] - public async Task PlaintextStreamFilter_ContextHasCorrectProperties_Success(bool useSsl) + [MemberData(nameof(PlaintextStreamFilter_ContextHasCorrectProperties_Success_MemberData))] + public async Task PlaintextStreamFilter_ContextHasCorrectProperties_Success(bool useSsl, bool syncRequest, bool syncCallback) { + if (syncRequest && UseVersion > HttpVersion.Version11) + { + // Sync requests are only supported on 1.x + return; + } + GenericLoopbackOptions options = new GenericLoopbackOptions() { UseSsl = useSsl }; await LoopbackServerFactory.CreateClientAndServerAsync( async uri => @@ -2655,17 +2654,24 @@ await LoopbackServerFactory.CreateClientAndServerAsync( using HttpClientHandler handler = CreateHttpClientHandler(); handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates; var socketsHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler); - socketsHandler.PlaintextStreamFilter = (context, token) => + socketsHandler.PlaintextStreamFilter = async (context, token) => { Assert.Equal(UseVersion, context.NegotiatedHttpVersion); Assert.Equal(requestMessage, context.InitialRequestMessage); - return ValueTask.FromResult(context.PlaintextStream); + if (!syncCallback) + { + await Task.Delay(1); // to increase the chances of the whole operation completing asynchronously, without consuming too much additional time + } + + return context.PlaintextStream; }; using HttpClient client = CreateHttpClient(handler); - HttpResponseMessage response = await client.SendAsync(requestMessage); + HttpResponseMessage response = await (syncRequest ? + Task.Run(() => client.Send(requestMessage)) : + client.SendAsync(requestMessage)); Assert.Equal("foo", await response.Content.ReadAsStringAsync()); }, async server =>