From d003688553794b9b7ea034e47ec78fa063d76142 Mon Sep 17 00:00:00 2001 From: TWEESTY Date: Fri, 9 Feb 2024 17:09:08 +0100 Subject: [PATCH 1/6] Handle the case where appid can contain some uppercases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Chaussé Signed-off-by: TWEESTY Signed-off-by: Nicolas Chaussé --- src/Dapr.Client/InvocationHandler.cs | 35 +++++++++++++++++-- .../InvocationHandlerTests.cs | 7 +++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index 1e9000c4d..94028c8b6 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -1,4 +1,4 @@ -// ------------------------------------------------------------------------ +// ------------------------------------------------------------------------ // Copyright 2021 The Dapr Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -13,7 +13,9 @@ using System; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Net.Http; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -128,17 +130,44 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) return false; } - var builder = new UriBuilder(uri) { Scheme = this.parsedEndpoint.Scheme, Host = this.parsedEndpoint.Host, Port = this.parsedEndpoint.Port, - Path = $"/v1.0/invoke/{uri.Host}/method" + uri.AbsolutePath, + Path = $"/v1.0/invoke/{GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, }; rewritten = builder.Uri; return true; } + + /// + /// Get the original host (case sensitive) from the URI (thanks to uri.OriginalString) + /// Mandatory to get the original host if the app id has at least one uppercase + /// + /// The uri + /// The original hostname from the uri + /// The original string from the uri is invalid + private string GetOriginalHostFromUri(Uri uri) + { + ArgumentNullException.ThrowIfNull(uri); + + // If there is no upper character inside the original string, we can directly return the uri host + if(!uri.OriginalString.Any(char.IsUpper)) + { + return uri.Host; + } + + Regex regex = new Regex("^.+?://(?[^:/]+)", RegexOptions.Singleline | RegexOptions.Compiled); + Match match = regex.Match(uri.OriginalString); + + if (!match.Success || !match.Groups.TryGetValue("host", out Group? host)) + { + throw new ArgumentException("The original string for the uri is invalid.", nameof(uri)); + } + + return host.Value; + } } } diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index c6adb93df..97acce38d 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -1,4 +1,4 @@ -// ------------------------------------------------------------------------ +// ------------------------------------------------------------------------ // Copyright 2021 The Dapr Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -80,11 +80,16 @@ public void TryRewriteUri_FailsForRelativeUris() [Theory] [InlineData("http://bank", "https://some.host:3499/v1.0/invoke/bank/method/")] + [InlineData("http://Bank", "https://some.host:3499/v1.0/invoke/Bank/method/")] [InlineData("http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] + [InlineData("http://Bank:3939", "https://some.host:3499/v1.0/invoke/Bank/method/")] [InlineData("http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] [InlineData("http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] + [InlineData("http://Bank:3939/", "https://some.host:3499/v1.0/invoke/Bank/method/")] [InlineData("http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] + [InlineData("http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/Bank/method/some/path")] [InlineData("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("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")] public void TryRewriteUri_RewritesUriToDaprInvoke(string uri, string expected) { var handler = new InvocationHandler() From b8d028f0e31aa5d5205a87a0ba3fa3b327c53c3a Mon Sep 17 00:00:00 2001 From: TWEESTY Date: Fri, 9 Feb 2024 17:24:22 +0100 Subject: [PATCH 2/6] Add one test sample MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Chaussé Signed-off-by: TWEESTY Signed-off-by: Nicolas Chaussé --- src/Dapr.Client/InvocationHandler.cs | 2 +- test/Dapr.Client.Test/InvocationHandlerTests.cs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index 94028c8b6..16a8d2bf3 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -135,7 +135,7 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) Scheme = this.parsedEndpoint.Scheme, Host = this.parsedEndpoint.Host, Port = this.parsedEndpoint.Port, - Path = $"/v1.0/invoke/{GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, + Path = $"/v1.0/invoke/{this.GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, }; rewritten = builder.Uri; diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index 97acce38d..9e62a2665 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -84,6 +84,7 @@ public void TryRewriteUri_FailsForRelativeUris() [InlineData("http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] [InlineData("http://Bank:3939", "https://some.host:3499/v1.0/invoke/Bank/method/")] [InlineData("http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] + [InlineData("http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/App-id.with.dots/method/")] [InlineData("http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] [InlineData("http://Bank:3939/", "https://some.host:3499/v1.0/invoke/Bank/method/")] [InlineData("http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] From 41a412fef3c916daa09ce2750ba3c4c097869259 Mon Sep 17 00:00:00 2001 From: TWEESTY Date: Fri, 9 Feb 2024 21:18:39 +0100 Subject: [PATCH 3/6] Optimization in order to not add some overhead time for the "normal" use case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: TWEESTY Signed-off-by: Nicolas Chaussé --- src/Dapr.Client/DaprClient.cs | 4 +- src/Dapr.Client/InvocationHandler.cs | 18 ++++- test/Dapr.Client.Test/DaprClientTest.cs | 2 +- .../InvocationHandlerTests.cs | 79 +++++++++++++++---- 4 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/Dapr.Client/DaprClient.cs b/src/Dapr.Client/DaprClient.cs index 361ac54bc..19deee2da 100644 --- a/src/Dapr.Client/DaprClient.cs +++ b/src/Dapr.Client/DaprClient.cs @@ -63,6 +63,7 @@ public abstract class DaprClient : IDisposable /// /// An optional app-id. If specified, the app-id will be configured as the value of /// so that relative URIs can be used. + /// If specified, the client could not call different services. /// /// The HTTP endpoint of the Dapr process to use for service invocation calls. /// The token to be added to all request headers to Dapr runtime. @@ -79,7 +80,8 @@ public static HttpClient CreateInvokeHttpClient(string appId = null, string dapr var handler = new InvocationHandler() { InnerHandler = new HttpClientHandler(), - DaprApiToken = daprApiToken + DaprApiToken = daprApiToken, + AppId = appId, }; if (daprEndpoint is string) diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index 16a8d2bf3..a9dedeef3 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -84,6 +84,12 @@ public string DaprEndpoint } } + /// + /// Gets or sets the AppId used for service invocation + /// + /// The AppId used for service invocation + public string? AppId { get; set; } + // Internal for testing internal string? DaprApiToken { @@ -130,12 +136,20 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) return false; } + // If the URI has a host which does not correspond to the AppId, it does not make sense. + // We do this check for optimization, to not add some overhead time for the "normal" use case + if (this.AppId is not null && !this.AppId.Equals(uri.Host, StringComparison.CurrentCultureIgnoreCase)) + { + rewritten = null; + return false; + } + var builder = new UriBuilder(uri) { Scheme = this.parsedEndpoint.Scheme, Host = this.parsedEndpoint.Host, Port = this.parsedEndpoint.Port, - Path = $"/v1.0/invoke/{this.GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, + Path = $"/v1.0/invoke/{this.AppId ?? this.GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, }; rewritten = builder.Uri; @@ -144,7 +158,7 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) /// /// Get the original host (case sensitive) from the URI (thanks to uri.OriginalString) - /// Mandatory to get the original host if the app id has at least one uppercase + /// Mandatory to get the original host if the app id has at least one uppercase and the app id has not been sent to the handler /// /// The uri /// The original hostname from the uri diff --git a/test/Dapr.Client.Test/DaprClientTest.cs b/test/Dapr.Client.Test/DaprClientTest.cs index a822bdf89..01d22edcf 100644 --- a/test/Dapr.Client.Test/DaprClientTest.cs +++ b/test/Dapr.Client.Test/DaprClientTest.cs @@ -1,4 +1,4 @@ -// ------------------------------------------------------------------------ +// ------------------------------------------------------------------------ // Copyright 2021 The Dapr Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index 9e62a2665..c3f031d70 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -79,23 +79,36 @@ public void TryRewriteUri_FailsForRelativeUris() } [Theory] - [InlineData("http://bank", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("http://Bank", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("http://bank:3939", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("http://Bank:3939", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("http://app-id.with.dots", "https://some.host:3499/v1.0/invoke/app-id.with.dots/method/")] - [InlineData("http://App-id.with.dots", "https://some.host:3499/v1.0/invoke/App-id.with.dots/method/")] - [InlineData("http://bank:3939/", "https://some.host:3499/v1.0/invoke/bank/method/")] - [InlineData("http://Bank:3939/", "https://some.host:3499/v1.0/invoke/Bank/method/")] - [InlineData("http://bank:3939/some/path", "https://some.host:3499/v1.0/invoke/bank/method/some/path")] - [InlineData("http://Bank:3939/some/path", "https://some.host:3499/v1.0/invoke/Bank/method/some/path")] - [InlineData("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("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")] - public void TryRewriteUri_RewritesUriToDaprInvoke(string uri, string expected) + [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(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(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(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(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(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(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(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(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(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(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(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")] + public void TryRewriteUri_WithNoAppId_RewritesUriToDaprInvoke(string? appId, string uri, string expected) { var handler = new InvocationHandler() { DaprEndpoint = "https://some.host:3499", + AppId = appId, }; Assert.True(handler.TryRewriteUri(new Uri(uri), out var rewritten)); @@ -103,7 +116,7 @@ public void TryRewriteUri_RewritesUriToDaprInvoke(string uri, string expected) } [Fact] - public async Task SendAsync_InvalidUri_ThrowsException() + public async Task SendAsync_InvalidNotSetUri_ThrowsException() { var handler = new InvocationHandler(); var ex = await Assert.ThrowsAsync(async () => @@ -114,6 +127,19 @@ public async Task SendAsync_InvalidUri_ThrowsException() Assert.Contains("The request URI '' is not a valid Dapr service invocation destination.", ex.Message); } + [Fact] + public async Task SendAsync_InvalidUriWithAppId_ThrowsException() + { + var handler = new InvocationHandler() { AppId = "bank" }; + string fakeUrl = "http://invalid/test"; + var ex = await Assert.ThrowsAsync(async () => + { + await CallSendAsync(handler, new HttpRequestMessage() { RequestUri = new Uri(fakeUrl) }); // No URI set + }); + + Assert.Contains($"The request URI '{fakeUrl}' is not a valid Dapr service invocation destination.", ex.Message); + } + [Fact] public async Task SendAsync_RewritesUri() { @@ -138,6 +164,31 @@ public async Task SendAsync_RewritesUri() Assert.False(request.Headers.TryGetValues("dapr-api-token", out _)); } + [Fact] + public async Task SendAsync_RewritesUri_AndAppId() + { + var uri = "http://bank/accounts/17?"; + + var capture = new CaptureHandler(); + var handler = new InvocationHandler() + { + InnerHandler = capture, + + DaprEndpoint = "https://localhost:5000", + DaprApiToken = null, + AppId = "bank" + }; + + var request = new HttpRequestMessage(HttpMethod.Post, uri); + var response = await CallSendAsync(handler, request); + + Assert.Equal("https://localhost:5000/v1.0/invoke/bank/method/accounts/17?", capture.RequestUri?.OriginalString); + Assert.Null(capture.DaprApiToken); + + Assert.Equal(uri, request.RequestUri?.OriginalString); + Assert.False(request.Headers.TryGetValues("dapr-api-token", out _)); + } + [Fact] public async Task SendAsync_RewritesUri_AndAddsApiToken() { From f2618009fe6b3daffb231b024c344b5d6aa15b27 Mon Sep 17 00:00:00 2001 From: TWEESTY Date: Fri, 9 Feb 2024 21:32:15 +0100 Subject: [PATCH 4/6] Change comment which was false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: TWEESTY Signed-off-by: Nicolas Chaussé --- test/Dapr.Client.Test/InvocationHandlerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index c3f031d70..cdb4e8ec9 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -134,7 +134,7 @@ public async Task SendAsync_InvalidUriWithAppId_ThrowsException() string fakeUrl = "http://invalid/test"; var ex = await Assert.ThrowsAsync(async () => { - await CallSendAsync(handler, new HttpRequestMessage() { RequestUri = new Uri(fakeUrl) }); // No URI set + await CallSendAsync(handler, new HttpRequestMessage() { RequestUri = new Uri(fakeUrl) }); // URI does not correspond to the appid }); Assert.Contains($"The request URI '{fakeUrl}' is not a valid Dapr service invocation destination.", ex.Message); From df02fc2691ff927635785cfaa1c16baa57fe4060 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Chauss=C3=A9?= Date: Sat, 10 Feb 2024 20:52:17 +0100 Subject: [PATCH 5/6] Remove the breaking change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: TWEESTY Signed-off-by: Nicolas Chaussé --- src/Dapr.Client/DaprClient.cs | 5 +-- src/Dapr.Client/InvocationHandler.cs | 24 ++++++----- .../InvocationHandlerTests.cs | 41 ++++++++++--------- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/Dapr.Client/DaprClient.cs b/src/Dapr.Client/DaprClient.cs index 19deee2da..7c2f3f46f 100644 --- a/src/Dapr.Client/DaprClient.cs +++ b/src/Dapr.Client/DaprClient.cs @@ -63,7 +63,6 @@ public abstract class DaprClient : IDisposable /// /// An optional app-id. If specified, the app-id will be configured as the value of /// so that relative URIs can be used. - /// If specified, the client could not call different services. /// /// The HTTP endpoint of the Dapr process to use for service invocation calls. /// The token to be added to all request headers to Dapr runtime. @@ -81,7 +80,7 @@ public static HttpClient CreateInvokeHttpClient(string appId = null, string dapr { InnerHandler = new HttpClientHandler(), DaprApiToken = daprApiToken, - AppId = appId, + DefaultAppId = appId, }; if (daprEndpoint is string) @@ -211,7 +210,7 @@ public abstract Task PublishEventAsync( string topicName, Dictionary metadata, CancellationToken cancellationToken = default); - + /// /// // Bulk Publishes multiple events to the specified topic. /// diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index a9dedeef3..5c7eab325 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -85,10 +85,10 @@ public string DaprEndpoint } /// - /// Gets or sets the AppId used for service invocation + /// Gets or sets the default AppId used for service invocation /// /// The AppId used for service invocation - public string? AppId { get; set; } + public string? DefaultAppId { get; set; } // Internal for testing internal string? DaprApiToken @@ -136,12 +136,16 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) return false; } - // If the URI has a host which does not correspond to the AppId, it does not make sense. - // We do this check for optimization, to not add some overhead time for the "normal" use case - if (this.AppId is not null && !this.AppId.Equals(uri.Host, StringComparison.CurrentCultureIgnoreCase)) + string host; + + // It is just for optimization, to not add some overhead time + if (this.DefaultAppId is not null && uri.Host.Equals(this.DefaultAppId, StringComparison.InvariantCultureIgnoreCase)) { - rewritten = null; - return false; + host = this.DefaultAppId; + } + else + { + host = this.GetOriginalHostFromUri(uri); } var builder = new UriBuilder(uri) @@ -149,13 +153,13 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) Scheme = this.parsedEndpoint.Scheme, Host = this.parsedEndpoint.Host, Port = this.parsedEndpoint.Port, - Path = $"/v1.0/invoke/{this.AppId ?? this.GetOriginalHostFromUri(uri)}/method" + uri.AbsolutePath, + Path = $"/v1.0/invoke/{host}/method" + uri.AbsolutePath, }; rewritten = builder.Uri; return true; } - + /// /// Get the original host (case sensitive) from the URI (thanks to uri.OriginalString) /// Mandatory to get the original host if the app id has at least one uppercase and the app id has not been sent to the handler @@ -168,7 +172,7 @@ private string GetOriginalHostFromUri(Uri uri) ArgumentNullException.ThrowIfNull(uri); // If there is no upper character inside the original string, we can directly return the uri host - if(!uri.OriginalString.Any(char.IsUpper)) + if (!uri.OriginalString.Any(char.IsUpper)) { return uri.Host; } diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index cdb4e8ec9..476dd4b47 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -30,8 +30,8 @@ public class InvocationHandlerTests public void DaprEndpoint_InvalidScheme() { var handler = new InvocationHandler(); - var ex = Assert.Throws(() => - { + var ex = Assert.Throws(() => + { handler.DaprEndpoint = "ftp://localhost:3500"; }); @@ -43,7 +43,7 @@ public void DaprEndpoint_InvalidUri() { var handler = new InvocationHandler(); Assert.Throws(() => - { + { handler.DaprEndpoint = ""; }); @@ -81,34 +81,48 @@ 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")] public void TryRewriteUri_WithNoAppId_RewritesUriToDaprInvoke(string? appId, string uri, string expected) { var handler = new InvocationHandler() { DaprEndpoint = "https://some.host:3499", - AppId = appId, + DefaultAppId = appId, }; Assert.True(handler.TryRewriteUri(new Uri(uri), out var rewritten)); @@ -121,25 +135,12 @@ public async Task SendAsync_InvalidNotSetUri_ThrowsException() var handler = new InvocationHandler(); var ex = await Assert.ThrowsAsync(async () => { - await CallSendAsync(handler, new HttpRequestMessage(){ }); // No URI set + await CallSendAsync(handler, new HttpRequestMessage() { }); // No URI set }); Assert.Contains("The request URI '' is not a valid Dapr service invocation destination.", ex.Message); } - [Fact] - public async Task SendAsync_InvalidUriWithAppId_ThrowsException() - { - var handler = new InvocationHandler() { AppId = "bank" }; - string fakeUrl = "http://invalid/test"; - var ex = await Assert.ThrowsAsync(async () => - { - await CallSendAsync(handler, new HttpRequestMessage() { RequestUri = new Uri(fakeUrl) }); // URI does not correspond to the appid - }); - - Assert.Contains($"The request URI '{fakeUrl}' is not a valid Dapr service invocation destination.", ex.Message); - } - [Fact] public async Task SendAsync_RewritesUri() { @@ -176,7 +177,7 @@ public async Task SendAsync_RewritesUri_AndAppId() DaprEndpoint = "https://localhost:5000", DaprApiToken = null, - AppId = "bank" + DefaultAppId = "bank" }; var request = new HttpRequestMessage(HttpMethod.Post, uri); @@ -221,7 +222,7 @@ private async Task CallSendAsync(InvocationHandler handler, try { - return await (Task)method!.Invoke(handler, new object[]{ message, cancellationToken, })!; + return await (Task)method!.Invoke(handler, new object[] { message, cancellationToken, })!; } catch (TargetInvocationException tie) // reflection always adds an extra layer of exceptions. { From dcd9deefd7ea6b8ee5d95e47f3a64fec4bdf6e6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Chauss=C3=A9?= Date: Fri, 16 Feb 2024 15:44:52 +0100 Subject: [PATCH 6/6] Simplify according to the review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Chaussé --- src/Dapr.Client/DaprClient.cs | 3 +- src/Dapr.Client/InvocationHandler.cs | 33 +------------------ .../InvocationHandlerTests.cs | 28 ++++++++-------- 3 files changed, 17 insertions(+), 47 deletions(-) diff --git a/src/Dapr.Client/DaprClient.cs b/src/Dapr.Client/DaprClient.cs index 7c2f3f46f..3e70962fc 100644 --- a/src/Dapr.Client/DaprClient.cs +++ b/src/Dapr.Client/DaprClient.cs @@ -62,7 +62,8 @@ public abstract class DaprClient : IDisposable /// /// /// An optional app-id. If specified, the app-id will be configured as the value of - /// so that relative URIs can be used. + /// so that relative URIs can be used. It is mandatory to set this parameter if your app-id contains at least one upper letter. + /// If some requests use absolute URL with an app-id which contains at least one upper letter, it will not work, the workaround is to create one HttpClient for each app-id with the app-ip parameter set. /// /// The HTTP endpoint of the Dapr process to use for service invocation calls. /// The token to be added to all request headers to Dapr runtime. diff --git a/src/Dapr.Client/InvocationHandler.cs b/src/Dapr.Client/InvocationHandler.cs index 5c7eab325..1b55436aa 100644 --- a/src/Dapr.Client/InvocationHandler.cs +++ b/src/Dapr.Client/InvocationHandler.cs @@ -13,9 +13,7 @@ using System; using System.Diagnostics.CodeAnalysis; -using System.Linq; using System.Net.Http; -using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; @@ -138,14 +136,13 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) string host; - // It is just for optimization, to not add some overhead time if (this.DefaultAppId is not null && uri.Host.Equals(this.DefaultAppId, StringComparison.InvariantCultureIgnoreCase)) { host = this.DefaultAppId; } else { - host = this.GetOriginalHostFromUri(uri); + host = uri.Host; } var builder = new UriBuilder(uri) @@ -159,33 +156,5 @@ internal bool TryRewriteUri(Uri? uri, [NotNullWhen(true)] out Uri? rewritten) rewritten = builder.Uri; return true; } - - /// - /// Get the original host (case sensitive) from the URI (thanks to uri.OriginalString) - /// Mandatory to get the original host if the app id has at least one uppercase and the app id has not been sent to the handler - /// - /// The uri - /// The original hostname from the uri - /// The original string from the uri is invalid - private string GetOriginalHostFromUri(Uri uri) - { - ArgumentNullException.ThrowIfNull(uri); - - // If there is no upper character inside the original string, we can directly return the uri host - if (!uri.OriginalString.Any(char.IsUpper)) - { - return uri.Host; - } - - Regex regex = new Regex("^.+?://(?[^:/]+)", RegexOptions.Singleline | RegexOptions.Compiled); - Match match = regex.Match(uri.OriginalString); - - if (!match.Success || !match.Groups.TryGetValue("host", out Group? host)) - { - throw new ArgumentException("The original string for the uri is invalid.", nameof(uri)); - } - - return host.Value; - } } } diff --git a/test/Dapr.Client.Test/InvocationHandlerTests.cs b/test/Dapr.Client.Test/InvocationHandlerTests.cs index 476dd4b47..3dac84113 100644 --- a/test/Dapr.Client.Test/InvocationHandlerTests.cs +++ b/test/Dapr.Client.Test/InvocationHandlerTests.cs @@ -83,40 +83,40 @@ public void TryRewriteUri_FailsForRelativeUris() [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(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("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(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("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(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("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(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("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(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("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(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("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")] public void TryRewriteUri_WithNoAppId_RewritesUriToDaprInvoke(string? appId, string uri, string expected) { var handler = new InvocationHandler() @@ -177,13 +177,13 @@ public async Task SendAsync_RewritesUri_AndAppId() DaprEndpoint = "https://localhost:5000", DaprApiToken = null, - DefaultAppId = "bank" + DefaultAppId = "Bank" }; var request = new HttpRequestMessage(HttpMethod.Post, uri); var response = await CallSendAsync(handler, request); - Assert.Equal("https://localhost:5000/v1.0/invoke/bank/method/accounts/17?", capture.RequestUri?.OriginalString); + Assert.Equal("https://localhost:5000/v1.0/invoke/Bank/method/accounts/17?", capture.RequestUri?.OriginalString); Assert.Null(capture.DaprApiToken); Assert.Equal(uri, request.RequestUri?.OriginalString);