Skip to content

Commit c6f929a

Browse files
russcamStuart Cam
russcam
authored and
Stuart Cam
committed
Fix default chunked transfer encoding and expose as configurable (#3958)
This commit reverts the behaviour of the client to not use chunked tranfer encoding by default, and exposes chunked transfer encoding as a configuration value on IConnectionConfigurationValues and RequestConfiguration. With the introduction of the pull based streaming model in RequestDataContent, a ContentLength value cannot be computed for what will be written to the request stream; a ContentLength value is attempted to be determined, and TryComputeLength() executed, _before_ PostData is serialized to the request stream, making it not possible to provide a ContentLength value in the implementation, and falling into the HttpClient behaviour of using chunked transfer encoding. A potential fix could have been applied in RequestDataContent, to serialize PostData ahead of time and store in a byte array or stream field, and the logical place to do this would have been the ctor. It was determined however that this would be less than optimal to do because 1. It starts to overcomplicate the RequestDataContent implementation 2. An implementation in the RequestDataContent ctor would not have been able to use aynchronous methods for the RequestAsync<T> asynchronous path. Instead, the serialization of PostData happens in sync and async paths in HttpConnection, using ByteArrayContent and the written request bytes when DisabledDirectStreaming is enabled to avoid keeping two copies of the request bytes around. Additional integration tests added for HttpConnection and HttpWebRequestConnection to assert behaviour. Fixes #3952 * Update default value * Update test name * Update test name
1 parent cd96ef9 commit c6f929a

File tree

8 files changed

+320
-39
lines changed

8 files changed

+320
-39
lines changed

‎src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
145145
private bool _sniffOnConnectionFault;
146146
private bool _sniffOnStartup;
147147
private bool _throwExceptions;
148+
private bool _transferEncodingChunked;
148149

149150
private string _userAgent = ConnectionConfiguration.DefaultUserAgent;
150151
private Func<HttpMethod, int, bool> _statusCodeToResponseSuccess;
@@ -213,6 +214,7 @@ protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection co
213214
ElasticsearchUrlFormatter IConnectionConfigurationValues.UrlFormatter => _urlFormatter;
214215
string IConnectionConfigurationValues.UserAgent => _userAgent;
215216
Func<HttpMethod, int, bool> IConnectionConfigurationValues.StatusCodeToResponseSuccess => _statusCodeToResponseSuccess;
217+
bool IConnectionConfigurationValues.TransferEncodingChunked => _transferEncodingChunked;
216218

217219
void IDisposable.Dispose() => DisposeManagedResources();
218220

@@ -530,6 +532,11 @@ public T SkipDeserializationForStatusCodes(params int[] statusCodes) =>
530532
/// </summary>
531533
public T UserAgent(string userAgent) => Assign(userAgent, (a, v) => a._userAgent = v);
532534

535+
/// <summary>
536+
/// Whether the request should be sent with chunked Transfer-Encoding. Default is <c>false</c>
537+
/// </summary>
538+
public T TransferEncodingChunked(bool transferEncodingChunked = true) => Assign(transferEncodingChunked, (a, v) => a._transferEncodingChunked = v);
539+
533540
protected virtual void DisposeManagedResources()
534541
{
535542
_connectionPool?.Dispose();

‎src/Elasticsearch.Net/Configuration/IConnectionConfigurationValues.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,5 +244,10 @@ public interface IConnectionConfigurationValues : IDisposable
244244
/// <para>NOTE: if a request specifies <see cref="IRequestConfiguration.AllowedStatusCodes"/> this takes precedence</para>
245245
/// </summary>
246246
Func<HttpMethod, int, bool> StatusCodeToResponseSuccess { get; }
247+
248+
/// <summary>
249+
/// Whether the request should be sent with chunked Transfer-Encoding.
250+
/// </summary>
251+
bool TransferEncodingChunked { get; }
247252
}
248253
}

