Do not set HttpRequestMessage content when no PostData (#3925)

This commit changes the HttpConnection to not set Content on HttpRequestMessage when there is no PostData. The check is performed inside of Request and RequestAsync to avoid creating any content instance and allocations.

Pass RequestData through to _onStreamAvailable delegates so that HttpCompression, ConnectionSettings and PostData can be accessed on the passed instance.

Fixes #3907

Author: russcam

src/Elasticsearch.Net/Connection/Content/RequestDataContent.cs

@@ -25,8 +25,7 @@ namespace Elasticsearch.Net
 	internal class RequestDataContent : HttpContent
 	{
 		private readonly RequestData _requestData;
-		private readonly Func<PostData, CompleteTaskOnCloseStream, RequestDataContent, TransportContext, Task> _onStreamAvailable;
-
+		private readonly Func<RequestData, CompleteTaskOnCloseStream, RequestDataContent, TransportContext, Task> _onStreamAvailable;
 
 		public RequestDataContent(RequestData requestData)
 		{
@@ -35,14 +34,17 @@ public RequestDataContent(RequestData requestData)
 			if (requestData.HttpCompression)
 				Headers.ContentEncoding.Add("gzip");
 
-			Task OnStreamAvailable(PostData data, Stream stream, HttpContent content, TransportContext context)
+			Task OnStreamAvailable(RequestData data, Stream stream, HttpContent content, TransportContext context)
 			{
-				if (_requestData.HttpCompression)
+				if (data.HttpCompression)
 					stream = new GZipStream(stream, CompressionMode.Compress, false);
+
 				using(stream)
-					data.Write(stream, requestData.ConnectionSettings);
+					data.PostData.Write(stream, data.ConnectionSettings);
+
 				return Task.CompletedTask;
 			}
+
 			_onStreamAvailable = OnStreamAvailable;
 		}
 		public RequestDataContent(RequestData requestData, CancellationToken token)
@@ -52,13 +54,15 @@ public RequestDataContent(RequestData requestData, CancellationToken token)
 			if (requestData.HttpCompression)
 				Headers.ContentEncoding.Add("gzip");
 
-			async Task OnStreamAvailable(PostData data, Stream stream, HttpContent content, TransportContext context)
+			async Task OnStreamAvailable(RequestData data, Stream stream, HttpContent content, TransportContext context)
 			{
-				if (_requestData.HttpCompression)
+				if (data.HttpCompression)
 					stream = new GZipStream(stream, CompressionMode.Compress, false);
+
 				using (stream)
-					await data.WriteAsync(stream, requestData.ConnectionSettings, token).ConfigureAwait(false);
+					await data.PostData.WriteAsync(stream, data.ConnectionSettings, token).ConfigureAwait(false);
 			}
+
 			_onStreamAvailable = OnStreamAvailable;
 		}
 
@@ -73,14 +77,9 @@ async Task OnStreamAvailable(PostData data, Stream stream, HttpContent content,
 		[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Exception is passed as task result.")]
 		protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context)
 		{
-
-			var data = _requestData.PostData;
-			if (data == null) return;
-
 			var serializeToStreamTask = new TaskCompletionSource<bool>();
-
 			var wrappedStream = new CompleteTaskOnCloseStream(stream, serializeToStreamTask);
-            await _onStreamAvailable(data, wrappedStream, this, context).ConfigureAwait(false);
+            await _onStreamAvailable(_requestData, wrappedStream, this, context).ConfigureAwait(false);
             await serializeToStreamTask.Task.ConfigureAwait(false);
 		}
 
@@ -113,7 +112,6 @@ protected override void Dispose(bool disposing)
 				base.Dispose();
 			}
 
-
 			public override void Close() => _serializeToStreamTask.TrySetResult(true);
 		}
 
@@ -195,6 +193,8 @@ public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, As
 			public override void EndWrite(IAsyncResult asyncResult) => _innerStream.EndWrite(asyncResult);
 
 			public override void WriteByte(byte value) => _innerStream.WriteByte(value);
+
+			public override void Close() => _innerStream.Close();
 		}
 	}
 }

src/Elasticsearch.Net/Connection/HttpConnection.cs

@@ -57,7 +57,10 @@ public virtual TResponse Request<TResponse>(RequestData requestData)
 			try
 			{
 				var requestMessage = CreateHttpRequestMessage(requestData);
-				SetContent(requestMessage, requestData);
+
+				if (requestData.PostData != null)
+					SetContent(requestMessage, requestData);
+
 				using(requestMessage?.Content ?? (IDisposable)Stream.Null)
 				using (var d = DiagnosticSource.Diagnose<RequestData, int?>(DiagnosticSources.HttpConnection.SendAndReceiveHeaders, requestData))
 				{
@@ -107,8 +110,11 @@ public virtual async Task<TResponse> RequestAsync<TResponse>(RequestData request
 			try
 			{
 				var requestMessage = CreateHttpRequestMessage(requestData);
-				SetAsyncContent(requestMessage, requestData, cancellationToken);
-				using(requestMessage?.Content ?? (IDisposable)Stream.Null) 
+
+				if (requestData.PostData != null)
+					SetAsyncContent(requestMessage, requestData, cancellationToken);
+
+				using(requestMessage?.Content ?? (IDisposable)Stream.Null)
 				using (var d = DiagnosticSource.Diagnose<RequestData, int?>(DiagnosticSources.HttpConnection.SendAndReceiveHeaders, requestData))
 				{
 					responseMessage = await client.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);

src/Tests/Tests.Reproduce/GithubIssue3907.cs

@@ -0,0 +1,32 @@
+using System;
+using System.Net;
+using Elastic.Xunit.XunitPlumbing;
+using FluentAssertions;
+using Nest;
+using Tests.Core.ManagedElasticsearch.Clusters;
+using Tests.Domain;
+
+namespace Tests.Reproduce
+{
+	public class GithubIssue3907 : IClusterFixture<ReadOnlyCluster>
+	{
+		private readonly IntrusiveOperationCluster _cluster;
+
+		// use intrusive operation cluster because we're changing the underlying http handler
+		// and this cluster runs with a max concurrency of 1, so changing http handler
+		// will not affect other integration tests
+		public GithubIssue3907(IntrusiveOperationCluster cluster) => _cluster = cluster;
+
+		[I]
+		public void NotUsingSocketsHttpHandlerDoesNotCauseException()
+		{
+			AppContext.SetSwitch("System.Net.Http.UseSocketsHttpHandler", false);
+
+			var response = _cluster.Client.Indices.Exists("non_existent_index");
+			response.ApiCall.HttpStatusCode.Should().Be(404);
+			response.OriginalException.Should().BeNull();
+
+			AppContext.SetSwitch("System.Net.Http.UseSocketsHttpHandler", true);
+		}
+	}
+}