Fix issue ThreeMammals#952 and ThreeMammals#1174

wast · Apr 6, 2020 · d47fb63 · d47fb63
1 parent 347ea72
commit d47fb63
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 5 deletions.
diff --git a/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs b/src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs
@@ -8,7 +8,10 @@
 
 namespace Ocelot.DownstreamUrlCreator.Middleware
 {
+    using System.Collections.Generic;
+    using System.Linq;
     using System.Text.RegularExpressions;
+    using System.Web;
 
     public class DownstreamUrlCreatorMiddleware : OcelotMiddleware
     {
@@ -55,20 +58,20 @@ public async Task Invoke(DownstreamContext context)
                 if (ContainsQueryString(dsPath))
                 {
                     context.DownstreamRequest.AbsolutePath = GetPath(dsPath);
-
                     if (string.IsNullOrEmpty(context.DownstreamRequest.Query))
                     {
                         context.DownstreamRequest.Query = GetQueryString(dsPath);
                     }
                     else
                     {
-                        context.DownstreamRequest.Query += GetQueryString(dsPath).Replace('?', '&');
+                        var newQueryString = GetQueryString(dsPath).Replace('?', '&');
+                        newQueryString = MergeQueryStringsWithoutDuplicateValues(context.DownstreamRequest.Query, newQueryString);
+                        context.DownstreamRequest.Query = "?" + newQueryString;
                     }
                 }
                 else
                 {
                     RemoveQueryStringParametersThatHaveBeenUsedInTemplate(context);
-
                     context.DownstreamRequest.AbsolutePath = dsPath.Value;
                 }
             }
@@ -78,6 +81,35 @@ public async Task Invoke(DownstreamContext context)
             await _next.Invoke(context);
         }
 
+
+        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 context.TemplatePlaceholderNameAndValues)

diff --git a/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs b/test/Ocelot.AcceptanceTests/RoutingWithQueryStringTests.cs
@@ -57,6 +57,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
@@ -345,8 +345,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();
         }