Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nesting of opaque services that paths that contain params #841

Merged
merged 4 commits into from
Mar 8, 2022

Conversation

davidpdrsn
Copy link
Member

While working on #830 I discovered that nesting opaque services at paths that contain params was totally broken. Things like

let api_routes = Router::new().route("/:b", get(|| async {})).boxed_clone();
let app = Router::new().nest("/:a", api_routes);

We would try and strip the prefix as /:a but thats a placeholder that should match anything. It shouldn't be used as the literal prefix.

This required some changes to StripPrefix. I also added a bunch more tests and some property tests to get better coverage against panics.

@davidpdrsn
Copy link
Member Author

Honestly surprised no one ran into this bug before. I guess users are either nesting ServeDir or Router.

@davidpdrsn davidpdrsn removed the request for review from jplatte March 8, 2022 15:16
@davidpdrsn davidpdrsn marked this pull request as draft March 8, 2022 15:16
@davidpdrsn
Copy link
Member Author

I swear I had the tests passing locally but I must have fat fingered something. I'll get it fixed.

axum/src/routing/strip_prefix.rs Show resolved Hide resolved
@davidpdrsn davidpdrsn marked this pull request as ready for review March 8, 2022 20:08
@davidpdrsn davidpdrsn merged commit dfa2e3b into main Mar 8, 2022
@davidpdrsn davidpdrsn deleted the fix-nested-captures branch March 8, 2022 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants