Skip to content

Commit 331ed0d

Browse files
MpdreamzMpdreamz
authored
Fix NRE when using bad authentication and ping is enabled. (#86)
1 parent 9a60bfe commit 331ed0d

File tree

8 files changed

+41
-27
lines changed

8 files changed

+41
-27
lines changed

‎src/Elastic.Transport/Components/Pipeline/DefaultRequestPipeline.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class DefaultRequestPipeline<TConfiguration> : RequestPipeline
3535

3636
private static readonly ActivitySource _activitySource = new("Elastic.Transport.RequestPipeline");
3737

38-
private RequestConfiguration _pingAndSniffRequestConfiguration;
38+
private RequestConfiguration? _pingAndSniffRequestConfiguration;
3939
private List<Audit> _auditTrail = null;
4040

4141
/// <inheritdoc cref="RequestPipeline" />
@@ -73,7 +73,7 @@ private RequestConfiguration PingAndSniffRequestConfiguration
7373
{
7474
PingTimeout = PingTimeout,
7575
RequestTimeout = PingTimeout,
76-
AuthenticationHeader = _settings.Authentication,
76+
AuthenticationHeader = RequestConfiguration?.AuthenticationHeader ?? _settings.Authentication,
7777
EnableHttpPipelining = RequestConfiguration?.EnableHttpPipelining ?? _settings.HttpPipeliningEnabled,
7878
ForceNode = RequestConfiguration?.ForceNode
7979
};
@@ -277,14 +277,14 @@ public override async Task<TResponse> CallProductEndpointAsync<TResponse>(Reques
277277
}
278278
}
279279

