diff --git a/src/Dapr.Client/DaprClient.cs b/src/Dapr.Client/DaprClient.cs index 43c640a69..0b5db312b 100644 --- a/src/Dapr.Client/DaprClient.cs +++ b/src/Dapr.Client/DaprClient.cs @@ -78,14 +78,14 @@ public abstract class DaprClient : IDisposable /// public static HttpClient CreateInvokeHttpClient(string appId = null, string daprEndpoint = null, string daprApiToken = null) { - var handler = new InvocationHandler() + var handler = new InvocationHandler { InnerHandler = new HttpClientHandler(), DaprApiToken = daprApiToken, - DefaultAppId = appId, + DefaultAppId = appId }; - if (daprEndpoint is string) + if (daprEndpoint is not null) { // DaprEndpoint performs validation. handler.DaprEndpoint = daprEndpoint; @@ -94,7 +94,7 @@ public static HttpClient CreateInvokeHttpClient(string appId = null, string dapr var httpClient = new HttpClient(handler); httpClient.DefaultRequestHeaders.UserAgent.Add(UserAgent()); - if (appId is string) + if (appId is not null) { try { @@ -703,7 +703,7 @@ public abstract Task InvokeMethodGrpcAsync( string methodName, TRequest data, CancellationToken cancellationToken = default) - where TRequest : IMessage; + where TRequest : IMessage; /// /// Perform service invocation using gRPC semantics for the application identified by and invokes the method @@ -720,7 +720,7 @@ public abstract Task InvokeMethodGrpcAsync( string appId, string methodName, CancellationToken cancellationToken = default) - where TResponse : IMessage, new(); + where TResponse : IMessage, new(); /// /// Perform service invocation using gRPC semantics for the application identified by and invokes the method @@ -740,8 +740,8 @@ public abstract Task InvokeMethodGrpcAsync( string methodName, TRequest data, CancellationToken cancellationToken = default) - where TRequest : IMessage - where TResponse : IMessage, new(); + where TRequest : IMessage + where TResponse : IMessage, new(); /// /// Gets the current value associated with the from the Dapr state store. diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index 36fd6b77f..be61d546b 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -13,6 +13,7 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -44,6 +45,7 @@ public class InvocationHandler : DelegatingHandler { private Uri parsedEndpoint; private string? apiToken; + private readonly string[] supportedUriSchemes = { "http", "https" }; /// /// Initializes a new instance of . @@ -60,22 +62,18 @@ public InvocationHandler() /// The URI of the Dapr HTTP endpoint used for service invocation. public string DaprEndpoint { - get - { - return this.parsedEndpoint.OriginalString; - } + get => this.parsedEndpoint.OriginalString; + set { - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } + ArgumentNullException.ThrowIfNull(value); // This will throw a reasonable exception if the URI is invalid. var uri = new Uri(value, UriKind.Absolute); - if (uri.Scheme != "http" && uri.Scheme != "https") + + if (!IsUriSchemeSupported(uri.Scheme)) { - throw new ArgumentException("The URI scheme of the Dapr endpoint must be http or https.", "value"); + throw new ArgumentException("The URI scheme of the Dapr endpoint must be http or https.", nameof(value)); } this.parsedEndpoint = uri; @@ -98,63 +96,68 @@ internal string? DaprApiToken /// protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - var original = request.RequestUri; - if (!this.TryRewriteUri(request.RequestUri, out var rewritten)) + var originalUri = request.RequestUri; + + if (!this.TryRewriteUri(request.RequestUri, out var rewrittenUri)) { - throw new ArgumentException($"The request URI '{original}' is not a valid Dapr service invocation destination.", nameof(request)); + throw new ArgumentException($"The request URI '{originalUri}' is not a valid Dapr service invocation destination.", nameof(request)); } try { var apiTokenHeader = DaprClient.GetDaprApiTokenHeader(this.apiToken); + if (apiTokenHeader is not null) { request.Headers.Add(apiTokenHeader.Value.Key, apiTokenHeader.Value.Value); } - request.RequestUri = rewritten; + + request.RequestUri = rewrittenUri; return await base.SendAsync(request, cancellationToken); } finally { - request.RequestUri = original; + request.RequestUri = originalUri; request.Headers.Remove("dapr-api-token"); } } // Internal for testing - internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) + internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewrittenUri) { - // For now the only invalid cases are when the request URI is missing or just silly. - // We may support additional cases for validation in the future (like an allow-list of App-Ids). - if (uri is null || !uri.IsAbsoluteUri || (uri.Scheme != "http" && uri.Scheme != "https")) + if (!IsUriValid(uri)) { // do nothing - rewritten = null; + rewrittenUri = null; return false; } - string host; - - if (this.DefaultAppId is not null && uri.Host.Equals(this.DefaultAppId, StringComparison.InvariantCultureIgnoreCase)) - { - host = this.DefaultAppId; - } - else - { - host = uri.Host; - } + string host = UriHostContainsValidDefaultAppId(uri!.Host) ? this.DefaultAppId! : uri!.Host; var builder = new UriBuilder(uri) { Scheme = this.parsedEndpoint.Scheme, Host = this.parsedEndpoint.Host, Port = this.parsedEndpoint.Port, - Path = $"/v1.0/invoke/{host}/method" + uri.AbsolutePath, + Path = $"/v1.0/invoke/{host}/method" + uri.AbsolutePath }; - rewritten = builder.Uri; + rewrittenUri = builder.Uri; return true; } + + private bool IsUriSchemeSupported(string uriScheme) + => supportedUriSchemes.Contains(uriScheme, StringComparer.InvariantCultureIgnoreCase); + + // For now the only invalid cases are when the request URI is missing or just silly. + // We may support additional cases for validation in the future (like an allow-list of App-Ids). + private bool IsUriValid(Uri? uri) + => uri is not null && uri.IsAbsoluteUri && IsUriSchemeSupported(uri.Scheme); + + private bool UriHostContainsValidDefaultAppId(string uriHost) + => this.DefaultAppId is not null && + uriHost.Equals(this.DefaultAppId, StringComparison.InvariantCultureIgnoreCase); + } } diff --git a/test/Dapr.Client.Test/DaprClientTest.cs b/test/Dapr.Client.Test/DaprClientTest.cs index 01d22edcf..4392ba877 100644 --- a/test/Dapr.Client.Test/DaprClientTest.cs +++ b/test/Dapr.Client.Test/DaprClientTest.cs @@ -29,11 +29,7 @@ public void CreateInvokeHttpClient_WithAppId() [Fact] public void CreateInvokeHttpClient_InvalidAppId() { - var ex = Assert.Throws(() => - { - // The appId needs to be something that can be used as hostname in a URI. - _ = DaprClient.CreateInvokeHttpClient(appId: ""); - }); + var ex = Assert.Throws(() => DaprClient.CreateInvokeHttpClient(appId: string.Empty)); Assert.Contains("The appId must be a valid hostname.", ex.Message); Assert.IsType(ex.InnerException); @@ -49,10 +45,7 @@ public void CreateInvokeHttpClient_WithoutAppId() [Fact] public void CreateInvokeHttpClient_InvalidDaprEndpoint_InvalidFormat() { - Assert.Throws(() => - { - _ = DaprClient.CreateInvokeHttpClient(daprEndpoint: ""); - }); + Assert.Throws(() => DaprClient.CreateInvokeHttpClient(daprEndpoint: string.Empty)); // Exception message comes from the runtime, not validating it here. } @@ -60,10 +53,7 @@ public void CreateInvokeHttpClient_InvalidDaprEndpoint_InvalidFormat() [Fact] public void CreateInvokeHttpClient_InvalidDaprEndpoint_InvalidScheme() { - var ex = Assert.Throws(() => - { - _ = DaprClient.CreateInvokeHttpClient(daprEndpoint: "ftp://localhost:3500"); - }); + var ex = Assert.Throws(() => DaprClient.CreateInvokeHttpClient(daprEndpoint: "ftp://localhost:3500")); Assert.Contains("The URI scheme of the Dapr endpoint must be http or https.", ex.Message); } @@ -71,7 +61,7 @@ public void CreateInvokeHttpClient_InvalidDaprEndpoint_InvalidScheme() [Fact] public void GetDaprApiTokenHeader_ApiTokenSet_SetsApiTokenHeader() { - var token = "test_token"; + const string token = "test_token"; var entry = DaprClient.GetDaprApiTokenHeader(token); Assert.NotNull(entry); Assert.Equal("test_token", entry.Value.Value); @@ -89,10 +79,7 @@ public async Task TestShutdownApi() { await using var client = TestClient.CreateForDaprClient(); - var request = await client.CaptureGrpcRequestAsync(async daprClient => - { - await daprClient.ShutdownSidecarAsync(); - }); + var request = await client.CaptureGrpcRequestAsync(async daprClient => await daprClient.ShutdownSidecarAsync()); request.Dismiss(); } diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index b171ca399..ce66d5f2f 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -18,6 +18,7 @@ using System.Reflection; using System.Threading; using System.Threading.Tasks; +using Dapr.Client.Test.TestData; using Xunit; #nullable enable @@ -30,10 +31,8 @@ public class InvocationHandlerTests public void DaprEndpoint_InvalidScheme() { var handler = new InvocationHandler(); - var ex = Assert.Throws(() => - { - handler.DaprEndpoint = "ftp://localhost:3500"; - }); + + var ex = Assert.Throws(() => handler.DaprEndpoint = "ftp://localhost:3500"); Assert.Contains("The URI scheme of the Dapr endpoint must be http or https.", ex.Message); } @@ -42,13 +41,18 @@ public void DaprEndpoint_InvalidScheme() public void DaprEndpoint_InvalidUri() { var handler = new InvocationHandler(); - Assert.Throws(() => - { - handler.DaprEndpoint = ""; - }); + + Assert.Throws(() => handler.DaprEndpoint = string.Empty); // Exception message comes from the runtime, not validating it here } + + [Fact] + public void DaprEndpoint_NullDaprEndpoint() + { + var handler = new InvocationHandler(); + Assert.Throws(() => handler.DaprEndpoint = default!); + } [Fact] public void TryRewriteUri_FailsForNullUri() @@ -79,47 +83,10 @@ public void TryRewriteUri_FailsForRelativeUris() } [Theory] - [InlineData(null, "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("bank", "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("Bank", "http://bank", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("invalid", "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("Bank", "http://Bank", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("bank", "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("invalid", "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("bank", "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("invalid", "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://Bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("Bank", "http://Bank:3939", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("invalid", "http://Bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData("app-id.with.dots", "http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData("invalid", "http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData(null, "http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData("App-id.with.dots", "http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/App-id.with.dots/method/")] - [InlineData("invalid", "http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData(null, "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("bank", "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("invalid", "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("Bank", "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("invalid", "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData(null, "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData("bank", "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData("invalid", "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData(null, "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData("Bank", "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/Bank/method/some/path")] - [InlineData("invalid", "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData(null, "http://bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment")] - [InlineData("bank", "http://bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment")] - [InlineData("invalid", "http://bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment")] - [InlineData(null, "http://Bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment")] - [InlineData("Bank", "http://Bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/Bank/method/some/path?q=test&p=another#fragment")] - [InlineData("invalid", "http://Bank:3939/some/path?q=test&p=another#fragment", "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment")] + [ClassData(typeof(TryRewriteUriWithNoAppIdRewritesUriToDaprInvokeTestData))] public void TryRewriteUri_WithNoAppId_RewritesUriToDaprInvoke(string? appId, string uri, string expected) { - var handler = new InvocationHandler() + var handler = new InvocationHandler { DaprEndpoint = "https://some.host:3499", DefaultAppId = appId, @@ -144,10 +111,10 @@ public async Task SendAsync_InvalidNotSetUri_ThrowsException() [Fact] public async Task SendAsync_RewritesUri() { - var uri = "http://bank/accounts/17?"; + const string uri = "http://bank/accounts/17?"; var capture = new CaptureHandler(); - var handler = new InvocationHandler() + var handler = new InvocationHandler { InnerHandler = capture, @@ -171,7 +138,7 @@ public async Task SendAsync_RewritesUri_AndAppId() var uri = "http://bank/accounts/17?"; var capture = new CaptureHandler(); - var handler = new InvocationHandler() + var handler = new InvocationHandler { InnerHandler = capture, @@ -196,7 +163,7 @@ public async Task SendAsync_RewritesUri_AndAddsApiToken() var uri = "http://bank/accounts/17?"; var capture = new CaptureHandler(); - var handler = new InvocationHandler() + var handler = new InvocationHandler { InnerHandler = capture, diff --git a/test/Dapr.Client.Test/TestData/InvocationHandlerTestsData.cs b/test/Dapr.Client.Test/TestData/InvocationHandlerTestsData.cs new file mode 100644 index 000000000..65ab35932 --- /dev/null +++ b/test/Dapr.Client.Test/TestData/InvocationHandlerTestsData.cs @@ -0,0 +1,56 @@ +using Xunit; + +namespace Dapr.Client.Test.TestData; + +public class TryRewriteUriWithNoAppIdRewritesUriToDaprInvokeTestData : TheoryData +{ + public TryRewriteUriWithNoAppIdRewritesUriToDaprInvokeTestData() + { + Add(null, "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("bank", "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("Bank", "http://bank", "https://some.host:3499/v1.0/invoke/Bank/method/"); + Add("invalid", "http://bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("Bank", "http://Bank", "https://some.host:3499/v1.0/invoke/Bank/method/"); + Add("bank", "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("invalid", "http://Bank", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("bank", "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("invalid", "http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://Bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("Bank", "http://Bank:3939", "https://some.host:3499/v1.0/invoke/Bank/method/"); + Add("invalid", "http://Bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/"); + Add("app-id.with.dots", "http://app-id.with.dots", + "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/"); + Add("invalid", "http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/"); + Add(null, "http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/"); + Add("App-id.with.dots", "http://App-id.with.dots", + "https://some.host:3499/v1.0/invoke/App-id.with.dots/method/"); + Add("invalid", "http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/"); + Add(null, "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("bank", "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("invalid", "http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add("Bank", "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/Bank/method/"); + Add("invalid", "http://Bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/"); + Add(null, "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path"); + Add("bank", "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path"); + Add("invalid", "http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path"); + Add(null, "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path"); + Add("Bank", "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/Bank/method/some/path"); + Add("invalid", "http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path"); + Add(null, "http://bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment"); + Add("bank", "http://bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment"); + Add("invalid", "http://bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment"); + Add(null, "http://Bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment"); + Add("Bank", "http://Bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/Bank/method/some/path?q=test&p=another#fragment"); + Add("invalid", "http://Bank:3939/some/path?q=test&p=another#fragment", + "https://some.host:3499/v1.0/invoke/bank/method/some/path?q=test&p=another#fragment"); + } +}