ServeDir in nested route causes invalid redirects

[yu-re-ka]

• [x] I have looked for existing issues (including closed) about this

Bug Report

Version

├── axum v0.5.13
│   ├── axum-core v0.2.7

│   ├── tower-http v0.3.4
└── tower-http v0.3.4 (*)

Platform

Linux yaya 6.1.7 #1-NixOS SMP PREEMPT_DYNAMIC Wed Jan 18 10:58:34 UTC 2023 x86_64 GNU/Linux

Description

https://yaya.yuka.dev/ is running a axum based webserver with a axum::Router. It has a nest route for /public that contains a ServeDir service with path /mnt/public/.

Now, in /mnt/public/, there exists an empty directory /mnt/public/foo. I request https://yaya.yuka.dev/public/foo And I get:

< HTTP/2 307 
< location: https://yaya.yuka.dev/foo/

while I would have expected a redirect to https://yaya.yuka.dev/public/foo/

This redirect is created here in ServeDir, and it uses the stripped path instead of the original uri to determine the redirect location.

Unsure how to solve this, but some ideas:

• maybe the StripPrefix middleware and OriginalUri could be moved to tower_http and then tower_http can try to use the OriginalUri first and then fall back to the req.uri().

• Or the nested route service could attempt to rewrite relative redirects, but that seems like it would not generally work since applications behind a reverse proxy could also have other ways of redirecting

• Or ServeDir could be configured with a static prefix that is added to the redirects

[davidpdrsn]

maybe the StripPrefix middleware and OriginalUri could be moved to tower_http and then tower_http can try to use the OriginalUri first and then fall back to the req.uri().

That seems like the only viable solution to me. It just feels wrong to move axum specific stuff into tower-http to solve an, arguably, axum bug. StripPrefix also supports handling URL params for things like .nest("/:id", _). tower-http doesn't have anything like that currently. So I'd like to avoid moving things if at all possible. Will require some more thinking.

[jplatte]

Yeah, this is definitely hard. We couldn't turn OriginalUri and the http::Request path modifications on its head either (putting the modified, rather than original path in an extension instead), because then ServeDir won't be able to find its files.

Maybe the best solution to be had is ServeDir getting a new knob where you can tell it that it's mounted at some non-root path?

[davidpdrsn]

Maybe the best solution to be had is ServeDir getting a new knob where you can tell it that it's mounted at some non-root path?

Yeah that might be the simplest solution. It would be great if it "just worked" but not sure how.

[avdb13]

Was SpaRouter removed from axum-extra? OP might have been able to use that in the meanwhile as a temporary solution.

[davidpdrsn]

SpaRouter was a thin wrapper around ServeDir so that would have been the same.

It was removed because ServeDir had gotten easier to use so SpaRouter didn't bring much value.

[gubsey]

Have there been any updates on this?

[davidpdrsn]

No I don't think so.

[georgmu]

My approach to solve this would be to have a ServeDir::with_mount_point() (see https://github.com/georgmu/tower-http/commit/32512d36589a2cdd858ef6d7b563ee56f2920bc5 - it is still in testing and not yet in form to create a pull request).

This is still missing some mechanism to instantiate it without repeating the nested path in axum or reinsantiating ServeDir with every request...

[davidpdrsn]

I've been putting together a middleware in axum to fix this https://github.com/tokio-rs/axum/pull/2230. Not quite there yet but that's another possible solution.

[austenadler]

@georgmu I like this patch and I know it isn't complete, but if it helps:

• I have .nest_service("/abc", ServeDir::new(".").with_mount_point("/abc")) and it looks like axum isn't sending /abc, so this is always None
• Redirects need to prepend the path

Not sure what the path forward is, but here's my fork that fits my needs when using axum: https://github.com/georgmu/tower-http/compare/serve-dir-mount-point...austenadler:tower-http:serve-dir-mount-point?expand=1

[georgmu]

From my point of view, the problem is the design decision to alter the URI for nested routers. I think that with the presence of NestedPath, this altering is not necessary any more, but modifying this behavior would result in a breaking change and I don't know if this is wanted for other use cases