Fix issue ThreeMammals#952 and ThreeMammals#1174

wast · Jul 26, 2023 · 8787956 · 8787956
1 parent 433dfca
commit 8787956
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 7 deletions.
diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs
@@ -7,14 +7,14 @@
 
 using Ocelot.DownstreamRouteFinder.UrlMatcher;
 
-using Ocelot.Logging;
+using Ocelot.Logging;
 
 using Microsoft.AspNetCore.Http;
 
-using Ocelot.Middleware;
+using Ocelot.Middleware;
 using Ocelot.Request.Middleware;
 
-using Ocelot.Responses;
+using Ocelot.Responses;
 
 using Ocelot.DownstreamUrlCreator.UrlTemplateReplacer;
 
@@ -86,7 +86,9 @@ public async Task Invoke(HttpContext httpContext)
                     }
                     else
                     {
-                        downstreamRequest.Query += GetQueryString(dsPath).Replace('?', '&');
+                        var newQueryString = GetQueryString(dsPath).Replace('?', '&');
+                        newQueryString = MergeQueryStringsWithoutDuplicateValues(context.DownstreamRequest.Query, newQueryString);
+                        context.DownstreamRequest.Query = "?" + newQueryString;
                     }
                 }
                 else
@@ -102,7 +104,36 @@ public async Task Invoke(HttpContext httpContext)
             await _next.Invoke(httpContext);
         }
 
-        private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamRequest downstreamRequest, List<PlaceholderNameAndValue> templatePlaceholderNameAndValues)
+
+        private static string MergeQueryStringsWithoutDuplicateValues(string queryString, string newQueryString)
+        {
+            var queries = HttpUtility.ParseQueryString(queryString);
+            var newQueries = HttpUtility.ParseQueryString(newQueryString);
+
+            var dict = new Dictionary<string, string>();
+            foreach (var key in newQueries.AllKeys)
+            {
+                if (!string.IsNullOrEmpty(key))
+                {
+                    dict.Add(key, newQueries[key]);
+
+                }
+            }
+
+            foreach (var key in queries.AllKeys)
+            {
+                if (string.IsNullOrEmpty(key) || dict.ContainsValue(queries[key]))
+                {
+                    continue;
+                }
+
+                dict.Add(key, queries[key]);
+            }
+
+            return string.Join("&", dict.Select(kvp => string.Format("{0}={1}", kvp.Key, kvp.Value)));
+        }
+
+        private static void RemoveQueryStringParametersThatHaveBeenUsedInTemplate(DownstreamContext context)
         {
             foreach (var nAndV in templatePlaceholderNameAndValues)
             {

diff --git a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs
@@ -61,6 +61,84 @@ public void should_return_response_200_with_query_string_template()
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_return_response_200_with_query_string_template_different_keys()
+        {
+            var subscriptionId = Guid.NewGuid().ToString();
+            var unitId = Guid.NewGuid().ToString();
+            var port = RandomPortFinder.GetRandomPort();
+
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/api/subscriptions/{subscriptionId}/updates?unitId={unitId}",
+                            DownstreamScheme = "http",
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new FileHostAndPort
+                                {
+                                    Host = "localhost",
+                                    Port = port,
+                                }
+                            },
+                            UpstreamPathTemplate = "/api/units/{subscriptionId}/updates?unit={unitId}",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                        }
+                    }
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}", $"/api/subscriptions/{subscriptionId}/updates", $"?unitId={unitId}", 200, "Hello from Laura"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway($"/api/units/{subscriptionId}/updates?unit={unitId}"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
+                .BDDfy();
+        }
+
+
+        [Fact]
+        public void should_return_response_200_with_query_string_template_additional_key() //issue #327
+        {
+            var subscriptionId = Guid.NewGuid().ToString();
+            var unitId = Guid.NewGuid().ToString();
+            var port = RandomPortFinder.GetRandomPort();
+
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/api/subscriptions/{subscriptionId}/updates?unitId={unitId}",
+                            DownstreamScheme = "http",
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new FileHostAndPort
+                                {
+                                    Host = "localhost",
+                                    Port = port,
+                                }
+                            },
+                            UpstreamPathTemplate = "/api/units/{subscriptionId}/updates?unit={unitId}",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                        }
+                    }
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}", $"/api/subscriptions/{subscriptionId}/updates", $"?unitId={unitId}&x=y", 200, "Hello from Laura"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway($"/api/units/{subscriptionId}/updates?unit={unitId}&x=y"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
+                .BDDfy();
+        }
+
+
         [Fact]
         public void should_return_response_200_with_odata_query_string()
         {

diff --git a/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs b/test/Ocelot.UnitTests/DownstreamUrlCreator/DownstreamUrlCreatorMiddlewareTests.cs
@@ -355,8 +355,8 @@ public void issue_473_should_not_remove_additional_query_string()
                 .And(x => GivenTheServiceProviderConfigIs(config))
                 .And(x => x.GivenTheUrlReplacerWillReturn("/Authorized/1?server=2"))
                 .When(x => x.WhenICallTheMiddleware())
-                .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:5000/Authorized/1?refreshToken=2288356cfb1338fdc5ff4ca558ec785118dfe1ff2864340937da8226863ff66d&server=2"))
-                .And(x => ThenTheQueryStringIs("?refreshToken=2288356cfb1338fdc5ff4ca558ec785118dfe1ff2864340937da8226863ff66d&server=2"))
+                .Then(x => x.ThenTheDownstreamRequestUriIs("http://localhost:5000/Authorized/1?server=2&refreshToken=2288356cfb1338fdc5ff4ca558ec785118dfe1ff2864340937da8226863ff66d"))
+                .And(x => ThenTheQueryStringIs("?server=2&refreshToken=2288356cfb1338fdc5ff4ca558ec785118dfe1ff2864340937da8226863ff66d"))
                 .BDDfy();
         }