diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs index 4db8f5d96..ec2bc13ff 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 { @@ -66,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 { @@ -108,31 +100,32 @@ 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 questionMarkOrAmpersand = downstreamRequest.Query.IndexOf(name, StringComparison.Ordinal); - downstreamRequest.Query = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); + 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 = downstreamRequest.Query.Remove(questionMarkOrAmpersand - 1, 1); if (!string.IsNullOrEmpty(downstreamRequest.Query)) { - downstreamRequest.Query = '?' + downstreamRequest.Query.Substring(1); + downstreamRequest.Query = string.Concat("?", downstreamRequest.Query.AsSpan(1)); } } } } 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) - { - 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) diff --git a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs index 5aa72fc72..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 @@ -247,8 +243,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 => diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs index cc027ad41..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 @@ -137,7 +129,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())