Skip to content

ServeDir in nested route causes invalid redirects #1731

@yu-re-ka

Description

@yu-re-ka
  • 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

Activity

davidpdrsn

davidpdrsn commented on Feb 20, 2023

@davidpdrsn
Member

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.

added
E-hardCall for participation: Experience needed to fix: Hard / a lot
on Feb 20, 2023
jplatte

jplatte commented on Feb 21, 2023

@jplatte
Member

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

davidpdrsn commented on Feb 21, 2023

@davidpdrsn
Member

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

avdb13 commented on Apr 26, 2023

@avdb13
Contributor

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

davidpdrsn

davidpdrsn commented on Apr 26, 2023

@davidpdrsn
Member

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

gubsey commented on Jul 19, 2023

@gubsey

Have there been any updates on this?

davidpdrsn

davidpdrsn commented on Jul 20, 2023

@davidpdrsn
Member

No I don't think so.

added a commit that references this issue on Sep 17, 2023
c8b9f62
linked a pull request that will close this issue on Sep 17, 2023
georgmu

georgmu commented on Oct 6, 2023

@georgmu

My approach to solve this would be to have a ServeDir::with_mount_point() (see georgmu/tower-http@32512d3 - 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

davidpdrsn commented on Oct 6, 2023

@davidpdrsn
Member

I've been putting together a middleware in axum to fix this #2230. Not quite there yet but that's another possible solution.

austenadler

austenadler commented on Oct 7, 2023

@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

georgmu commented on Oct 8, 2023

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-axumC-bugCategory: This is a bug.E-hardCall for participation: Experience needed to fix: Hard / a lot

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @davidpdrsn@jplatte@georgmu@austenadler@avdb13

      Issue actions

        ServeDir in nested route causes invalid redirects · Issue #1731 · tokio-rs/axum