From c56d07914587bc23cbaed95cf04f5c7572283629 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Fri, 4 Sep 2020 12:33:55 +0200 Subject: [PATCH 1/7] fix char missing when query string contains the placeholder name and value --- .../DownstreamUrlCreatorMiddleware.cs | 7 ++- .../RoutingWithQueryStringTests.cs | 43 ++++++++++++++++++- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 4db8f5d96..952762f55 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -108,13 +108,12 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst { var name = nAndV.Name.Replace("{", string.Empty).Replace("}", string.Empty); - if (downstreamRequest.Query.Contains(name) && - downstreamRequest.Query.Contains(nAndV.Value)) + var rgx = new Regex($@"\b{name}={nAndV.Value}\b"); + + if (rgx.IsMatch(downstreamRequest.Query)) { var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); - - var rgx = new Regex($@"\b{name}={nAndV.Value}\b"); downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); if (!string.IsNullOrEmpty(downstreamRequest.Query)) diff --git a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs index 5aa72fc72..3b96f68fb 100644 --- a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs +++ b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs @@ -247,8 +247,49 @@ public void should_return_response_200_with_query_string_upstream_template_multi .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) .BDDfy(); - } + } + + // to reproduce 1288: query string should contain the placeholder name and value + [Fact] + public void should_copy_query_string_to_downstream_path_issue_1288() + { + var idName = "id"; + var idValue = "3"; + var queryName = idName + "1"; + var queryValue = "2" + idValue + "12"; + var port = RandomPortFinder.GetRandomPort(); + + var configuration = new FileConfiguration + { + Routes = new List + { + new FileRoute + { + DownstreamPathTemplate = $"/cpx/t1/{{{idName}}}", + DownstreamScheme = "http", + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "localhost", + Port = port, + }, + }, + UpstreamPathTemplate = $"/safe/{{{idName}}}", + UpstreamHttpMethod = new List { "Get" }, + }, + }, + }; + this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}", $"/cpx/t1/{idValue}", $"?{queryName}={queryValue}", 200, "Hello from Laura")) + .And(x => _steps.GivenThereIsAConfiguration(configuration)) + .And(x => _steps.GivenOcelotIsRunning()) + .When(x => _steps.WhenIGetUrlOnTheApiGateway($"/safe/{idValue}?{queryName}={queryValue}")) + .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK)) + .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura")) + .BDDfy(); + } + private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, string queryString, int statusCode, string responseBody) { _serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, basePath, async context => From 5cb55ec728f79be10d4d131ddacfdac9f3b12392 Mon Sep 17 00:00:00 2001 From: jlukawska Date: Fri, 12 Mar 2021 11:24:30 +0100 Subject: [PATCH 2/7] fix the regex problem with removing a query param when it is not the first --- .../DownstreamUrlCreatorMiddleware.cs | 6 ++-- .../DownstreamUrlCreatorMiddlewareTests.cs | 34 ++++++++++++++++++- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 952762f55..9078d0943 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -107,14 +107,14 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst foreach (var nAndV in templatePlaceholderNameAndValues) { var name = nAndV.Name.Replace("{", string.Empty).Replace("}", string.Empty); - + var rgx = new Regex($@"\b{name}={nAndV.Value}\b"); if (rgx.IsMatch(downstreamRequest.Query)) { - var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); + var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); + downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); - downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); if (!string.IsNullOrEmpty(downstreamRequest.Query)) { diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs index cc027ad41..b17b1b3b9 100644 --- a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs @@ -137,7 +137,39 @@ public void should_replace_query_string_but_leave_non_placeholder_queries() .WithDownstreamRoute(downstreamRoute) .WithUpstreamHttpMethod(new List { "Get" }) .Build()))) - .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:5000/api/subscriptions/1/updates?unitId=2&productId=2")) + .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:5000/api/subscriptions/1/updates?unitId=2&productId=2")) // unitId is the first + .And(x => GivenTheServiceProviderConfigIs(config)) + .And(x => x.GivenTheUrlReplacerWillReturn("api/units/1/2/updates")) + .When(x => x.WhenICallTheMiddleware()) + .Then(x => x.ThenTheDownstreamRequestUriIs("https://localhost:5000/api/units/1/2/updates?productId=2")) + .And(x => ThenTheQueryStringIs("?productId=2")) + .BDDfy(); + } + + [Fact] + public void should_replace_query_string_but_leave_non_placeholder_queries_2() + { + var downstreamRoute = new DownstreamRouteBuilder() + .WithDownstreamPathTemplate("/api/units/{subscriptionId}/{unitId}/updates") + .WithUpstreamHttpMethod(new List { "Get" }) + .WithDownstreamScheme("https") + .Build(); + + var config = new ServiceProviderConfigurationBuilder() + .Build(); + + this.Given(x => x.GivenTheDownStreamRouteIs( + new DownstreamRouteHolder( + new List + { + new PlaceholderNameAndValue("{subscriptionId}", "1"), + new PlaceholderNameAndValue("{unitId}", "2") + }, + new RouteBuilder() + .WithDownstreamRoute(downstreamRoute) + .WithUpstreamHttpMethod(new List { "Get" }) + .Build()))) + .And(x => x.GivenTheDownstreamRequestUriIs("http://localhost:5000/api/subscriptions/1/updates?productId=2&unitId=2")) // unitId is the second .And(x => GivenTheServiceProviderConfigIs(config)) .And(x => x.GivenTheUrlReplacerWillReturn("api/units/1/2/updates")) .When(x => x.WhenICallTheMiddleware()) From 1b43f1949210d55964f87f1fa68dada597b11f7d Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 17 Jul 2023 17:32:42 +0300 Subject: [PATCH 3/7] Remove and Sort Usings --- .../DownstreamUrlCreatorMiddleware.cs | 38 ++++++++----------- .../RoutingWithQueryStringTests.cs | 8 +--- .../DownstreamUrlCreatorMiddlewareTests.cs | 22 ++++------- 3 files changed, 24 insertions(+), 44 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 9078d0943..8436f2c1c 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -1,24 +1,16 @@ -using System; -using System.Collections.Generic; -using System.Text.RegularExpressions; -using System.Threading.Tasks; - -using Ocelot.Configuration; - -using Ocelot.DownstreamRouteFinder.UrlMatcher; - -using Ocelot.Logging; - -using Microsoft.AspNetCore.Http; - -using Ocelot.Middleware; -using Ocelot.Request.Middleware; - -using Ocelot.Responses; - -using Ocelot.DownstreamUrlCreator.UrlTemplateReplacer; - +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration; +using Ocelot.DownstreamRouteFinder.UrlMatcher; +using Ocelot.DownstreamUrlCreator.UrlTemplateReplacer; +using Ocelot.Logging; +using Ocelot.Middleware; +using Ocelot.Request.Middleware; +using Ocelot.Responses; using Ocelot.Values; +using System; +using System.Collections.Generic; +using System.Text.RegularExpressions; +using System.Threading.Tasks; namespace Ocelot.DownstreamUrlCreator.Middleware { @@ -107,13 +99,13 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst foreach (var nAndV in templatePlaceholderNameAndValues) { var name = nAndV.Name.Replace("{", string.Empty).Replace("}", string.Empty); - - var rgx = new Regex($@"\b{name}={nAndV.Value}\b"); + + var rgx = new Regex($@"\b{name}={nAndV.Value}\b"); if (rgx.IsMatch(downstreamRequest.Query)) { var questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); - downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); + downstreamRequest.Query = rgx.Replace(downstreamRequest.Query, string.Empty); downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); if (!string.IsNullOrEmpty(downstreamRequest.Query)) diff --git a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs index 3b96f68fb..d8a90b99e 100644 --- a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs +++ b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs @@ -1,13 +1,9 @@ +using Microsoft.AspNetCore.Http; +using Ocelot.Configuration.File; using System; using System.Collections.Generic; using System.Net; - -using Ocelot.Configuration.File; - -using Microsoft.AspNetCore.Http; - using TestStack.BDDfy; - using Xunit; namespace Ocelot.AcceptanceTests diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs index b17b1b3b9..c8b008b71 100644 --- a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs +++ b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs @@ -1,12 +1,5 @@ -using System; -using System.Collections.Generic; -using System.Net.Http; -using System.Threading.Tasks; - -using Microsoft.AspNetCore.Http; - +using Microsoft.AspNetCore.Http; using Moq; - using Ocelot.Configuration; using Ocelot.Configuration.Builder; using Ocelot.DownstreamRouteFinder; @@ -17,15 +10,14 @@ using Ocelot.Logging; using Ocelot.Middleware; using Ocelot.Request.Middleware; - using Ocelot.Responses; - +using Ocelot.Values; using Shouldly; - +using System; +using System.Collections.Generic; +using System.Net.Http; +using System.Threading.Tasks; using TestStack.BDDfy; - -using Ocelot.Values; - using Xunit; namespace Ocelot.UnitTests.DownstreamUrlCreator @@ -163,7 +155,7 @@ public void should_replace_query_string_but_leave_non_placeholder_queries_2() new List { new PlaceholderNameAndValue("{subscriptionId}", "1"), - new PlaceholderNameAndValue("{unitId}", "2") + new PlaceholderNameAndValue("{unitId}", "2"), }, new RouteBuilder() .WithDownstreamRoute(downstreamRoute) From b8d748e7c4f96293810d01d5de486584a8f9afa3 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 17 Jul 2023 18:22:38 +0300 Subject: [PATCH 4/7] CA1845 Use span-based 'string.Concat' and 'AsSpan' instead of 'Substring' --- .../Middleware/DownstreamUrlCreatorMiddleware.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 8436f2c1c..d4b5a2d4d 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -110,7 +110,7 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst if (!string.IsNullOrEmpty(downstreamRequest.Query)) { - downstreamRequest.Query = '?' + downstreamRequest.Query.Substring(1); + downstreamRequest.Query = string.Concat("?", downstreamRequest.Query.AsSpan(1)); } } } From b147b43e21fa1abb236cf03e958657b794f6b6cb Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 17 Jul 2023 18:38:48 +0300 Subject: [PATCH 5/7] IDE0057 Substring can be simplified. Use range operator --- .../Middleware/DownstreamUrlCreatorMiddleware.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index d4b5a2d4d..58193fee2 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -117,8 +117,9 @@ private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(Downst } private static string GetPath(DownstreamPath dsPath) - { - return dsPath.Value.Substring(0, dsPath.Value.IndexOf('?', StringComparison.Ordinal)); + { + int length = dsPath.Value.IndexOf('?', StringComparison.Ordinal); + return dsPath.Value[..length]; } private static string GetQueryString(DownstreamPath dsPath) From d531718039fa4409418ac88b8ac45979f5412594 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 17 Jul 2023 18:42:58 +0300 Subject: [PATCH 6/7] IDE0057 Substring can be simplified. Use range operator --- .../Middleware/DownstreamUrlCreatorMiddleware.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 58193fee2..1639c3aa6 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -123,8 +123,9 @@ private static string GetPath(DownstreamPath dsPath) } private static string GetQueryString(DownstreamPath dsPath) - { - return dsPath.Value.Substring(dsPath.Value.IndexOf('?', StringComparison.Ordinal)); + { + int startIndex = dsPath.Value.IndexOf('?', StringComparison.Ordinal); + return dsPath.Value[startIndex..]; } private static bool ContainsQueryString(DownstreamPath dsPath) From 69c950a1d94c3454f4d7a4ab8b1fd47123ed4a24 Mon Sep 17 00:00:00 2001 From: raman-m Date: Mon, 17 Jul 2023 18:47:33 +0300 Subject: [PATCH 7/7] IDE0042 Variable declaration can be deconstructed. Deconstruct variable declaration --- .../Middleware/DownstreamUrlCreatorMiddleware.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 1639c3aa6..ec2bc13ff 100644 --- a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs +++ b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs @@ -58,11 +58,11 @@ public async Task Invoke(HttpContext httpContext) if (ServiceFabricRequest(internalConfiguration, downstreamRoute)) { - var pathAndQuery = CreateServiceFabricUri(downstreamRequest, downstreamRoute, templatePlaceholderNameAndValues, response); + var (path, query) = CreateServiceFabricUri(downstreamRequest, downstreamRoute, templatePlaceholderNameAndValues, response); //todo check this works again hope there is a test.. - downstreamRequest.AbsolutePath = pathAndQuery.Path; - downstreamRequest.Query = pathAndQuery.Query; + downstreamRequest.AbsolutePath = path; + downstreamRequest.Query = query; } else {