‎src/Elasticsearch.Net/Configuration/RequestConfiguration.cs‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,35 +106,51 @@ public interface IRequestConfiguration
106106
/// <para>Reasons for such exceptions could be search parser errors, index missing exceptions, etc...</para>
107107
/// </summary>
108108
bool? ThrowExceptions { get; set; }
109+
110+
/// <summary>
111+
/// Whether the request should be sent with chunked Transfer-Encoding.
112+
/// </summary>
113+
bool? TransferEncodingChunked { get; set; }
109114
}
110115

111116
public class RequestConfiguration : IRequestConfiguration
112117
{
118+
/// <inheritdoc />
113119
public string Accept { get; set; }
120+
/// <inheritdoc />
114121
public IReadOnlyCollection<int> AllowedStatusCodes { get; set; }
122+
/// <inheritdoc />
115123
public BasicAuthenticationCredentials BasicAuthenticationCredentials { get; set; }
116-
124+
/// <inheritdoc />
117125
public ApiKeyAuthenticationCredentials ApiKeyAuthenticationCredentials { get; set; }
118-
126+
/// <inheritdoc />
119127
public X509CertificateCollection ClientCertificates { get; set; }
128+
/// <inheritdoc />
120129
public string ContentType { get; set; }
130+
/// <inheritdoc />
121131
public bool? DisableDirectStreaming { get; set; }
132+
/// <inheritdoc />
122133
public bool? DisablePing { get; set; }
134+
/// <inheritdoc />
123135
public bool? DisableSniff { get; set; }
136+
/// <inheritdoc />
124137
public bool? EnableHttpPipelining { get; set; } = true;
138+
/// <inheritdoc />
125139
public Uri ForceNode { get; set; }
140+
/// <inheritdoc />
126141
public int? MaxRetries { get; set; }
142+
/// <inheritdoc />
127143
public string OpaqueId { get; set; }
144+
/// <inheritdoc />
128145
public TimeSpan? PingTimeout { get; set; }
146+
/// <inheritdoc />
129147
public TimeSpan? RequestTimeout { get; set; }
130-
131-
/// <summary>
132-
/// Submit the request on behalf in the context of a different user
133-
/// https://www.elastic.co/guide/en/shield/current/submitting-requests-for-other-users.html
134-
/// </summary>
148+
/// <inheritdoc />
135149
public string RunAs { get; set; }
136-
150+
/// <inheritdoc />
137151
public bool? ThrowExceptions { get; set; }
152+
/// <inheritdoc />
153+
public bool? TransferEncodingChunked { get; set; }
138154
}
139155

140156
public class RequestConfigurationDescriptor : IRequestConfiguration
@@ -158,6 +174,7 @@ public RequestConfigurationDescriptor(IRequestConfiguration config)
158174
Self.ClientCertificates = config?.ClientCertificates;
159175
Self.ThrowExceptions = config?.ThrowExceptions;
160176
Self.OpaqueId = config?.OpaqueId;
177+
Self.TransferEncodingChunked = config?.TransferEncodingChunked;
161178
}
162179

163180
string IRequestConfiguration.Accept { get; set; }
@@ -171,14 +188,14 @@ public RequestConfigurationDescriptor(IRequestConfiguration config)
171188
bool? IRequestConfiguration.DisableSniff { get; set; }
172189
bool? IRequestConfiguration.EnableHttpPipelining { get; set; } = true;
173190
Uri IRequestConfiguration.ForceNode { get; set; }
174-
175191
int? IRequestConfiguration.MaxRetries { get; set; }
176192
string IRequestConfiguration.OpaqueId { get; set; }
177193
TimeSpan? IRequestConfiguration.PingTimeout { get; set; }
178194
TimeSpan? IRequestConfiguration.RequestTimeout { get; set; }
179195
string IRequestConfiguration.RunAs { get; set; }
180196
private IRequestConfiguration Self => this;
181197
bool? IRequestConfiguration.ThrowExceptions { get; set; }
198+
bool? IRequestConfiguration.TransferEncodingChunked { get; set; }
182199