280-
public override TransportException CreateClientException<TResponse>(
281-
TResponse response, ApiCallDetails callDetails, RequestData data, List<PipelineException> pipelineExceptions)
280+
public override TransportException? CreateClientException<TResponse>(TResponse response, ApiCallDetails? callDetails,
281+
RequestData data, List<PipelineException> seenExceptions)
282282
{
283283
if (callDetails?.HasSuccessfulStatusCodeAndExpectedContentType ?? false) return null;
284284

285285
var pipelineFailure = data.OnFailurePipelineFailure;
286286
var innerException = callDetails?.OriginalException;
287-
if (pipelineExceptions.HasAny(out var exs))
287+
if (seenExceptions.HasAny(out var exs))
288288
{
289289
pipelineFailure = exs.Last().FailureReason;
290290
innerException = exs.AsAggregateOrFirst();

‎src/Elastic.Transport/Components/Pipeline/PipelineException.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public PipelineException(PipelineFailure failure, Exception innerException)
3535
|| FailureReason == PipelineFailure.PingFailure;
3636

3737
/// <summary> The response that triggered this exception </summary>
38-
public TransportResponse Response { get; internal set; }
38+
public TransportResponse? Response { get; internal set; }
3939

4040
private static string GetMessage(PipelineFailure failure) =>
4141
failure switch

‎src/Elastic.Transport/Components/Pipeline/RequestData.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ MemoryStreamFactory memoryStreamFactory
140140
public MemoryStreamFactory MemoryStreamFactory { get; }
141141
public HttpMethod Method { get; }
142142

143-
public Node Node
143+
public Node? Node
144144
{
145145
get => _node;
146146
set
@@ -240,7 +240,7 @@ internal bool ValidateResponseContentType(string responseMimeType)
240240
// - 404 responses from ES8 don't include the vendored header
241241
// - ES8 EQL responses don't include vendored type
242242

243-
|| trimmedAccept.Contains("application/vnd.elasticsearch+json") && trimmedResponseMimeType.StartsWith(DefaultMimeType, StringComparison.OrdinalIgnoreCase);
243+
|| trimmedAccept.Contains("application/vnd.elasticsearch+json") && trimmedResponseMimeType.StartsWith(DefaultMimeType, StringComparison.OrdinalIgnoreCase);
244244
}
245245

246246
public static string ToQueryString(NameValueCollection collection) => collection.ToQueryString();

‎src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,13 @@ public abstract void BadResponse<TResponse>(ref TResponse response, ApiCallDetai
9494

9595
public abstract void AuditCancellationRequested();
9696

97-
public abstract TransportException CreateClientException<TResponse>(TResponse response, ApiCallDetails callDetails, RequestData data,
98-
List<PipelineException> seenExceptions)
97+
public abstract TransportException? CreateClientException<TResponse>(TResponse? response, ApiCallDetails? callDetails,
98+
RequestData data, List<PipelineException> seenExceptions)
9999
where TResponse : TransportResponse, new();
100100
#pragma warning restore 1591
101101

102102
/// <summary>
103-
///
103+
///
104104
/// </summary>
105105
public void Dispose()
106106
{
@@ -109,7 +109,7 @@ public void Dispose()
109109
}
110110

111111
/// <summary>
112-
///
112+
///
113113
/// </summary>
114114
/// <param name="disposing"></param>
115115
protected virtual void Dispose(bool disposing)
@@ -126,7 +126,7 @@ protected virtual void Dispose(bool disposing)
126126
}
127127

128128
/// <summary>
129-
///
129+
///
130130
/// </summary>
131131
protected virtual void DisposeManagedResources() { }
132132
}

‎src/Elastic.Transport/DefaultHttpTransport.cs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,21 +126,20 @@ public DefaultHttpTransport(
126126
private RequestPipelineFactory<TConfiguration> PipelineProvider { get; }
127127

128128
/// <summary>
129-
///
129+
///
130130
/// </summary>
131131
public override TConfiguration Settings { get; }
132132

133133
/// <summary>
134-
///
134+
///
135135
/// </summary>
136136
/// <typeparam name="TResponse"></typeparam>
137137
/// <param name="method"></param>
138138
/// <param name="path"></param>
139139
/// <param name="data"></param>
140140
/// <param name="requestParameters"></param>
141141
/// <returns></returns>
142-
public override TResponse Request<TResponse>(HttpMethod method, string path, PostData data = null,
143-
RequestParameters requestParameters = null)
142+
public override TResponse Request<TResponse>(HttpMethod method, string path, PostData? data = null, RequestParameters? requestParameters = null)
144143
{
145144
using var pipeline =
146145
PipelineProvider.Create(Settings, DateTimeProvider, MemoryStreamFactory, requestParameters);
@@ -217,7 +216,7 @@ public override TResponse Request<TResponse>(HttpMethod method, string path, Pos
217216
}
218217

219218
/// <summary>
220-
///
219+
///
221220
/// </summary>
222221
/// <typeparam name="TResponse"></typeparam>
223222
/// <param name="method"></param>
@@ -228,7 +227,7 @@ public override TResponse Request<TResponse>(HttpMethod method, string path, Pos
228227
/// <returns></returns>
229228
/// <exception cref="UnexpectedTransportException"></exception>
230229
public override async Task<TResponse> RequestAsync<TResponse>(HttpMethod method, string path,
231-
PostData data = null, RequestParameters requestParameters = null,
230+
PostData? data = null, RequestParameters? requestParameters = null,
232231
CancellationToken cancellationToken = default)
233232
{
234233
using var pipeline =
@@ -343,7 +342,7 @@ ICollection<PipelineException> seenExceptions
343342

344343
private TResponse FinalizeResponse<TResponse>(RequestData requestData, RequestPipeline pipeline,
345344
List<PipelineException> seenExceptions,
346-
TResponse response
345+
TResponse? response
347346
) where TResponse : TransportResponse, new()
348347
{
349348
if (requestData.Node == null) //foreach never ran
@@ -359,11 +358,11 @@ TResponse response
359358
return response;
360359
}
361360

362-
private static ApiCallDetails GetMostRecentCallDetails<TResponse>(TResponse response,
361+
private static ApiCallDetails? GetMostRecentCallDetails<TResponse>(TResponse? response,
363362
IEnumerable<PipelineException> seenExceptions)
364363
where TResponse : TransportResponse, new()
365364
{
366-
var callDetails = response?.ApiCallDetails ?? seenExceptions.LastOrDefault(e => e.Response.ApiCallDetails != null)?.Response.ApiCallDetails;
365+
var callDetails = response?.ApiCallDetails ?? seenExceptions.LastOrDefault(e => e.Response?.ApiCallDetails != null)?.Response?.ApiCallDetails;
367366
return callDetails;
368367
}
369368

‎src/Elastic.Transport/HttpTransport.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@ public abstract class HttpTransport
1818
public abstract TResponse Request<TResponse>(
1919
HttpMethod method,
2020
string path,
21-
PostData data = null,
22-
RequestParameters requestParameters = null)
21+
PostData? data = null,
22+
RequestParameters? requestParameters = null)
2323
where TResponse : TransportResponse, new();
2424

2525
/// <inheritdoc cref="Request{TResponse}" />
2626
public abstract Task<TResponse> RequestAsync<TResponse>(
2727
HttpMethod method,
2828
string path,
29-
PostData data = null,
30-
RequestParameters requestParameters = null,
29+
PostData? data = null,
30+
RequestParameters? requestParameters = null,
3131
CancellationToken cancellationToken = default)
3232
where TResponse : TransportResponse, new();
3333
}

‎tests/Elastic.Elasticsearch.IntegrationTests/DefaultCluster.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Elastic.Elasticsearch.Ephemeral;
66
using Elastic.Elasticsearch.Xunit;
77
using Elastic.Transport;
8+
using Elastic.Transport.Products.Elasticsearch;
89
using Xunit;
910
using Xunit.Abstractions;
1011
using static Elastic.Elasticsearch.Ephemeral.ClusterAuthentication;
@@ -28,7 +29,7 @@ public DefaultHttpTransport CreateClient(ITestOutputHelper output) =>
2829
: "localhost");
2930
var nodes = NodesUris(hostName);
3031
var connectionPool = new StaticNodePool(nodes);
31-
var settings = new TransportConfiguration(connectionPool)
32+
var settings = new TransportConfiguration(connectionPool, productRegistration: ElasticsearchProductRegistration.Default)
3233
.Proxy(new Uri("http://ipv4.fiddler:8080"), null!, null!)
3334
.RequestTimeout(TimeSpan.FromSeconds(5))
3435
.ServerCertificateValidationCallback(CertificateValidations.AllowAll)

‎tests/Elastic.Elasticsearch.IntegrationTests/SecurityClusterTests.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,18 @@ public void SyncRequestDoesNotThrow()
2929
response.ApiCallDetails.Should().NotBeNull();
3030
response.ApiCallDetails.HasSuccessfulStatusCode.Should().BeTrue();
3131
}
32+
33+
[Fact]
34+
public void SyncRequestDoesNotThrowOnBadAuth()
35+
{
36+
var response = Transport.Request<StringResponse>(GET, "/", null, new DefaultRequestParameters
37+
{
38+
RequestConfiguration = new RequestConfiguration
39+
{
40+
AuthenticationHeader = new BasicAuthentication("unknown-user", "bad-password")
41+
}
42+
});
43+
response.ApiCallDetails.Should().NotBeNull();
44+
response.ApiCallDetails.HasSuccessfulStatusCode.Should().BeFalse();
45+
}
3246
}

0 commit comments

Comments
 (0)