route has small footgun when trying to create nested routes

[dbofmmbt]

Originally reported on Axum's discord channel.

As a new user to Axum, this is a piece of code which may be written while trying to implement nested routes:

let app = Router::new().route(
        "/base",
        Router::new()
            .route("/:anything", get(handler))
            .route("/abc", get(another_handler)),
    );

It compiles fine, although it doesn't work. While investigating the problem, I found out that it's necessary to use the nest function to nest routes properly. It would be great if this code could just work, or generate an error instead of compiling.

[davidpdrsn]

From discord for context:

it happens because axum routes implement tower::Service and route supports calling any tower::Service. So the types all fit, but you're right it wont actually work because the path prefix doesn't get stripped.

[davidpdrsn]

I would actually like to explore if this could be made to "just work" in the way nest works today. Then we could remove nest and just use route. I think that is more natural. Would require some tweaks to the routing in axum but should be do-able.

Note that if we do this we should make sure the implementation is still compatible with openapi generation.

[dbofmmbt]

@davidpdrsn it looks like a great idea, it might be worth exploring this possibility.

[OldManFroggy]

normalizing route and nest so that only route is needed, sounds like a great way forward, if it can be done -> this would also impart issue #177

[davidpdrsn]

I have thought a bit about this and not sure its possible. In order for it to work route should only require a prefix match with the URI, instead of a full match like it does today.

That would mean route("/foo", route("/bar", get(handler))) would match GET /foo/bar which is good. However route doesn't know which service is inside it so route("/foo", get(handler)) would also match GET /foo/bar and handler would be called, which is clearly isn't what you want.

I suppose get, post, etc could check that the whole URL has been "matched" before, otherwise return 404, but that would break routing to arbitrary tower::Services. Would like also be broken in other ways.

[dbofmmbt]

I wonder if it could be possible to detect these routing footguns "while starting/building the service". I don't know how this idea could translate into something doable, though. I might have to take a look at the source myself to make my ideas more concrete 😄

[davidpdrsn]

That would probably encounter the same issues. Axums routing is quite generic by design which sometimes complicates things.

[davidpdrsn]

But you're off course welcome to give it a shot 😊

[dbofmmbt]

In order for it to work route should only require a prefix match with the URI, instead of a full match like it does today.

I suppose get, post, etc could check that the whole URL has been "matched" before, otherwise return 404, but that would break routing to arbitrary tower::Services. Would like also be broken in other ways.

@davidpdrsn if you don't mind, could you give an example showing how these changes would break routing to arbitrary services?

I've been thinking about it lately and the solution of 1) removing nest, 2) making route use prefix match and 3) make handlers like get check if they're in the end of the path seems reasonable. In fact, I think that's exactly what express, from the javascript environment, does. If you want to, take a look at the express router section on their docs.

[davidpdrsn]

I've been thinking about it lately and the solution of 1) removing nest, 2) making route use prefix match and 3) make handlers like get check if they're in the end of the path seems reasonable.

I think your right and this is probably the way we want to go. I'm gonna check out the express docs. Thanks for sharing those.

Something else thats related I've thought about: Currently this is a bit of a foot gun as well route("/foo/bar", ...).nest("/foo", ...). The first handler is inaccessible since /foo is a prefix match of /foo/bar so nest swallows those requests. If we were to remove nest I would be slightly worried about this because route(...).route(...) is less visually distinct so it might be harder to spot.

However I think it can be solved by requests not getting "trapped" inside a nest. So if a nest matches, but none of its sub-routes do, then it would back up and continue before the nest. It would mean that we have to check requests against more routes but I'm not worried about that.

I think we have to implement it and test before we'll know for sure though. I hope to get some time during the weekend to explore this.

[dbofmmbt]

However I think it can be solved by requests not getting "trapped" inside a nest. So if a nest matches, but none of its sub-routes do, then it would back up and continue before the nest.

That would be interesting to experiment. I don't know any web framework who does it this way.

I think that another valid solution would be to detect this "unreachable route" problem before actually serving requests. It looks feasible and it would be very helpful, in case no other solution is accepted.

[Kestrer]

I don't know any web framework who does it this way.

Warp does; Filter::or will try the second filter if the first filter fails in any way.

[davidpdrsn]

Warp does; Filter::or will try the second filter if the first filter fails in any way.

Right. Axum's RouteDsl::or works the same way.

[davidpdrsn]

I did #224 yesterday which makes routing a bit more intuitive I think.

I also took a stab at making route perform prefix matching in the way nest currently does but ran into this issue: What should route("/", some_random_tower_service) do? If route performs prefix matching then this configuration will match all requests. I think this is very surprising and not how I would expect things to work.

nest("/", ...) has the same behavior of capturing all requests, but I think its okay because the word nest explicitly calls out that something is different.

It might still be fixable by only calling some_random_tower_service if the whole URI has been matched. I'll give it a shot at another time.

However with #224 I think routing is in a decent place so I think we should punt on this issue for 0.2 and address it later. I would like to get 0.2 out next week and don't consider this issue critical.

[dbofmmbt]

It might still be fixable by only calling some_random_tower_service if the whole URI has been matched. I'll give it a shot at another time.

If you add that check, then route wouldn't do prefix match anymore, would it?

I think I'll try to write up everything I absorbed about this routing issue later to ease our understanding of the problem. This way we'll be able to figure out an adequate solution.

[dbofmmbt]

Wish List

• I want to just use route to everything without footguns such as the one described in this particular issue.
• It is necessary to preserve routing to arbitrary Services, not only handlers like get.
• Bonus: there must be a way to detect unreachable routes (which are certainly programming mistakes) before serving any requests.

My current view

Currently, axum's routing API is composed of a_function(description, handler). I don't see a way to solve those problems by changing handlers behaviour. Today we have route and nest to distinguish between prefix match and full match, but this may not be intuitive at first.

Fortunately, there's the option to encode prefix | full match desire into the description of the route using * wildcard (which is part of #109). This way, we would have the benefits of just using route for everything while allowing the developer to choose between prefix and full match, depending on the case.

Misconception about Express

Also, I needed to talk about this sentence I wrote a while ago:

1. removing nest, 2) making route use prefix match and 3) make handlers like get check if they're in the end of the path seems reasonable. In fact, I think that's exactly what express, from the javascript environment, does.

Actually, this isn't what Express does. It distinguishes between defining endpoints (which uses full match + regexes) and mouting a group of routes in a prefix path.

Summary

With the following changes the routing part of axum would be in a even better place:

• Add * wildcard to routes description and remove nest (since it loses its usefulness)
• Add detection for unreachable routes before serving requests
• Reverse route ordering #259 (I don't really care so much about this one, but it would be a better default, indeed)

I can try to help with some of those if you don't mind about the timeline right now (I'm close to finish the semester on my university)

Edit: Alternatives

We explain route and nest as "full match" and "prefix match" explicitly in the docs and add detection for unreachable routes. This would probably be less intuitive than * but it would be good enough. However, I think the wildcards for paths will be implemented anyway, so it might be worth to remove nest in favor of them for prefix match.

[davidpdrsn]

Bonus: there must be a way to detect unreachable routes (which are certainly programming mistakes) before serving any requests.

This is orthogonal to how "route" or "nest" works. Changing "route" to do prefix matching won't make detecting unreachable routes easier. That will require changes to the overall router design similar to what's described in #240

Actually, this isn't what Express does. It distinguishes between defining endpoints (which uses full match + regexes) and mouting a group of routes in a prefix path.

Isn't this also what axum does? Except that "mount" is called "nest".

I would be fine with renaming "nest" to "mount". I named it nest originally because that's what tide calls it.

Fortunately, there's the option to encode prefix | full match desire into the description of the route using * wildcard (which is part of #109). This way, we would have the benefits of just using route for everything while allowing the developer to choose between prefix and full match, depending on the case.

Can you give some examples of the kinds of routes you imagine being able to define, and which requests they'd accept? Include both handlers and general services.

Are there other frameworks where using wildcards strips the matched prefix from URI before calling the handler/endpoint/service in the way "nest" does, while using the common "add endpoint here"-method.

Personally this isn't the behavior I would expect. I think the there value in having those two behaviors in two separate functions. It's also not what tide does. They have a specific method you call to enable prefix matching.

[3olkin]

I would be fine with renaming "nest" to "mount".

In rocket mount keyword is used for routing like route in Axum, so this change won’t be intuitive for developers used to use rust for backend IMHO.
Also nest describes the purpose of the keyword better than abstract mount

I believe that self-documented code improve maintainability of the project. So IMHO better solution would be rename route to something like endpoint. Endpoint means finished route. It will be more intuitive then having route and nest in one Router::new()

[davidpdrsn]

So IMHO better solution would be rename route to something like endpoint. Endpoint means finished route. It will be more intuitive then having route and nest in one Router::new()

I disagree. I think "route" is fine. Really there are many words that describe similar things. I don't "endpoint" is objectively better than "route".

Rails also has the concept of "mounting" other apps inside a router. So mounting is a word that's used.

To be clear I have no intentions of renaming nest, unless people thinks it would make it's behavior more clear.

[dbofmmbt]

Changing "route" to do prefix matching won't make detecting unreachable routes easier.

I don't think we should go this way anymore. route would behave based on its path.

Can you give some examples of the kinds of routes you imagine being able to define, and which requests they'd accept? Include both handlers and general services.

Here's a small example:

Current behavior:

.route("/users", handler)
.nest("/api", api_routes)
.route("/service", tower_service)

Propposed behavior:

.route("/users", handler)
.route("/api/*", api_routes)
.route("/service", tower_service)

Let me know if I missed any case that could be interesting to think about.

Isn't this also what axum does? Except that "mount" is called "nest".

Yes. Also, for Express "everything is middleware", and for Axum "everything is Service". I like it.

I would be fine with renaming "nest" to "mount".

I think we shouldn't rename them unless we find evidence that it would be, in fact, better.

It's also not what tide does. They have a specific method you call to enable prefix matching.

Here, we have the option to unify these concepts because axum sees everything as Service. I think Tide could have done the same, based on their Endpoint trait. I'll see if they evaluated this idea.

The main point is: both APIs are fine for me, as long as we add unreachable route detection. However, if * wildcards for paths were implemented, I don't see why we would keep nest, since route("/*", service) would mean nest("/", service).

[davidpdrsn]

My initial thoughts were that route("/api/*", svc) means svc would still see the /api of requests, whereas nest("/api", svc) means svc won't see the /api prefix. It kinda feels like both are useful 🤔

I agree that nest("/api/*", svc) isn't useful, and arguably shouldn't be supported, since the matched prefix is the whole URI so the service would always see /.

[dbofmmbt]

My initial thoughts were that route("/api/*", svc) means svc would still see the /api of requests

Is this useful is practice? I tend to think that handlers don't care where they are mounted. You just "plug" them where you want to.

[davidpdrsn]

Is this useful is practice? I tend to think that handlers don't care where they are mounted. You just "plug" them where you want to.

I feel like both are useful but I don't really have any specific use case in mind. At least to me it would be surprising if route("/api/*", svc) ate the prefix.

[3olkin]

However, I think the wildcards for paths will be implemented anyway, so it might be worth to remove nest in favor of them for prefix match.

Lot of frameworks separate concept of defining endpoints and group of routes.
Examples:

1. gin:

	grp1 := r.Group("/grp")
	{
		grp1.GET("/route1", Controller1)
		grp1.GET("/route2", Controller2)
	}

2. rails:

scope '/admin' do
  resources :articles, :comments
end

3. actix:

App::new().service(
            web::scope("/api")
                .service(...)
                .service(...),
        )

So I find that route("/api/*", svc) would be less intuitive than nest for grouping routes for someone who already have some experience. In addition, idk any use case for regex in routes. Some specific validations like

route("/greet/<r[1-9][0-9]{3}:id>", handler); # match into id only ints between 1000 and 9999 as mentioned in #109
can be done in middleware attached to it route.

[dbofmmbt]

@z4rx thanks for bringing the examples. I think the difference here in Axum is that it's reasonably easy to try to mix these concepts because of the use of Service everywhere (which is desirable).

Let me make some observations about the examples to illustrate that:

• gin: the methods which define endpoints (e.g. grp1.GET) take a HandlerFunc as the second argument. You couldn't pass a "Group" instance by mistake.

• Rails: I think it may be possible to do the same mistake related on this issue with it using some lower level API, but Rails's defaults are really strong. Rust devs simply won't have problems with it because the concepts are easy to distinguish in the API. (Convention can be really powerful 😄).

• Actix: same as gin, the types won't allow you to reach it.

Anyway, to improve the current situation without touching in the API, maybe we could have a way to generate routing information (like rails routes) to ease debugging. Another thing which could help is to try to improve documentation.

[davidpdrsn]

Anyway, to improve the current situation without touching in the API, maybe we could have a way to generate routing information (like rails routes) to ease debugging. Another thing which could help is to try to improve documentation.

I consider that part of #50