183200
/// <summary>
184201
/// Submit the request on behalf in the context of a different shield user
@@ -334,5 +351,12 @@ public RequestConfigurationDescriptor ClientCertificate(X509Certificate certific
334351
/// <summary> Use the following client certificate to authenticate this request to Elasticsearch </summary>
335352
public RequestConfigurationDescriptor ClientCertificate(string certificatePath) =>
336353
ClientCertificates(new X509Certificate2Collection { new X509Certificate(certificatePath) });
354+
355+
/// <inheritdoc cref="IRequestConfiguration.TransferEncodingChunked" />
356+
public RequestConfigurationDescriptor TransferEncodingChunked(bool? transferEncodingChunked = true)
357+
{
358+
Self.TransferEncodingChunked = transferEncodingChunked;
359+
return this;
360+
}
337361
}
338362
}

‎src/Elasticsearch.Net/Connection/HttpConnection.cs‎

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Collections.Generic;
55
using System.Diagnostics;
66
using System.IO;
7+
using System.IO.Compression;
78
using System.Net;
89
using System.Net.Http;
910
using System.Net.Http.Headers;
@@ -68,7 +69,7 @@ public virtual TResponse Request<TResponse>(RequestData requestData)
6869
statusCode = (int)responseMessage.StatusCode;
6970
d.EndState = statusCode;
7071
}
71-
72+
7273
requestData.MadeItToResponse = true;
7374
responseMessage.Headers.TryGetValues("Warning", out warnings);
7475
mimeType = responseMessage.Content.Headers.ContentType?.MediaType;
@@ -112,7 +113,7 @@ public virtual async Task<TResponse> RequestAsync<TResponse>(RequestData request
112113
var requestMessage = CreateHttpRequestMessage(requestData);
113114

114115
if (requestData.PostData != null)
115-
SetAsyncContent(requestMessage, requestData, cancellationToken);
116+
await SetContentAsync(requestMessage, requestData, cancellationToken).ConfigureAwait(false);
116117

117118
using(requestMessage?.Content ?? (IDisposable)Stream.Null)
118119
using (var d = DiagnosticSource.Diagnose<RequestData, int?>(DiagnosticSources.HttpConnection.SendAndReceiveHeaders, requestData))
@@ -121,7 +122,7 @@ public virtual async Task<TResponse> RequestAsync<TResponse>(RequestData request
121122
statusCode = (int)responseMessage.StatusCode;
122123
d.EndState = statusCode;
123124
}
124-
125+
125126
requestData.MadeItToResponse = true;
126127
mimeType = responseMessage.Content.Headers.ContentType?.MediaType;
127128
responseMessage.Headers.TryGetValues("Warning", out warnings);
@@ -253,7 +254,7 @@ protected virtual bool SetApiKeyAuthenticationIfNeeded(HttpRequestMessage reques
253254
return true;
254255

255256
}
256-
257+
257258
// TODO - make private in 8.0 and only expose SetAuthenticationIfNeeded
258259
protected virtual void SetBasicAuthenticationIfNeeded(HttpRequestMessage requestMessage, RequestData requestData)
259260
{
@@ -300,10 +301,71 @@ protected virtual HttpRequestMessage CreateRequestMessage(RequestData requestDat
300301
return requestMessage;
301302
}
302303

303-
private static void SetContent(HttpRequestMessage message, RequestData requestData) => message.Content = new RequestDataContent(requestData);
304+
private static void SetContent(HttpRequestMessage message, RequestData requestData)
305+
{
306+
if (requestData.TransferEncodingChunked)
307+
{
308+
message.Content = new RequestDataContent(requestData);
309+
}
310+
else
311+
{
312+
var stream = requestData.MemoryStreamFactory.Create();
313+
if (requestData.HttpCompression)
314+
using (var zipStream = new GZipStream(stream, CompressionMode.Compress, true))
315+
requestData.PostData.Write(zipStream, requestData.ConnectionSettings);
316+
else
317+
requestData.PostData.Write(stream, requestData.ConnectionSettings);
318+
319+
if (requestData.PostData.DisableDirectStreaming.GetValueOrDefault(false))
320+
{
321+
message.Content = new ByteArrayContent(requestData.PostData.WrittenBytes);
322+
stream.Dispose();
323+
}
324+
else
325+
{
326+
stream.Position = 0;
327+
message.Content = new StreamContent(stream);
328+
}
329+
330+
if (requestData.HttpCompression)
331+
message.Content.Headers.ContentEncoding.Add("gzip");
304332

305-
private static void SetAsyncContent(HttpRequestMessage message, RequestData requestData, CancellationToken token) =>
306-
message.Content = new RequestDataContent(requestData, token);
333+
message.Content.Headers.ContentType = new MediaTypeHeaderValue(requestData.RequestMimeType);
334+
}
335+
}
336+
337+
private static async Task SetContentAsync(HttpRequestMessage message, RequestData requestData, CancellationToken cancellationToken)
338+
{
339+
if (requestData.TransferEncodingChunked)
340+
{
341+
message.Content = new RequestDataContent(requestData, cancellationToken);
342+
}
343+
else
344+
{
345+
var stream = requestData.MemoryStreamFactory.Create();
346+
if (requestData.HttpCompression)
347+
using (var zipStream = new GZipStream(stream, CompressionMode.Compress, true))
348+
await requestData.PostData.WriteAsync(zipStream, requestData.ConnectionSettings, cancellationToken).ConfigureAwait(false);
349+
else
350+
await requestData.PostData.WriteAsync(stream, requestData.ConnectionSettings, cancellationToken).ConfigureAwait(false);
351+
352+
if (requestData.PostData.DisableDirectStreaming.GetValueOrDefault(false))
353+
{
354+
message.Content = new ByteArrayContent(requestData.PostData.WrittenBytes);
355+
stream.Dispose();
356+
}
357+
else
358+
{
359+
stream.Position = 0;
360+
message.Content = new StreamContent(stream);
361+
}
362+
363+
if (requestData.HttpCompression)
364+
message.Content.Headers.ContentEncoding.Add("gzip");
365+
366+
message.Content.Headers.ContentType = new MediaTypeHeaderValue(requestData.RequestMimeType);
367+
}
368+
}
307369

308370
private static System.Net.Http.HttpMethod ConvertHttpMethod(HttpMethod httpMethod)
309371
{

‎src/Elasticsearch.Net/Connection/HttpWebRequestConnection.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ protected virtual HttpWebRequest CreateWebRequest(RequestData requestData)
181181
#endif
182182
request.Pipelined = requestData.Pipelined;
183183

184+
if (requestData.TransferEncodingChunked)
185+
request.SendChunked = true;
186+
184187
if (requestData.HttpCompression)
185188
{
186189
request.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;

‎src/Elasticsearch.Net/Transport/Pipeline/RequestData.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Collections.Specialized;
4-
using System.IO;
5-
using System.Linq;
64
using System.Security;
75
using System.Security.Cryptography.X509Certificates;
86
using Elasticsearch.Net.Extensions;
@@ -80,6 +78,7 @@ IMemoryStreamFactory memoryStreamFactory
8078
AllowedStatusCodes = local?.AllowedStatusCodes ?? EmptyReadOnly<int>.Collection;
8179
ClientCertificates = local?.ClientCertificates ?? global.ClientCertificates;
8280
UserAgent = global.UserAgent;
81+
TransferEncodingChunked = local?.TransferEncodingChunked ?? global.TransferEncodingChunked;
8382
}
8483

8584
private readonly string _path;
@@ -122,6 +121,7 @@ IMemoryStreamFactory memoryStreamFactory
122121
public IReadOnlyCollection<int> SkipDeserializationForStatusCodes { get; }
123122
public bool ThrowExceptions { get; }
124123
public string UserAgent { get; }
124+
public bool TransferEncodingChunked { get; }
125125

126126
public Uri Uri => Node != null ? new Uri(Node.Uri, PathAndQuery) : null;
127127

0 commit comments

Comments
 (0)