Skip to content

Commit

Permalink
#748 Match Route configurations for upstream paths when empty Catch-A…
Browse files Browse the repository at this point in the history 
…ll placeholders at the end of template (#1911)

* Update RoutingTests.cs

* Fix end of line empty placeholder

* Fix unit tests

* Fix PR Comments

* Update src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Update src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Update test/Ocelot.AcceptanceTests/RoutingTests.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Update test/Ocelot.AcceptanceTests/RoutingTests.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Update test/Ocelot.UnitTests/Configuration/UpstreamTemplatePatternCreatorTests.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* compact the tests constant name

* remove constant

* Update RoutingTests.cs

* Update RoutingTests.cs

* Update RoutingTests.cs

* Update UrlPathPlaceholderNameAndValueFinderTests.cs

* Use range operator

* Use expression body for method

* Update src/Ocelot/DownstreamUrlCreator/Middleware/DownstreamUrlCreatorMiddleware.cs

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>

* Update DownstreamUrlCreatorMiddleware.cs

* add extra unit tests

* Update RoutingTests.cs

* Clean code

* Update UrlPathPlaceholderNameAndValueFinderTests.cs

* Update DownstreamUrlCreatorMiddlewareTests.cs

* Update DownstreamUrlCreatorMiddlewareTests.cs

* Fix broken dsPath

* Review tests. Add query string scenarios

* Fix unit test and fix +1 issue

* Add final routing tests for Catch-All query string cases

* Fixed added unit tests

* Test traits

* Update docs of Routing feature

---------

Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
  • Loading branch information
@AlyHKafoury @raman-m
AlyHKafoury and raman-m authored Jan 18, 2024
1 parent 5e7e76b commit f4803c2
Show file tree
Hide file tree
Showing 9 changed files with 375 additions and 134 deletions.
39 changes: 37 additions & 2 deletions docs/features/routing.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,16 @@ The **UpstreamPathTemplate** property is the URL that Ocelot will use to identif
The **UpstreamHttpMethod** is used so Ocelot can distinguish between requests with different HTTP verbs to the same URL.
You can set a specific list of HTTP methods or set an empty list to allow any of them.

.. _routing-placeholders:

Placeholders
------------

In Ocelot you can add placeholders for variables to your Templates in the form of ``{something}``.
The placeholder variable needs to be present in both the **DownstreamPathTemplate** and **UpstreamPathTemplate** properties.
When it is Ocelot will attempt to substitute the value in the **UpstreamPathTemplate** placeholder into the **DownstreamPathTemplate** for each request Ocelot processes.

You can also do a `Catch All <#catch-all>`_ type of Route e.g.
You can also do a :ref:`routing-catch-all` type of Route e.g.

.. code-block:: json
Expand All @@ -69,6 +71,36 @@ In order to change this you can specify on a per Route basis the following setti
This means that when Ocelot tries to match the incoming upstream URL with an upstream template the evaluation will be case sensitive.

.. _routing-empty-placeholders:

Empty Placeholders
^^^^^^^^^^^^^^^^^^

This is a special edge case of :ref:`routing-placeholders`, where the value of the placeholder is simply an empty string ``""``.

For example, **Given a route**:

.. code-block:: json
{
"UpstreamPathTemplate": "/invoices/{url}",
"DownstreamPathTemplate": "/api/invoices/{url}",
}
.. role:: htm(raw)
:format: html

| **Then**, it works correctly when ``{url}`` is specified: ``/invoices/123`` :htm:`&rarr;` ``/api/invoices/123``.
| **And then**, there are two edge cases with empty placeholder value:
* Also, it works when ``{url}`` is empty. We would expect upstream path ``/invoices/`` to route to downstream path ``/api/invoices/``
* Moreover, it should work when omitting last slash. We also expect upstream ``/invoices`` to be routed to downstream ``/api/invoices``, which is intuitive to humans

This feature is available starting from Ocelot version `23.0 <https://github.com/ThreeMammals/Ocelot/releases/tag/23.0.0>`_,
see more in issue `748 <https://github.com/ThreeMammals/Ocelot/issues/748>`_ and release `23.0 <https://github.com/ThreeMammals/Ocelot/releases/tag/23.0.0>`__ notes.

.. _routing-catch-all:

Catch All
---------

Expand Down Expand Up @@ -211,7 +243,7 @@ Note, the best practice is giving different placeholder name than the name of qu
Catch All Query String
^^^^^^^^^^^^^^^^^^^^^^

Ocelot's routing also supports a *Catch All* style routing to forward all query string parameters.
Ocelot's routing also supports a :ref:`routing-catch-all` style routing to forward all query string parameters.
The placeholder ``{everything}`` name does not matter, any name will work.

.. code-block:: json
Expand All @@ -224,6 +256,9 @@ The placeholder ``{everything}`` name does not matter, any name will work.
This entire query string routing feature is very useful in cases where the query string should not be transformed but rather routed without any changes,
such as OData filters and etc (see issue `1174 <https://github.com/ThreeMammals/Ocelot/issues/1174>`_).

**Note**, the ``{everything}`` placeholder can be empty while catching all query strings, because this is a part of the :ref:`routing-empty-placeholders` feature!
Thus, upstream paths ``/contracts?`` and ``/contracts`` are routed to downstream path ``/apipath/contracts``, which has no query string at all.

Restrictions on use
^^^^^^^^^^^^^^^^^^^

Expand Down
6 changes: 3 additions & 3 deletions docs/introduction/gotchas.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ We try to optimize Ocelot web app for Kestrel & Docker hosting scenarios, but ke
We believe that your client apps should have direct integration to (static) files persistent storages and services: remote & destributed file systems, CDNs, static files & blob storages, etc.
We **do not** recommend to pump large files (100Mb+ or even larger 1GB+) using gateway because of performance reasons: consuming memory and CPU, long delay times, producing network errors for downstream streaming, impact on other routes.

| The community constanly reports issues related to `large files <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+%22large+file%22&type=issues>`_ (``application/octet-stream`` content type, :ref:`chunked-encoding`, etc.), see issues `749 <https://github.com/ThreeMammals/Ocelot/issues/749>`_, `1472 <https://github.com/ThreeMammals/Ocelot/issues/1472>`_.
If you still want to pump large files through an Ocelot gateway instance, we believe our PRs (`1724 <https://github.com/ThreeMammals/Ocelot/pull/1724>`_, `1769 <https://github.com/ThreeMammals/Ocelot/pull/1769>`_) will help resolve the issues and stabilize large content proxying.
In case of some errors, see the next point.
| The community constanly reports issues related to `large files <https://github.com/search?q=repo%3AThreeMammals%2FOcelot+%22large+file%22&type=issues>`_, ``application/octet-stream`` content type, :ref:`chunked-encoding`, etc., see issues `749 <https://github.com/ThreeMammals/Ocelot/issues/749>`_, `1472 <https://github.com/ThreeMammals/Ocelot/issues/1472>`_.
| If you still want to pump large files through an Ocelot gateway instance, we believe our PRs `1724 <https://github.com/ThreeMammals/Ocelot/pull/1724>`_, `1769 <https://github.com/ThreeMammals/Ocelot/pull/1769>`_ will help resolve the issues and stabilize large content proxying.
| In case of some errors, see the next point.
* **Maximum request body size**. ASP.NET ``HttpRequest`` behaves erroneously for application instances that do not have their Kestrel `MaxRequestBodySize <https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserverlimits.maxrequestbodysize>`_ option configured correctly and having pumped large files of unpredictable size which exceeds the limit.

Expand Down
14 changes: 10 additions & 4 deletions src/Ocelot/Configuration/Creator/UpstreamTemplatePatternCreator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
using Ocelot.Configuration.File;
using Ocelot.Values;
using Ocelot.Values;

namespace Ocelot.Configuration.Creator
{
public class UpstreamTemplatePatternCreator : IUpstreamTemplatePatternCreator
{
private const string RegExMatchOneOrMoreOfEverything = ".+";
public const string RegExMatchZeroOrMoreOfEverything = ".*";
private const string RegExMatchOneOrMoreOfEverythingUntilNextForwardSlash = "[^/]+";
private const string RegExMatchEndString = "$";
private const string RegExIgnoreCase = "(?i)";
Expand Down Expand Up @@ -40,7 +40,7 @@ public UpstreamPathTemplate Create(IRoute route)
if (upstreamTemplate.Contains('?'))
{
containsQueryString = true;
upstreamTemplate = upstreamTemplate.Replace("?", "\\?");
upstreamTemplate = upstreamTemplate.Replace("?", "(|\\?)");
}

for (var i = 0; i < placeholders.Count; i++)
Expand All @@ -49,7 +49,7 @@ public UpstreamPathTemplate Create(IRoute route)
var indexOfNextForwardSlash = upstreamTemplate.IndexOf("/", indexOfPlaceholder, StringComparison.Ordinal);
if (indexOfNextForwardSlash < indexOfPlaceholder || (containsQueryString && upstreamTemplate.IndexOf('?', StringComparison.Ordinal) < upstreamTemplate.IndexOf(placeholders[i], StringComparison.Ordinal)))
{
upstreamTemplate = upstreamTemplate.Replace(placeholders[i], RegExMatchOneOrMoreOfEverything);
upstreamTemplate = upstreamTemplate.Replace(placeholders[i], RegExMatchZeroOrMoreOfEverything);
}
else
{
Expand All @@ -60,6 +60,12 @@ public UpstreamPathTemplate Create(IRoute route)
if (upstreamTemplate == "/")
{
return new UpstreamPathTemplate(RegExForwardSlashOnly, route.Priority, containsQueryString, route.UpstreamPathTemplate);
}

var index = upstreamTemplate.LastIndexOf('/'); // index of last forward slash
if (index < (upstreamTemplate.Length - 1) && upstreamTemplate[index + 1] == '.')
{
upstreamTemplate = upstreamTemplate[..index] + "(?:|/" + upstreamTemplate[++index..] + ")";
}

if (upstreamTemplate.EndsWith("/"))
Expand Down
67 changes: 41 additions & 26 deletions src/Ocelot/DownstreamRouteFinder/UrlMatcher/UrlPathPlaceholderNameAndValueFinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,56 +17,55 @@ public Response<List<PlaceholderNameAndValue>> Find(string path, string query, s

for (var counterForTemplate = 0; counterForTemplate < pathTemplate.Length; counterForTemplate++)
{
if ((path.Length > counterForPath) && CharactersDontMatch(pathTemplate[counterForTemplate], path[counterForPath]) && ContinueScanningUrl(counterForPath, path.Length))
if (ContinueScanningUrl(counterForPath, path.Length)
&& CharactersDontMatch(pathTemplate[counterForTemplate], path[counterForPath])
&& IsPlaceholder(pathTemplate[counterForTemplate]))
{
if (IsPlaceholder(pathTemplate[counterForTemplate]))
//should_find_multiple_query_string make test pass
if (PassedQueryString(pathTemplate, counterForTemplate))
{
//should_find_multiple_query_string make test pass
if (PassedQueryString(pathTemplate, counterForTemplate))
{
delimiter = '&';
nextDelimiter = '&';
}
delimiter = '&';
nextDelimiter = '&';
}

//should_find_multiple_query_string_and_path makes test pass
if (NotPassedQueryString(pathTemplate, counterForTemplate) && NoMoreForwardSlash(pathTemplate, counterForTemplate))
{
delimiter = '?';
nextDelimiter = '?';
}
//should_find_multiple_query_string_and_path makes test pass
if (NotPassedQueryString(pathTemplate, counterForTemplate) && NoMoreForwardSlash(pathTemplate, counterForTemplate))
{
delimiter = '?';
nextDelimiter = '?';
}

var placeholderName = GetPlaceholderName(pathTemplate, counterForTemplate);
var placeholderName = GetPlaceholderName(pathTemplate, counterForTemplate);

var placeholderValue = GetPlaceholderValue(pathTemplate, query, placeholderName, path, counterForPath, delimiter);
var placeholderValue = GetPlaceholderValue(pathTemplate, query, placeholderName, path, counterForPath, delimiter);

placeHolderNameAndValues.Add(new PlaceholderNameAndValue(placeholderName, placeholderValue));
placeHolderNameAndValues.Add(new PlaceholderNameAndValue(placeholderName, placeholderValue));

counterForTemplate = GetNextCounterPosition(pathTemplate, counterForTemplate, '}');
counterForTemplate = GetNextCounterPosition(pathTemplate, counterForTemplate, '}');

counterForPath = GetNextCounterPosition(path, counterForPath, nextDelimiter);
counterForPath = GetNextCounterPosition(path, counterForPath, nextDelimiter);

continue;
}

return new OkResponse<List<PlaceholderNameAndValue>>(placeHolderNameAndValues);
continue;
}
else if (IsCatchAll(path, counterForPath, pathTemplate))
else if (IsCatchAll(path, counterForPath, pathTemplate) || IsCatchAllAfterOtherPlaceholders(pathTemplate, counterForTemplate))
{
var endOfPlaceholder = GetNextCounterPosition(pathTemplate, counterForTemplate, '}');

var placeholderName = GetPlaceholderName(pathTemplate, 1);
var placeholderName = GetPlaceholderName(pathTemplate, counterForTemplate + 1);

if (NothingAfterFirstForwardSlash(path))
{
placeHolderNameAndValues.Add(new PlaceholderNameAndValue(placeholderName, string.Empty));
}
else
{
var placeholderValue = GetPlaceholderValue(pathTemplate, query, placeholderName, path, counterForPath + 1, '?');
var placeholderValue = GetPlaceholderValue(pathTemplate, query, placeholderName, path, counterForPath, '?');
placeHolderNameAndValues.Add(new PlaceholderNameAndValue(placeholderName, placeholderValue));
}

counterForTemplate = endOfPlaceholder;
counterForPath = GetNextCounterPosition(path, counterForPath, '?');
continue;
}

counterForPath++;
Expand Down Expand Up @@ -97,13 +96,29 @@ private static bool IsCatchAll(string path, int counterForPath, string pathTempl
&& pathTemplate.IndexOf('}') == pathTemplate.Length - 1;
}

private static bool IsCatchAllAfterOtherPlaceholders(string pathTemplate, int counterForTemplate)
=> (pathTemplate[counterForTemplate] == '/' || pathTemplate[counterForTemplate] == '?')
&& (counterForTemplate < pathTemplate.Length - 1)
&& (pathTemplate[counterForTemplate + 1] == '{')
&& NoMoreForwardSlash(pathTemplate, counterForTemplate + 1);

private static bool NothingAfterFirstForwardSlash(string path)
{
return path.Length == 1 || path.Length == 0;
}

private static string GetPlaceholderValue(string urlPathTemplate, string query, string variableName, string urlPath, int counterForUrl, char delimiter)
{
if (counterForUrl >= urlPath.Length)
{
return string.Empty;
}

if ( urlPath[counterForUrl] == '/')
{
counterForUrl++;
}

var positionOfNextSlash = urlPath.IndexOf(delimiter, counterForUrl);

if (positionOfNextSlash == -1 || (urlPathTemplate.Trim(delimiter).EndsWith(variableName) && string.IsNullOrEmpty(query)))
Expand Down
Loading

0 comments on commit f4803c2

Please sign in to comment.