ThreeMammals#534 fixed failing tests for this issue (ThreeMammals#575)

js8080 · Aug 25, 2018 · b0bdeb9 · b0bdeb9
1 parent 00a6000
commit b0bdeb9
Show file tree

Hide file tree

Showing 4 changed files with 676 additions and 596 deletions.
diff --git a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs
@@ -1,84 +1,93 @@
-using System.Collections.Generic;
-using Ocelot.Configuration.File;
-using Ocelot.Values;
-
-namespace Ocelot.Configuration.Creator
-{
-    public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator
-    {
-        private const string RegExMatchOneOrMoreOfEverything = ".+";
-        private const string RegExMatchEndString = "$";
-        private const string RegExIgnoreCase = "(?i)";
-        private const string RegExForwardSlashOnly = "^/$";
-        private const string RegExForwardSlashAndOnePlaceHolder = "^/.*";
-
-        public UpstreamPathTemplate Create(IReRoute reRoute)
-        {
+using System.Collections.Generic;
+using Ocelot.Configuration.File;
+using Ocelot.Values;
+
+namespace Ocelot.Configuration.Creator
+{
+    public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator
+    {
+        private const string RegExMatchOneOrMoreOfEverything = ".+";
+        private const string RegExMatchOneOrMoreOfEverythingUntilNextForwardSlash = "[^/]+";
+        private const string RegExMatchEndString = "$";
+        private const string RegExIgnoreCase = "(?i)";
+        private const string RegExForwardSlashOnly = "^/$";
+        private const string RegExForwardSlashAndOnePlaceHolder = "^/.*";
+
+        public UpstreamPathTemplate Create(IReRoute reRoute)
+        {
             var upstreamTemplate = reRoute.UpstreamPathTemplate;
-
-
-            var placeholders = new List<string>();
-
-            for (var i = 0; i < upstreamTemplate.Length; i++)
-            {
-                if (IsPlaceHolder(upstreamTemplate, i))
-                {
-                    var postitionOfPlaceHolderClosingBracket = upstreamTemplate.IndexOf('}', i);
-                    var difference = postitionOfPlaceHolderClosingBracket - i + 1;
-                    var placeHolderName = upstreamTemplate.Substring(i, difference);
-                    placeholders.Add(placeHolderName);
-
-                    //hack to handle /{url} case
-                    if(ForwardSlashAndOnePlaceHolder(upstreamTemplate, placeholders, postitionOfPlaceHolderClosingBracket))
-                    {
-                        return new UpstreamPathTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false);
-                    }
-                }
+
+            var placeholders = new List<string>();
+
+            for (var i = 0; i < upstreamTemplate.Length; i++)
+            {
+                if (IsPlaceHolder(upstreamTemplate, i))
+                {
+                    var postitionOfPlaceHolderClosingBracket = upstreamTemplate.IndexOf('}', i);
+                    var difference = postitionOfPlaceHolderClosingBracket - i + 1;
+                    var placeHolderName = upstreamTemplate.Substring(i, difference);
+                    placeholders.Add(placeHolderName);
+
+                    //hack to handle /{url} case
+                    if(ForwardSlashAndOnePlaceHolder(upstreamTemplate, placeholders, postitionOfPlaceHolderClosingBracket))
+                    {
+                        return new UpstreamPathTemplate(RegExForwardSlashAndOnePlaceHolder, 0, false);
+                    }
+                }
             }
-
-            var containsQueryString = false;
-
-            if (upstreamTemplate.Contains("?"))
-            {
-                containsQueryString = true;
-                upstreamTemplate = upstreamTemplate.Replace("?", "\\?");
-            }
-
-            foreach (var placeholder in placeholders)
-            {
-                upstreamTemplate = upstreamTemplate.Replace(placeholder, RegExMatchOneOrMoreOfEverything);
-            }
-
-            if (upstreamTemplate == "/")
-            {
-                return new UpstreamPathTemplate(RegExForwardSlashOnly, reRoute.Priority, containsQueryString);
-            }
-
-            if(upstreamTemplate.EndsWith("/"))
-            {
-                upstreamTemplate = upstreamTemplate.Remove(upstreamTemplate.Length -1, 1) + "(/|)";
-            }
-
-            var route = reRoute.ReRouteIsCaseSensitive 
-                ? $"^{upstreamTemplate}{RegExMatchEndString}" 
-                : $"^{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}";
-
-            return new UpstreamPathTemplate(route, reRoute.Priority, containsQueryString);
-        }
-
-        private bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List<string> placeholders, int postitionOfPlaceHolderClosingBracket)
-        {
-            if(upstreamTemplate.Substring(0, 2) == "/{" && placeholders.Count == 1 && upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1)
-            {   
-                return true;
-            }
-
-            return false;
-        }
-
-        private bool IsPlaceHolder(string upstreamTemplate, int i)
-        {
-            return upstreamTemplate[i] == '{';
-        }
-    }
-}
+
+            var containsQueryString = false;
+
+            if (upstreamTemplate.Contains("?"))
+            {
+                containsQueryString = true;
+                upstreamTemplate = upstreamTemplate.Replace("?", "\\?");
+            }
+
+            for (int i = 0; i < placeholders.Count; i++)
+            {
+                var indexOfPlaceholder = upstreamTemplate.IndexOf(placeholders[i]);
+                var indexOfNextForwardSlash = upstreamTemplate.IndexOf("/", indexOfPlaceholder);
+                if(indexOfNextForwardSlash < indexOfPlaceholder || (containsQueryString && upstreamTemplate.IndexOf("?") < upstreamTemplate.IndexOf(placeholders[i])))
+                {
+                    upstreamTemplate = upstreamTemplate.Replace(placeholders[i], RegExMatchOneOrMoreOfEverything);
+                } 
+                else 
+                {
+                    upstreamTemplate = upstreamTemplate.Replace(placeholders[i], RegExMatchOneOrMoreOfEverythingUntilNextForwardSlash);
+                }
+            }
+
+            if (upstreamTemplate == "/")
+            {
+                return new UpstreamPathTemplate(RegExForwardSlashOnly, reRoute.Priority, containsQueryString);
+            }
+
+            if(upstreamTemplate.EndsWith("/"))
+            {
+                upstreamTemplate = upstreamTemplate.Remove(upstreamTemplate.Length -1, 1) + "(/|)";
+            }
+
+            var route = reRoute.ReRouteIsCaseSensitive 
+                ? $"^{upstreamTemplate}{RegExMatchEndString}" 
+                : $"^{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}";
+
+            return new UpstreamPathTemplate(route, reRoute.Priority, containsQueryString);
+        }
+
+        private bool ForwardSlashAndOnePlaceHolder(string upstreamTemplate, List<string> placeholders, int postitionOfPlaceHolderClosingBracket)
+        {
+            if(upstreamTemplate.Substring(0, 2) == "/{" && placeholders.Count == 1 && upstreamTemplate.Length == postitionOfPlaceHolderClosingBracket + 1)
+            {   
+                return true;
+            }
+
+            return false;
+        }
+
+        private bool IsPlaceHolder(string upstreamTemplate, int i)
+        {
+            return upstreamTemplate[i] == '{';
+        }
+    }
+}
diff --git a/test/Ocelot.AcceptanceTests/RoutingTests.cs b/test/Ocelot.AcceptanceTests/RoutingTests.cs
@@ -21,6 +21,41 @@ public RoutingTests()
             _steps = new Steps();
         }
 
+        [Fact]
+        public void should_not_match_forward_slash_in_pattern_before_next_forward_slash()
+        {
+            var port = 31879;
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/api/v{apiVersion}/cards",
+                            DownstreamScheme = "http",
+                            UpstreamPathTemplate = "/api/v{apiVersion}/cards",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                            DownstreamHostAndPorts = new List<FileHostAndPort>
+                            {
+                                new FileHostAndPort
+                                {
+                                    Host = "localhost",
+                                    Port = port,
+                                }
+                            },
+                            Priority = 1
+                        }
+                    }
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn($"http://localhost:{port}/", "/api/v1/aaaaaaaaa/cards", 200, "Hello from Laura"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/api/v1/aaaaaaaaa/cards"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound))
+                .BDDfy();
+        }
+
         [Fact]
         public void should_return_response_404_when_no_configuration_at_all()
         {