Catch All functionality priority routing partially broken

[suchmememanyskill]

Bug

Since release v23, the catch all functionality seems to be broken on subpaths. Absolute routes no longer match over catch-all ones.

Screenshot 1 works as intended, Screenshot 2 does not.
This seems to be related to this PR: #1911

Expected behaviour (Screenshot 1):

• http://localhost:5000/ -> http://localhost:8000/specific/
• http://localhost:5000/url -> http://localhost:8000/generic/url

Actual behaviour (Screenshot 1):

• http://localhost:5000/ -> http://localhost:8000/specific/
• http://localhost:5000/url -> http://localhost:8000/generic/url

Screenshot 1:

Expected behaviour (Screenshot 2):

• http://localhost:5000/test -> http://localhost:8000/specific/
• http://localhost:5000/test/ -> http://localhost:8000/specific/
• http://localhost:5000/test/abc -> http://localhost:8000/generic/abc

Actual behaviour (Screenshot 2):

• http://localhost:5000/test -> http://localhost:8000/generic/ (Unexpected!)
• http://localhost:5000/test/ -> http://localhost:8000/generic/ (Unexpected!)
• http://localhost:5000/test/abc -> http://localhost:8000/generic/abc

Screenshot 2:

Specifications

• Version: Ocelot 23.3.3
• Platform: Windows, Dotnet 8.0.302
• Subsystem: Kestrel

[suchmememanyskill]

This seems to also happen with multiple levels of catch all

Ocelot:

  "Routes": [
    {
      "UpstreamHttpMethod": [ "Get" ],
      "UpstreamPathTemplate": "/test/{url}/{url2}",
      "DownstreamPathTemplate": "/generic/{url}/{url2}",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "localhost",
          "Port": 8000
        }
      ]
    },
    {
      "UpstreamHttpMethod": [ "Get" ],
      "UpstreamPathTemplate": "/test/{url}",
      "DownstreamPathTemplate": "/specific/{url}",
      "DownstreamScheme": "http",
      "DownstreamHostAndPorts": [
        {
          "Host": "localhost",
          "Port": 8000
        }
      ]
    }
  ],
  "GlobalConfiguration": {
    "BaseUrl": "https://localhost:5000"
  }

Only curl http://localhost:5000/test > nul matches the /specific route. http://localhost:5000/test/abc/a, http://localhost:5000/test/abc, http://localhost:5000/test/a all match the /generic route.

[raman-m]

Hello Sims!
I see you like to have a testing fun. OK.

Since release v23, the catch all functionality seems to be broken on subpaths. Absolute routes no longer match over catch-all ones.

Actually, the functionality was "broken"/changed specifically in version 23.0.0 as seen in the commit f4803c2.

Screenshot 1 works as intended, Screenshot 2 does not.

Understood. However, it's important to note that the two screenshots pertain to distinct features. Additional details follow.

This seems to be related to this PR: #1911

Correct! It is connected to PR #1911. While #748 was initially closed with the bug fix in #1911, it turns out that #748 contains new functionality requirements, making #1911 a feature PR rather than a simple bug fix.

[raman-m]

Expected behaviour (Screenshot 1):

• http://localhost:5000/ -> http://localhost:8000/specific/
• http://localhost:5000/url -> http://localhost:8000/generic/url

Actual behaviour (Screenshot 1):

• http://localhost:5000/ -> http://localhost:8000/specific/
• http://localhost:5000/url -> http://localhost:8000/generic/url

Screenshot 1:

The screenshot and user scenario depicted precisely illustrate the Catch All Routing feature. It is operational and continues to function in version 23.3. Perfect!

[suchmememanyskill]

Hi @raman-m,
Thank you for your quick reply

I'm a little confused by your response. Is this a bug or am i not using the features of Ocelot correctly?

Understood. However, it's important to note that the two screenshots pertain to distinct features. Additional details follow.

So is the catch-all feature only supposed to be used for the root path? Can you link me the documentation for a similar feature for a catch-all on subpaths?

