From 72cddd3709d50e551d46489018b82d20b75187b9 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Wed, 24 May 2023 19:42:59 +0200 Subject: [PATCH] Fix HTTP/1.1 concurrent request content read race condition --- .../Net/Http/SocketsHttpHandler/HttpConnection.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 76f904d79b203..e0128ac76a327 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 @@ -66,7 +66,6 @@ internal sealed partial class HttpConnection : HttpConnectionBase private bool _inUse; private bool _detachedFromPool; private bool _canRetry; - private bool _startedSendingRequestBody; private bool _connectionClose; // Connection: close was seen on last response private const int Status_Disposed = 1; @@ -524,7 +523,6 @@ public async Task SendAsync(HttpRequestMessage request, boo HttpMethod normalizedMethod = HttpMethod.Normalize(request.Method); _canRetry = false; - _startedSendingRequestBody = false; // Send the request. if (NetEventSource.Log.IsEnabled()) Trace($"Sending request: {request}"); @@ -630,7 +628,7 @@ public async Task SendAsync(HttpRequestMessage request, boo // The server shutdown the connection on their end, likely because of an idle timeout. // If we haven't started sending the request body yet (or there is no request body), // then we allow the request to be retried. - if (!_startedSendingRequestBody) + if (request.Content is null || allowExpect100ToContinue is not null) { _canRetry = true; } @@ -825,7 +823,11 @@ public async Task SendAsync(HttpRequestMessage request, boo cancellationRegistration.Dispose(); // Make sure to complete the allowExpect100ToContinue task if it exists. - allowExpect100ToContinue?.TrySetResult(false); + if (allowExpect100ToContinue is not null && !allowExpect100ToContinue.TrySetResult(false)) + { + // allowExpect100ToContinue was already signaled and we may have started sending the request body. + _canRetry = false; + } if (_readAheadTask != default) { @@ -939,9 +941,6 @@ private CancellationTokenRegistration RegisterCancellation(CancellationToken can private async ValueTask SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream, bool async, CancellationToken cancellationToken) { - // Now that we're sending content, prohibit retries of this request by setting this flag. - _startedSendingRequestBody = true; - Debug.Assert(stream.BytesWritten == 0); if (HttpTelemetry.Log.IsEnabled()) HttpTelemetry.Log.RequestContentStart();