-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 routing for opaque nested services #842
Conversation
let path = if path == "/" { | ||
format!("/*{}", NEST_TAIL_PARAM) | ||
let path = if path.ends_with('/') { | ||
format!("{}*{}", path, NEST_TAIL_PARAM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we would add a double slash to nest("/foo/", _)
.
@@ -183,7 +199,7 @@ mod tests { | |||
single_segment_root_prefix, | |||
uri = "/a", | |||
prefix = "/", | |||
expected = None, | |||
expected = Some("/a"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The outcome of this is the same. Not matching the prefix or matching and removing nothing from the path is equivalent.
uri = "/a/a", | ||
prefix = "/a/", | ||
expected = Some("/a"), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Case that wasn't handled properly before.
Co-authored-by: Jonas Platte <jplatte+git@posteo.de>
Fixes #830
This brings nesting of opaque services inline with nesting
Router
s, except for one case which is explained in a test. Also added a bunch more comments to the prefix stripping so I'll still be able to understanding it two days from now 😅