Skip to content

Commit

Permalink
Fix NRE when using bad authentication and ping is enabled. (#86)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mpdreamz authored Jul 7, 2023
1 parent 9a60bfe commit 331ed0d
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public class DefaultRequestPipeline<TConfiguration> : RequestPipeline

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

private RequestConfiguration _pingAndSniffRequestConfiguration;
private RequestConfiguration? _pingAndSniffRequestConfiguration;
private List<Audit> _auditTrail = null;

/// <inheritdoc cref="RequestPipeline" />
Expand Down Expand Up @@ -73,7 +73,7 @@ private RequestConfiguration PingAndSniffRequestConfiguration
{
PingTimeout = PingTimeout,
RequestTimeout = PingTimeout,
AuthenticationHeader = _settings.Authentication,
AuthenticationHeader = RequestConfiguration?.AuthenticationHeader ?? _settings.Authentication,
EnableHttpPipelining = RequestConfiguration?.EnableHttpPipelining ?? _settings.HttpPipeliningEnabled,
ForceNode = RequestConfiguration?.ForceNode
};
Expand Down Expand Up @@ -277,14 +277,14 @@ public override async Task<TResponse> CallProductEndpointAsync<TResponse>(Reques
}
}

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

var pipelineFailure = data.OnFailurePipelineFailure;
var innerException = callDetails?.OriginalException;
if (pipelineExceptions.HasAny(out var exs))
if (seenExceptions.HasAny(out var exs))
{
pipelineFailure = exs.Last().FailureReason;
innerException = exs.AsAggregateOrFirst();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public PipelineException(PipelineFailure failure, Exception innerException)
|| FailureReason == PipelineFailure.PingFailure;

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

private static string GetMessage(PipelineFailure failure) =>
failure switch
Expand Down
4 changes: 2 additions & 2 deletions src/Elastic.Transport/Components/Pipeline/RequestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ MemoryStreamFactory memoryStreamFactory
public MemoryStreamFactory MemoryStreamFactory { get; }
public HttpMethod Method { get; }

public Node Node
public Node? Node
{
get => _node;
set
Expand Down Expand Up @@ -240,7 +240,7 @@ internal bool ValidateResponseContentType(string responseMimeType)
// - 404 responses from ES8 don't include the vendored header
// - ES8 EQL responses don't include vendored type

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

public static string ToQueryString(NameValueCollection collection) => collection.ToQueryString();
Expand Down
10 changes: 5 additions & 5 deletions src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ public abstract void BadResponse<TResponse>(ref TResponse response, ApiCallDetai

public abstract void AuditCancellationRequested();

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

/// <summary>
///
///
/// </summary>
public void Dispose()
{
Expand All @@ -109,7 +109,7 @@ public void Dispose()
}

/// <summary>
///
///
/// </summary>
/// <param name="disposing"></param>
protected virtual void Dispose(bool disposing)
Expand All @@ -126,7 +126,7 @@ protected virtual void Dispose(bool disposing)
}

/// <summary>
///
///
/// </summary>
protected virtual void DisposeManagedResources() { }
}
17 changes: 8 additions & 9 deletions src/Elastic.Transport/DefaultHttpTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,20 @@ public DefaultHttpTransport(
private RequestPipelineFactory<TConfiguration> PipelineProvider { get; }

/// <summary>
///
///
/// </summary>
public override TConfiguration Settings { get; }

/// <summary>
///
///
/// </summary>
/// <typeparam name="TResponse"></typeparam>
/// <param name="method"></param>
/// <param name="path"></param>
/// <param name="data"></param>
/// <param name="requestParameters"></param>
/// <returns></returns>
public override TResponse Request<TResponse>(HttpMethod method, string path, PostData data = null,
RequestParameters requestParameters = null)
public override TResponse Request<TResponse>(HttpMethod method, string path, PostData? data = null, RequestParameters? requestParameters = null)
{
using var pipeline =
PipelineProvider.Create(Settings, DateTimeProvider, MemoryStreamFactory, requestParameters);
Expand Down Expand Up @@ -217,7 +216,7 @@ public override TResponse Request<TResponse>(HttpMethod method, string path, Pos
}

/// <summary>
///
///
/// </summary>
/// <typeparam name="TResponse"></typeparam>
/// <param name="method"></param>
Expand All @@ -228,7 +227,7 @@ public override TResponse Request<TResponse>(HttpMethod method, string path, Pos
/// <returns></returns>
/// <exception cref="UnexpectedTransportException"></exception>
public override async Task<TResponse> RequestAsync<TResponse>(HttpMethod method, string path,
PostData data = null, RequestParameters requestParameters = null,
PostData? data = null, RequestParameters? requestParameters = null,
CancellationToken cancellationToken = default)
{
using var pipeline =
Expand Down Expand Up @@ -343,7 +342,7 @@ ICollection<PipelineException> seenExceptions

private TResponse FinalizeResponse<TResponse>(RequestData requestData, RequestPipeline pipeline,
List<PipelineException> seenExceptions,
TResponse response
TResponse? response
) where TResponse : TransportResponse, new()
{
if (requestData.Node == null) //foreach never ran
Expand All @@ -359,11 +358,11 @@ TResponse response
return response;
}

private static ApiCallDetails GetMostRecentCallDetails<TResponse>(TResponse response,
private static ApiCallDetails? GetMostRecentCallDetails<TResponse>(TResponse? response,
IEnumerable<PipelineException> seenExceptions)
where TResponse : TransportResponse, new()
{
var callDetails = response?.ApiCallDetails ?? seenExceptions.LastOrDefault(e => e.Response.ApiCallDetails != null)?.Response.ApiCallDetails;
var callDetails = response?.ApiCallDetails ?? seenExceptions.LastOrDefault(e => e.Response?.ApiCallDetails != null)?.Response?.ApiCallDetails;
return callDetails;
}

Expand Down
8 changes: 4 additions & 4 deletions src/Elastic.Transport/HttpTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ public abstract class HttpTransport
public abstract TResponse Request<TResponse>(
HttpMethod method,
string path,
PostData data = null,
RequestParameters requestParameters = null)
PostData? data = null,
RequestParameters? requestParameters = null)
where TResponse : TransportResponse, new();

/// <inheritdoc cref="Request{TResponse}" />
public abstract Task<TResponse> RequestAsync<TResponse>(
HttpMethod method,
string path,
PostData data = null,
RequestParameters requestParameters = null,
PostData? data = null,
RequestParameters? requestParameters = null,
CancellationToken cancellationToken = default)
where TResponse : TransportResponse, new();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Elastic.Elasticsearch.Ephemeral;
using Elastic.Elasticsearch.Xunit;
using Elastic.Transport;
using Elastic.Transport.Products.Elasticsearch;
using Xunit;
using Xunit.Abstractions;
using static Elastic.Elasticsearch.Ephemeral.ClusterAuthentication;
Expand All @@ -28,7 +29,7 @@ public DefaultHttpTransport CreateClient(ITestOutputHelper output) =>
: "localhost");
var nodes = NodesUris(hostName);
var connectionPool = new StaticNodePool(nodes);
var settings = new TransportConfiguration(connectionPool)
var settings = new TransportConfiguration(connectionPool, productRegistration: ElasticsearchProductRegistration.Default)
.Proxy(new Uri("http://ipv4.fiddler:8080"), null!, null!)
.RequestTimeout(TimeSpan.FromSeconds(5))
.ServerCertificateValidationCallback(CertificateValidations.AllowAll)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,18 @@ public void SyncRequestDoesNotThrow()
response.ApiCallDetails.Should().NotBeNull();
response.ApiCallDetails.HasSuccessfulStatusCode.Should().BeTrue();
}

[Fact]
public void SyncRequestDoesNotThrowOnBadAuth()
{
var response = Transport.Request<StringResponse>(GET, "/", null, new DefaultRequestParameters
{
RequestConfiguration = new RequestConfiguration
{
AuthenticationHeader = new BasicAuthentication("unknown-user", "bad-password")
}
});
response.ApiCallDetails.Should().NotBeNull();
response.ApiCallDetails.HasSuccessfulStatusCode.Should().BeFalse();
}
}

0 comments on commit 331ed0d

Please sign in to comment.