Merge pull request #122 from TomPallister/feature/fix-always-adding-t…

…railing-slash fixes issue #117
ThreeMammals · Aug 29, 2017 · 6f5c296 · 6f5c296
2 parents 41e9dfa + 7ef26f5
commit 6f5c296
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 4 deletions.
diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs
@@ -6,6 +6,7 @@
 using Ocelot.DownstreamRouteFinder.UrlMatcher;
 using Ocelot.Errors;
 using Ocelot.Responses;
+using Ocelot.Utilities;
 
 namespace Ocelot.DownstreamRouteFinder.Finder
 {
@@ -24,6 +25,8 @@ public DownstreamRouteFinder(IOcelotConfigurationProvider configProvider, IUrlPa
 
         public async Task<Response<DownstreamRoute>> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod)
         {
+            upstreamUrlPath = upstreamUrlPath.SetLastCharacterAs('/');
+
             var configuration = await _configProvider.Get();
 
             var applicableReRoutes = configuration.Data.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower()));

diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs
@@ -30,7 +30,7 @@ public DownstreamRouteFinderMiddleware(RequestDelegate next,
 
         public async Task Invoke(HttpContext context)
         {
-            var upstreamUrlPath = context.Request.Path.ToString().SetLastCharacterAs('/');
+            var upstreamUrlPath = context.Request.Path.ToString();
 
             _logger.LogDebug("upstream url path is {upstreamUrlPath}", upstreamUrlPath);
 

diff --git a/test/Ocelot.AcceptanceTests/RoutingTests.cs b/test/Ocelot.AcceptanceTests/RoutingTests.cs
@@ -6,6 +6,7 @@
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Ocelot.Configuration.File;
+using Shouldly;
 using TestStack.BDDfy;
 using Xunit;
 
@@ -15,6 +16,7 @@ public class RoutingTests : IDisposable
     {
         private IWebHost _builder;
         private readonly Steps _steps;
+        private string _downstreamPath;
 
         public RoutingTests()
         {
@@ -232,6 +234,34 @@ public void should_return_response_200_with_complex_url()
                 .BDDfy();
         }
 
+
+        [Fact]
+        public void should_not_add_trailing_slash_to_downstream_url()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/api/products/{productId}",
+                            DownstreamScheme = "http",
+                            DownstreamHost = "localhost",
+                            DownstreamPort = 51879,
+                            UpstreamPathTemplate = "/products/{productId}",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                        }
+                    }
+            };
+
+            this.Given(x => GivenThereIsAServiceRunningOn("http://localhost:51879/api/products/1", 200, "Some Product"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/products/1"))
+                .Then(x => ThenTheDownstreamUrlPathShouldBe("/api/products/1"))
+                .BDDfy();
+        }
+
         [Fact]
         public void should_return_response_201_with_simple_url()
         {
@@ -379,11 +409,11 @@ private void GivenThereIsAServiceRunningOn(string url, int statusCode, string re
                 .UseKestrel()
                 .UseContentRoot(Directory.GetCurrentDirectory())
                 .UseIISIntegration()
-                .UseUrls(url)
                 .Configure(app =>
                 {
                     app.Run(async context =>
                     {
+                        _downstreamPath = context.Request.PathBase.Value;
                         context.Response.StatusCode = statusCode;
                         await context.Response.WriteAsync(responseBody);
                     });
@@ -393,6 +423,11 @@ private void GivenThereIsAServiceRunningOn(string url, int statusCode, string re
             _builder.Start();
         }
 
+        internal void ThenTheDownstreamUrlPathShouldBe(string expectedDownstreamPath)
+        {
+            _downstreamPath.ShouldBe(expectedDownstreamPath);
+        }
+
         public void Dispose()
         {
             _builder?.Dispose();

diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs
@@ -36,7 +36,7 @@ public DownstreamRouteFinderTests()
         [Fact]
         public void should_return_route()
         {
-            this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher"))
+            this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher/"))
                 .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns(
                         new OkResponse<List<UrlPathPlaceholderNameAndValue>>(
                             new List<UrlPathPlaceholderNameAndValue>())))
@@ -65,6 +65,39 @@ public void should_return_route()
                 .BDDfy();
         }
 
+
+        [Fact]
+        public void should_append_slash_to_upstream_url_path()
+        {
+            this.Given(x => x.GivenThereIsAnUpstreamUrlPath("matchInUrlMatcher"))
+                .And(x =>x.GivenTheTemplateVariableAndNameFinderReturns(
+                        new OkResponse<List<UrlPathPlaceholderNameAndValue>>(
+                            new List<UrlPathPlaceholderNameAndValue>())))
+                .And(x => x.GivenTheConfigurationIs(new List<ReRoute>
+                {
+                    new ReRouteBuilder()
+                        .WithDownstreamPathTemplate("someDownstreamPath")
+                        .WithUpstreamPathTemplate("someUpstreamPath")
+                        .WithUpstreamHttpMethod(new List<string> { "Get" })
+                        .WithUpstreamTemplatePattern("someUpstreamPath")
+                        .Build()
+                }, string.Empty
+                ))
+                .And(x => x.GivenTheUrlMatcherReturns(new OkResponse<UrlMatch>(new UrlMatch(true))))
+                .And(x => x.GivenTheUpstreamHttpMethodIs("Get"))
+                .When(x => x.WhenICallTheFinder())
+                .Then(
+                    x => x.ThenTheFollowingIsReturned(new DownstreamRoute(
+                            new List<UrlPathPlaceholderNameAndValue>(),
+                            new ReRouteBuilder()
+                                .WithDownstreamPathTemplate("someDownstreamPath")
+                                .WithUpstreamHttpMethod(new List<string> { "Get" })
+                                .Build()
+                )))
+                .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher/"))
+                .BDDfy();
+        }
+
         [Fact]
         public void should_return_route_if_upstream_path_and_upstream_template_are_the_same()
         {
@@ -137,7 +170,7 @@ public void should_return_correct_route_for_http_verb()
         [Fact]
         public void should_not_return_route()
         {
-            this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath"))
+            this.Given(x => x.GivenThereIsAnUpstreamUrlPath("dontMatchPath/"))
                  .And(x => x.GivenTheConfigurationIs(new List<ReRoute>
                      {
                         new ReRouteBuilder()
@@ -269,6 +302,12 @@ private void ThenTheUrlMatcherIsCalledCorrectly()
                 .Verify(x => x.Match(_upstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once);
         }
 
+        private void ThenTheUrlMatcherIsCalledCorrectly(string expectedUpstreamUrlPath)
+        {
+            _mockMatcher
+                .Verify(x => x.Match(expectedUpstreamUrlPath, _reRoutesConfig[0].UpstreamPathTemplate.Value), Times.Once);
+        }
+
         private void ThenTheUrlMatcherIsNotCalled()
         {
             _mockMatcher