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

Nested router does not work when path ends with slash #857

Open
sorcix opened this issue Oct 21, 2021 · 1 comment
Open

Nested router does not work when path ends with slash #857

sorcix opened this issue Oct 21, 2021 · 1 comment
Labels
bug Something isn't working

Comments

@sorcix
Copy link
Contributor

sorcix commented Oct 21, 2021

While using a nested router I got unexpected 404 responses when requesting /foo/ on a router nested in /foo.

Test:

#[async_std::test]
async fn nested_with_slash_path() -> tide::Result<()> {
    let mut outer = tide::new();
    let mut inner = tide::new();
    inner.at("/").get(|_| async { Ok("nested") });
    outer.at("/").get(|_| async { Ok("root") });
    outer.at("/foo").nest(inner);

    assert_eq!(outer.get("/foo").recv_string().await?, "nested");
    assert_eq!(outer.get("/foo/").recv_string().await?, "nested");
    assert_eq!(outer.get("/").recv_string().await?, "root");
    Ok(())
}

Failing result:

$ cargo test --features unstable nested_with_slash_path

running 1 test
test nested_with_slash_path ... FAILED

failures:

---- nested_with_slash_path stdout ----
thread 'nested_with_slash_path' panicked at 'assertion failed: `(left == right)`
  left: `""`,
 right: `"nested"`', tests/nested.rs:75:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    nested_with_slash_path

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s

Test in 3a285bb

@yoshuawuyts yoshuawuyts added the bug Something isn't working label Oct 25, 2021
@u5surf
Copy link
Contributor

u5surf commented Nov 16, 2021

Hi, I'm interested in this issues. and I've dived in it slightly.

At first, simply(although may be stupidly?), I just added path.trim_end_matches('/'); both router.route and server.at.
Of course this test which @sorcix had indicated has succeeded.

diff --git a/src/router.rs b/src/router.rs 
index e59e260..36ac513 100644
--- a/src/router.rs
+++ b/src/router.rs
@@ -46,6 +46,7 @@ impl<State: Clone + Send + Sync + 'static> Router<State> {
     }

     pub(crate) fn route(&self, path: &str, method: http_types::Method) -> Selection<'_, State> {
+        let path = path.trim_end_matches('/');
         if let Some(Match { handler, params }) = self
             .method_map
             .get(&method)
diff --git a/src/server.rs b/src/server.rs
index 1e6f8c1..c96f7c1 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -163,6 +163,7 @@ where
     /// match or not, which means that the order of adding resources has no
     /// effect.
     pub fn at<'a>(&'a mut self, path: &str) -> Route<'a, State> {
+        let path = path.trim_end_matches('/');
         let router = Arc::get_mut(&mut self.router)
             .expect("Registering routes is not possible after the Server has started");
         Route::new(router, path.to_owned())

But there are one conventional test which been failed.

     Running tests/wildcard.rs (target/debug/deps/wildcard-2df3af0ed73f684a)

running 11 tests
test invalid_segment_error ... ok
test ambiguous_router_wildcard_vs_star ... ok
test multi_wildcard ... ok
test nameless_internal_wildcard2 ... ok
test nameless_internal_wildcard ... ok
test not_found_error ... ok
test nameless_wildcard ... ok
test invalid_wildcard ... FAILED
test wild_last_segment ... ok
test wildcard ... ok
test wild_path ... ok

failures:

---- invalid_wildcard stdout ----
thread 'invalid_wildcard' panicked at 'assertion failed: `(left == right)`
  left: `Ok`,
 right: `NotFound`', tests/wildcard.rs:102:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    invalid_wildcard

test result: FAILED. 10 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

error: test failed, to rerun pass '--test wildcard'

./tests/wildcard.rs

#[async_std::test]
async fn invalid_wildcard() -> tide::Result<()> {
    let mut app = tide::new();
    app.at("/echo/*path/:one/").get(echo_path);
    assert_eq!(
        app.get("/echo/one/two").await?.status(),
        StatusCode::NotFound
    );
    Ok(())
}

Conventionally, server.at has not removed their last slash which can use to be a last blank segment.
Thus, /echo/*path/:one/ which is used in invalid_wildcard could be divided echo, *path, :one, .
/echo/one/two was recognized as echo, one, two, it returns StatusCode::NotFound cause it cannot be matched echo, *path, :one, .
Although, my stupidly proposed changes has removed last segment, in other words segments has been divided echo, *path, :one.
Unfortunately, it can be matched segments. and then it has returned Ok.

@yoshuawuyts
Could tell us whether last blank segment has necessary or expected expression?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants