diff --git a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs b/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs index bffb8392..3aba7c20 100644 --- a/src/Ocelot/Configuration/Creator/FileOcelotConfigurationCreator.cs +++ b/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 { diff --git a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs b/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs index 4e02a158..92559bfb 100644 --- a/src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs +++ b/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(); 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}"; diff --git a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs b/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs index e39fffc6..2c3077cc 100644 --- a/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs +++ b/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 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) diff --git a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs b/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs index b5351e2a..b5e72218 100644 --- a/src/Ocelot/DownstreamRouteFinder/Middleware/DownstreamRouteFinderMiddleware.cs +++ b/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 { diff --git a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs b/src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs index 8e5a2416..946a365c 100644 --- a/src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs +++ b/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])) { diff --git a/src/Ocelot/Utilities/StringExtensions.cs b/src/Ocelot/Utilities/StringExtensions.cs deleted file mode 100644 index 4655a98a..00000000 --- a/src/Ocelot/Utilities/StringExtensions.cs +++ /dev/null @@ -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; - } - } -} diff --git a/test/Ocelot.AcceptanceTests/RoutingTests.cs b/test/Ocelot.AcceptanceTests/RoutingTests.cs index 4994aff7..926cb849 100644 --- a/test/Ocelot.AcceptanceTests/RoutingTests.cs +++ b/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 + { + new FileReRoute + { + DownstreamPathTemplate = "/api/{url}", + DownstreamScheme = "http", + DownstreamHost = "localhost", + DownstreamPort = 51899, + UpstreamPathTemplate = "/platform/{url}", + UpstreamHttpMethod = new List { "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); }); diff --git a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs b/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs index 21c5be38..fba285e2 100644 --- a/test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs +++ b/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(); } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs index 2e586c23..a1839bea 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/DownstreamRouteFinderTests.cs +++ b/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 { "Get" }) .Build() ))) - .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher/")) + .And(x => x.ThenTheUrlMatcherIsCalledCorrectly("matchInUrlMatcher")) .BDDfy(); } diff --git a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/TemplateVariableNameAndValueFinderTests.cs b/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/TemplateVariableNameAndValueFinderTests.cs index 7c73483a..fded9ecf 100644 --- a/test/Ocelot.UnitTests/DownstreamRouteFinder/UrlMatcher/TemplateVariableNameAndValueFinderTests.cs +++ b/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())) + .BDDfy(); + } + [Fact] public void can_match_down_stream_url_with_no_slash() { diff --git a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj index 415fb07b..f0ad0d8d 100644 --- a/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj +++ b/test/Ocelot.UnitTests/Ocelot.UnitTests.csproj @@ -31,6 +31,12 @@ + + + PreserveNewest + + + diff --git a/test/Ocelot.UnitTests/idsrv3test.pfx b/test/Ocelot.UnitTests/idsrv3test.pfx new file mode 100644 index 00000000..0247dea0 Binary files /dev/null and b/test/Ocelot.UnitTests/idsrv3test.pfx differ