Skip to content

Commit

Permalink
Fixing issue 2209, adding extensive set of unit and acceptance tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@ggnaegi
ggnaegi committed Nov 20, 2024
1 parent 7a5046f commit aaa8dbd
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,15 +117,16 @@ private static (bool IsMatch, string Placeholder) IsCatchAllQuery(string templat
/// </summary>
/// <param name="template">The path template.</param>
/// <returns><see langword="true"/> if it matches.</returns>
private static bool IsCatchAllPath(string template) => RegexCatchAllPath().IsMatch(template) && !template.Contains('?');
private static bool IsCatchAllPath(string template) => RegexCatchAllPath().IsMatch(template) && !template.Contains(@"\?");

/// <summary>Checks if the query should be skipped.
/// <para>It should be skipped if it is not present in the path template.</para>
/// <para>It should be skipped if it is not present in the path template.
/// Since the template is escaped, looking for \? not only ?.</para>
/// </summary>
/// <param name="query">The query string.</param>
/// <param name="template">The path template.</param>
/// <returns><see langword="true"/> if query should be skipped.</returns>
private static bool ShouldSkipQuery(string query, string template) => !string.IsNullOrEmpty(query) && !template.Contains('?');
private static bool ShouldSkipQuery(string query, string template) => !string.IsNullOrEmpty(query) && !template.Contains(@"\?");

/// <summary>Escapes all characters except braces, eg { and }.</summary>
/// <param name="input">The input string.</param>
Expand All @@ -149,7 +150,10 @@ private static string EscapeExceptBraces(string input)
escaped.Append(Regex.Escape(c.ToString()));
}
}

return escaped.ToString();

// Here we are not interested in the path itself, only the placeholders.
// Path validation is not part of this class, therefore allowing case-insensitive
// matching.
return $"^(?i){escaped}";
}
}
58 changes: 58 additions & 0 deletions test/Ocelot.AcceptanceTests/Routing/RoutingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,64 @@ public void ShouldMatchComplexQueriesWithEmbeddedPlaceholders(string downstream,
.And(x => ThenTheResponseBodyShouldBe("Hello from Guillaume"))
.BDDfy();
}

[Theory]
[Trait("Bug", "2209")]
[InlineData("/api/invoices/{url0}-{url1}-{url2}", "/api/invoices_{url0}/{url1}-{url2}_abcd/{url3}?urlId={url4}",
"/api/InvoIces_abc/def-ghi_abcd/xyz?urlId=bla", "/api/invoices/abc-def-ghi", "?urlId=bla")]
[InlineData("/api/products/{category}-{subcategory}/{filter}", "/api/products_{category}/{subcategory}_details/{itemId}?filter={filter}",
"/API/PRODUCTS_electronics/computers_details/123?filter=active", "/api/products/electronics-computers/active", "")]
[InlineData("/api/users/{userId}/posts/{postId}/{lang}", "/api/users/{userId}/{postId}_content/{timestamp}?lang={lang}",
"/api/UsErS/101/2022_content/2024?lang=en", "/api/users/101/posts/2022/en", "")]
[InlineData("/api/categories/{cat}-{subcat}?sort={sort}", "/api/categories_{cat}/{subcat}_items/{itemId}?sort={sort}",
"/api/CATEGORIES_home/furniture_items/789?sort=asc", "/api/categories/home-furniture", "?sort=asc")]
[InlineData("/api/orders/{order}-{detail}?status={status}", "/api/orders_{order}/{detail}_invoice/{ref}?status={status}",
"/API/ORDERS_987/abc_invOiCE/123?status=shipped", "/api/orders/987-abc", "?status=shipped")]
[InlineData("/api/transactions/{type}-{region}", "/api/transactions_{type}/{region}_summary/{year}?q={query}",
"/api/TRanSacTIONS_sales/NA_summary/2024?q=forecast", "/api/transactions/sales-NA", "?q=forecast")]
public void ShouldMatchComplexQueriesCaseInsensitive(string downstream, string upstream, string requestUrl, string downstreamPath, string queryString)
{
var port = PortFinder.GetRandomPort();
var route = GivenRoute(port, upstream, downstream);
var configuration = GivenConfiguration(route);
this.Given(x => GivenThereIsAServiceRunningOn(port, downstreamPath, HttpStatusCode.OK, "Hello from Guillaume"))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway(requestUrl))
.Then(x => ThenTheDownstreamUrlPathShouldBe(downstreamPath))
.And(x => ThenTheDownstreamUrlQueryStringShouldBe(queryString))
.And(x => ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
.And(x => ThenTheResponseBodyShouldBe("Hello from Guillaume"))
.BDDfy();
}

