Make Ocelot not add forward slash to downstream url (#158)

* removed code where we add a trailing slash as this means if we request /1.txt/ instead of /1.txt then some servers will not return the resource at /1.txt. After reading up it seems if you dont have a trailing slash then its a file, if you do then its a resource

* test for 145

* removed unused code

* fix broken build..my bad

src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs

@@ -14,7 +14,6 @@ using Ocelot.LoadBalancer.LoadBalancers;
 using Ocelot.Logging;
 using Ocelot.Requester.QoS;
 using Ocelot.Responses;
-using Ocelot.Utilities;
 
 namespace Ocelot.Configuration.Creator
 {

src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs

@@ -1,12 +1,11 @@
 using System.Collections.Generic;
 using Ocelot.Configuration.File;
-using Ocelot.Utilities;
 
 namespace Ocelot.Configuration.Creator
 {
     public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator
     {
-        private const string RegExMatchEverything = ".*";
+        private const string RegExMatchEverything = "[0-9a-zA-Z].*";
         private const string RegExMatchEndString = "$";
         private const string RegExIgnoreCase = "(?i)";
         private const string RegExForwardSlashOnly = "^/$";
@@ -15,8 +14,6 @@ namespace Ocelot.Configuration.Creator
         {
             var upstreamTemplate = reRoute.UpstreamPathTemplate;
 
-            upstreamTemplate = upstreamTemplate.SetLastCharacterAs('/');
-
             var placeholders = new List<string>();
 
             for (var i = 0; i < upstreamTemplate.Length; i++)
@@ -40,6 +37,11 @@ namespace Ocelot.Configuration.Creator
                 return RegExForwardSlashOnly;
             }
 
+            if(upstreamTemplate.EndsWith("/"))
+            {
+                upstreamTemplate = upstreamTemplate.Remove(upstreamTemplate.Length -1, 1) + "(/|)";
+            }
+
             var route = reRoute.ReRouteIsCaseSensitive 
                 ? $"^{upstreamTemplate}{RegExMatchEndString}" 
                 : $"^{RegExIgnoreCase}{upstreamTemplate}{RegExMatchEndString}";

src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs

@@ -7,7 +7,6 @@ using Ocelot.Configuration.Provider;
 using Ocelot.DownstreamRouteFinder.UrlMatcher;
 using Ocelot.Errors;
 using Ocelot.Responses;
-using Ocelot.Utilities;
 
 namespace Ocelot.DownstreamRouteFinder.Finder
 {
@@ -24,8 +23,6 @@ namespace Ocelot.DownstreamRouteFinder.Finder
 
         public Response<DownstreamRoute> FindDownstreamRoute(string upstreamUrlPath, string upstreamHttpMethod, IOcelotConfiguration configuration)
         {
-            upstreamUrlPath = upstreamUrlPath.SetLastCharacterAs('/');
-            
             var applicableReRoutes = configuration.ReRoutes.Where(r => r.UpstreamHttpMethod.Count == 0 || r.UpstreamHttpMethod.Select(x => x.Method.ToLower()).Contains(upstreamHttpMethod.ToLower()));
 
             foreach (var reRoute in applicableReRoutes)

src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs

@@ -8,7 +8,6 @@ using Ocelot.Infrastructure.Extensions;
 using Ocelot.Infrastructure.RequestData;
 using Ocelot.Logging;
 using Ocelot.Middleware;
-using Ocelot.Utilities;
 
 namespace Ocelot.DownstreamRouteFinder.Middleware
 {

src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs

@@ -13,7 +13,7 @@ namespace Ocelot.DownstreamRouteFinder.UrlMatcher
          
             for (int counterForTemplate = 0; counterForTemplate < upstreamUrlPathTemplate.Length; counterForTemplate++)
             {
-                if (CharactersDontMatch(upstreamUrlPathTemplate[counterForTemplate], upstreamUrlPath[counterForUrl]) && ContinueScanningUrl(counterForUrl,upstreamUrlPath.Length))
+                if ((upstreamUrlPath.Length > counterForUrl) && CharactersDontMatch(upstreamUrlPathTemplate[counterForTemplate], upstreamUrlPath[counterForUrl]) && ContinueScanningUrl(counterForUrl,upstreamUrlPath.Length))
                 {
                     if (IsPlaceholder(upstreamUrlPathTemplate[counterForTemplate]))
                     {

src/Ocelot/Utilities/StringExtensions.cs

@@ -1,17 +0,0 @@
-namespace Ocelot.Utilities
-{
-    public static class StringExtensions
-    {
-        public static string SetLastCharacterAs(this string valueToSetLastChar, 
-            char expectedLastChar)
-        {
-            var last = valueToSetLastChar[valueToSetLastChar.Length - 1];
-
-            if (last != expectedLastChar)
-            {
-                valueToSetLastChar = $"{valueToSetLastChar}{expectedLastChar}";
-            }
-            return valueToSetLastChar;
-        }
-    }
-}

test/Ocelot.AcceptanceTests/RoutingTests.cs

@@ -159,7 +159,7 @@ namespace Ocelot.AcceptanceTests
         }
 
         [Fact]
-        public void should_not_care_about_no_trailing()
+        public void should_return_ok_when_upstream_url_ends_with_forward_slash_but_template_does_not()
         {
             var configuration = new FileConfiguration
             {
@@ -187,7 +187,7 @@ namespace Ocelot.AcceptanceTests
         }
 
         [Fact]
-        public void should_not_care_about_trailing()
+        public void should_return_not_found_when_upstream_url_ends_with_forward_slash_but_template_does_not()
         {
             var configuration = new FileConfiguration
             {
@@ -209,8 +209,7 @@ namespace Ocelot.AcceptanceTests
                 .And(x => _steps.GivenThereIsAConfiguration(configuration))
                 .And(x => _steps.GivenOcelotIsRunning())
                 .When(x => _steps.WhenIGetUrlOnTheApiGateway("/products/"))
-                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
-                .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound))
                 .BDDfy();
         }
 
@@ -483,6 +482,41 @@ namespace Ocelot.AcceptanceTests
                 .BDDfy();
         }
 
+
+        [Fact]
+        public void should_fix_145()
+        {
+            var configuration = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                    {
+                        new FileReRoute
+                        {
+                            DownstreamPathTemplate = "/api/{url}",
+                            DownstreamScheme = "http",
+                            DownstreamHost = "localhost",
+                            DownstreamPort = 51899,
+                            UpstreamPathTemplate = "/platform/{url}",
+                            UpstreamHttpMethod = new List<string> { "Get" },
+                            QoSOptions = new FileQoSOptions {
+                                ExceptionsAllowedBeforeBreaking = 3,
+                                DurationOfBreak = 10,
+                                TimeoutValue = 5000
+                            }
+                        }
+                    }
+            };
+
+            this.Given(x => x.GivenThereIsAServiceRunningOn("http://localhost:51899", "", 200, "Hello from Laura"))
+                .And(x => _steps.GivenThereIsAConfiguration(configuration))
+                .And(x => _steps.GivenOcelotIsRunning())
+                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/platform/swagger/lib/backbone-min.js"))
+                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
+                .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
+                .And(x => ThenTheDownstreamUrlPathShouldBe("/api/swagger/lib/backbone-min.js"))
+                .BDDfy();
+        }
+
         private void GivenThereIsAServiceRunningOn(string baseUrl, string basePath, int statusCode, string responseBody)
         {
             _builder = new WebHostBuilder()
@@ -495,7 +529,7 @@ namespace Ocelot.AcceptanceTests
                     app.UsePathBase(basePath);
                     app.Run(async context =>
                     {   
-                        _downstreamPath = context.Request.PathBase.Value;
+                        _downstreamPath = !string.IsNullOrEmpty(context.Request.PathBase.Value) ? context.Request.PathBase.Value : context.Request.Path.Value;
                         context.Response.StatusCode = statusCode;
                         await context.Response.WriteAsync(responseBody);
                     });

test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs

@@ -28,7 +28,23 @@ namespace Ocelot.UnitTests.Configuration
 
             this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
                 .When(x => x.WhenICreateTheTemplatePattern())
-                .Then(x => x.ThenTheFollowingIsReturned("^(?i)/PRODUCTS/.*/$"))
+                .Then(x => x.ThenTheFollowingIsReturned("^(?i)/PRODUCTS/[0-9a-zA-Z].*$"))
+                .BDDfy();
+        }
+
+
+        [Fact]
+        public void should_match_forward_slash_or_no_forward_slash_if_template_end_with_forward_slash()
+        {
+            var fileReRoute = new FileReRoute
+            {
+                UpstreamPathTemplate = "/PRODUCTS/",
+                ReRouteIsCaseSensitive = false
+            };
+
+            this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
+                .When(x => x.WhenICreateTheTemplatePattern())
+                .Then(x => x.ThenTheFollowingIsReturned("^(?i)/PRODUCTS(/|)$"))
                 .BDDfy();
         }
 
@@ -42,7 +58,7 @@ namespace Ocelot.UnitTests.Configuration
                 };
             this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
                 .When(x => x.WhenICreateTheTemplatePattern())
-                .Then(x => x.ThenTheFollowingIsReturned("^/PRODUCTS/.*/$"))
+                .Then(x => x.ThenTheFollowingIsReturned("^/PRODUCTS/[0-9a-zA-Z].*$"))
                 .BDDfy();
         }
 
@@ -57,7 +73,7 @@ namespace Ocelot.UnitTests.Configuration
 
             this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
                 .When(x => x.WhenICreateTheTemplatePattern())
-                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/.*/$"))
+                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/[0-9a-zA-Z].*$"))
                 .BDDfy();
         }
 
@@ -72,9 +88,10 @@ namespace Ocelot.UnitTests.Configuration
 
             this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
                 .When(x => x.WhenICreateTheTemplatePattern())
-                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/.*/variants/.*/$"))
+                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/[0-9a-zA-Z].*/variants/[0-9a-zA-Z].*$"))
                 .BDDfy();
         }
+
         [Fact]
         public void should_create_template_pattern_that_matches_more_than_one_placeholder_with_trailing_slash()
         {
@@ -86,7 +103,7 @@ namespace Ocelot.UnitTests.Configuration
 
             this.Given(x => x.GivenTheFollowingFileReRoute(fileReRoute))
                 .When(x => x.WhenICreateTheTemplatePattern())
-                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/.*/variants/.*/$"))
+                .Then(x => x.ThenTheFollowingIsReturned("^/api/products/[0-9a-zA-Z].*/variants/[0-9a-zA-Z].*(/|)$"))
                 .BDDfy();
         }
 

test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs

@@ -68,7 +68,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder
 
 
         [Fact]
-        public void should_append_slash_to_upstream_url_path()
+        public void should_not_append_slash_to_upstream_url_path()
         {
             var serviceProviderConfig = new ServiceProviderConfigurationBuilder().Build();
 
@@ -97,7 +97,7 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder
                                 .WithUpstreamHttpMethod(new List<string> { "Get" })
                                 .Build()
                 )))
-                .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher/"))
+                .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher"))
                 .BDDfy();
         }
 

test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/TemplateVariableNameAndValueFinderTests.cs

@@ -30,6 +30,16 @@ namespace Ocelot.UnitTests.DownstreamRouteFinder.UrlMatcher
                 .BDDfy();
         }
 
+        [Fact]
+        public void should_not_find_anything()
+        {
+            this.Given(x => x.GivenIHaveAUpstreamPath("/products"))
+                .And(x => x.GivenIHaveAnUpstreamUrlTemplate("/products/"))
+                .When(x => x.WhenIFindTheUrlVariableNamesAndValues())
+                .And(x => x.ThenTheTemplatesVariablesAre(new List<UrlPathPlaceholderNameAndValue>()))
+                .BDDfy();
+        }
+
         [Fact]
         public void can_match_down_stream_url_with_no_slash()
         {

test/Ocelot.UnitTests/Ocelot.UnitTests.csproj

@@ -31,6 +31,12 @@
   <Service Include="{82a7f48d-3b50-4b1e-b82e-3ada8210c358}" />
   </ItemGroup>
 
+   <ItemGroup>
+    <None Update="idsrv3test.pfx">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.0" />
     <PackageReference Include="Microsoft.AspNetCore.TestHost" Version="2.0.0" />