Also, am i correct to think that the input url that matches the upstream template more closely should be taken, rather than a priority from first to last? (Otherwise you'd be unable to create more advanced routing definitions)

Another question, can i disable the feature that's causing this as a temporary workaround?
(In my opinion it's a little absurd to add routing definitions that are not explicitly defined. How can i define very specific routes then that don't result in more paths than intended being mapped?)

[raman-m]

Expected behaviour (Screenshot 2):

• http://localhost:5000/test -> http://localhost:8000/specific/
• http://localhost:5000/test/ -> http://localhost:8000/specific/
• http://localhost:5000/test/abc -> http://localhost:8000/generic/abc

Actual behaviour (Screenshot 2):

• http://localhost:5000/test -> http://localhost:8000/generic/ (Unexpected!)
• http://localhost:5000/test/ -> http://localhost:8000/generic/ (Unexpected!)
• http://localhost:5000/test/abc -> http://localhost:8000/generic/abc

Screenshot 2:

The "Screenshot 2" user scenario pertains to the Empty Placeholders feature. Both upstream templates /test/{url} and /test/ are valid for the Empty Placeholders feature, making both routes applicable. In line with the logic for applicable routes and the Priority routing feature, Ocelot's internal logic selects all suitable routes and assigns priorities as shown in this code snippet:

Ocelot/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs

Lines 32 to 34 in b4e21c4

	var applicableRoutes = configuration.Routes
	.Where(r => RouteIsApplicableToThisRequest(r, httpMethod, upstreamHost))
	.OrderByDescending(x => x.UpstreamTemplatePattern.Priority);

Subsequently, the foreach loop iterates over all the applicable routes and generates a new list of downstream routes:

Ocelot/src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteFinder.cs

Line 36 in b4e21c4

foreach (var route in applicableRoutes)

However, since the Priority feature is not employed, the first route in the list is selected by default.

Additionally, it can be said that the Actual behavior is derived from the Empty Placeholders feature. Conversely, the Expected behavior is a result of the Priority feature.

To achieve the expected behavior, one must employ the Priority routing feature, which is specifically designed for scenarios with implicit or unclear route priorities.

[suchmememanyskill]

To achieve the expected behavior, one must employ the Priority routing feature, which is specifically designed for scenarios with implicit or unclear route priorities.

For #2134 (comment), it's impossible to use priority to solve this. The first match will always swallow the entire url.

[raman-m]

@suchmememanyskill commented

This seems to also happen with multiple levels of catch all

I understand your interest in software testing, but what is the purpose of such detailed analysis? Both "/test/{url}/{url2}" and /test/{url} are logically equivalent. What is the reason for this discussion? Theoretically, you might want to extract the first {url} subpath and apply it in custom C# logic, but is there actually any custom logic in place?

Only curl http://localhost:5000/test > nul matches the /specific route.

Indeed, the URL matches the second route template because it does not have two subpaths required to match the first route. Bingo!

http://localhost:5000/test/abc/a, http://localhost:5000/test/abc, http://localhost:5000/test/a all match the /generic route.

Indeed, the URLs http://localhost:5000/test/abc/a, http://localhost:5000/test/abc, and http://localhost:5000/test/a all correspond to the /generic route. This is due to each URL containing at least one subpath, which triggers the first route to be applicable under the internal criteria of the Empty Placeholders feature.

All behavior is correct! There are no issues. However, there seems to be a lack of clarity regarding the Routing feature, which warrants a second reading by you. Experimenting with subpaths while disregarding the existing, ready-to-use features is nonsensical. Such experiments often lead to confusion and are prone to failure. It's advisable to fully utilize the power of the already implemented Routing features before attempting to develop more customized solutions on top of the existing functionality.

[raman-m]

@suchmememanyskill commented on Aug 2, 2024

For #2134 (comment), it's impossible to use priority to solve this. The first match will always swallow the entire url.

I haven't had the time to verify the accuracy of the statement, but it appears everything is detailed in my #2134 (comment).
Priorities are only sensible for applicable routes! In this instance, since the routes differ and each URL has just one applicable route, the Priority feature becomes redundant. Correct? Thus, for this scenario, you would need to use a different upstream path prefix, and a possible solution could be as follows:

  "Routes": [
    {
      "UpstreamPathTemplate": "/generic/{url}/{url2}",
      "DownstreamPathTemplate": "/generic/{url}/{url2}",
    },
    {
      "UpstreamPathTemplate": "/specific/{url}",
      "DownstreamPathTemplate": "/specific/{url}",
    }
  ],
  "GlobalConfiguration": {
    "BaseUrl": "https://localhost:5000"
  }

This solution is elegant and avoids the need for debates with the product owner. 😉
If you don't mind, I'll convert this thread into a public discussion.
Best of luck with your endeavor with Ocelot!