[Theory]
[Trait("Bug", "2209")]
[InlineData("/api/invoices/{url0}-{url1}-{url2}", "/api/invoices_{url0}/{url1}-{url2}_abcd/{url3}?urlId={url4}",
"/api/InvoIces_abc/def-ghi_abcd/xyz?urlId=bla", "/api/invoices/abc-def-ghi", "?urlId=bla")]
[InlineData("/api/products/{category}-{subcategory}/{filter}", "/api/products_{category}/{subcategory}_details/{itemId}?filter={filter}",
"/API/PRODUCTS_electronics/computers_details/123?filter=active", "/api/products/electronics-computers/active", "")]
[InlineData("/api/users/{userId}/posts/{postId}/{lang}", "/api/users/{userId}/{postId}_content/{timestamp}?lang={lang}",
"/api/UsErS/101/2022_content/2024?lang=en", "/api/users/101/posts/2022/en", "")]
[InlineData("/api/categories/{cat}-{subcat}?sort={sort}", "/api/categories_{cat}/{subcat}_items/{itemId}?sort={sort}",
"/api/CATEGORIES_home/furniture_items/789?sort=asc", "/api/categories/home-furniture", "?sort=asc")]
[InlineData("/api/orders/{order}-{detail}?status={status}", "/api/orders_{order}/{detail}_invoice/{ref}?status={status}",
"/API/ORDERS_987/abc_invOiCE/123?status=shipped", "/api/orders/987-abc", "?status=shipped")]
[InlineData("/api/transactions/{type}-{region}", "/api/transactions_{type}/{region}_summary/{year}?q={query}",
"/api/TRanSacTIONS_sales/NA_summary/2024?q=forecast", "/api/transactions/sales-NA", "?q=forecast")]
public void ShouldNotMatchComplexQueriesCaseSensitive(string downstream, string upstream, string requestUrl, string downstreamPath, string queryString)
{
var port = PortFinder.GetRandomPort();
var route = GivenRoute(port, upstream, downstream);
route.RouteIsCaseSensitive = true;
var configuration = GivenConfiguration(route);
this.Given(x => GivenThereIsAServiceRunningOn(port, downstreamPath, HttpStatusCode.OK, "Hello from Guillaume"))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunning())
.When(x => WhenIGetUrlOnTheApiGateway(requestUrl))
.Then(x => ThenTheStatusCodeShouldBe(HttpStatusCode.NotFound))
.BDDfy();
}

[Fact]
[Trait("Feat", "91, 94")]
Expand Down
61 changes: 61 additions & 0 deletions ...t.UnitTests/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,59 @@ public void Can_match_all_placeholders_between_slashes(string template, string p
// Assert
ThenTheTemplatesVariablesAre(expectedTemplates.ToArray());
}

[Theory]
[Trait("Bug", "2209")]
[InlineData("/entities/{id}/events/recordsdata/{subCategoryId}_abcd/{itemId}?sort={sortBy}", "/Entities/43/Events/RecordsData/2_abcd/5?sort=desc",
"{id}", "43", "{subCategoryId}", "2", "{itemId}", "5", "{sortBy}", "desc")]
[InlineData("/api/PRODUCTS/{productId}/{category}_{itemId}_DeTails/{status}", "/API/Products/789/electronics_123_details/available",
"{productId}", "789", "{category}", "electronics", "{itemId}", "123", "{status}", "available")]
public void Can_match_all_placeholders_between_slashes_case_insensitive(string template, string path,
string placeholderName1, string placeholderValue1, string placeholderName2, string placeholderValue2,
string placeholderName3, string placeholderValue3, string placeholderName4, string placeholderValue4)
{
// Arrange
var expectedTemplates = new List<PlaceholderNameAndValue>
{
new(placeholderName1, placeholderValue1),
new(placeholderName2, placeholderValue2),
new(placeholderName3, placeholderValue3),
new(placeholderName4, placeholderValue4),
};

// Act
_result = _finder.Find(path, Empty, template);

// Assert
ThenTheTemplatesVariablesAre(expectedTemplates.ToArray());
}

[Theory]
[Trait("Bug", "2209")]
[InlineData("/entities/{Id}/events/recordsdata/{subCategoryId}_abcd/{itemId}?sort={sortBy}",
"/Entities/43/Events/RecordsData/2_abcd/5?sort=desc",
"{id}", "43", "{subcategoryid}", "2", "{itemid}", "5", "{sortby}", "desc")]
[InlineData("/api/PRODUCTS/{productid}/{category}_{itemid}_DeTails/{status}",
"/API/Products/789/electronics_123_details/available",
"{productId}", "789", "{Category}", "electronics", "{itemId}", "123", "{Status}", "available")]
public void Even_if_case_insensitive_cannot_match_placeholders(string template, string path,
string placeholderName1, string placeholderValue1, string placeholderName2, string placeholderValue2,
string placeholderName3, string placeholderValue3, string placeholderName4, string placeholderValue4)
{
var expectedTemplates = new List<PlaceholderNameAndValue>
{
new(placeholderName1, placeholderValue1),
new(placeholderName2, placeholderValue2),
new(placeholderName3, placeholderValue3),
new(placeholderName4, placeholderValue4),
};

// Act
_result = _finder.Find(path, Empty, template);

// Assert;
ThenTheExpectedVariablesCantBeFound(expectedTemplates.ToArray());
}

private void ThenSinglePlaceholderIs(string expectedName, string expectedValue)
{
Expand All @@ -371,4 +424,12 @@ private void ThenTheTemplatesVariablesAre(params PlaceholderNameAndValue[] colle
ThenSinglePlaceholderIs(expected.Name, expected.Value);
}
}

private void ThenTheExpectedVariablesCantBeFound(params PlaceholderNameAndValue[] collection)
{
foreach (var expected in collection)
{
_result.Data.FirstOrDefault(t => t.Name == expected.Name).ShouldBeNull();
}
}
}

0 comments on commit aaa8dbd

Please sign in to comment.