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

NormalizePath in actix-web-3 is not compatible with that in actix-web-2 #1694

Closed
SuperHacker-liuan opened this issue Sep 18, 2020 · 0 comments · Fixed by #1695
Closed

NormalizePath in actix-web-3 is not compatible with that in actix-web-2 #1694

SuperHacker-liuan opened this issue Sep 18, 2020 · 0 comments · Fixed by #1695

Comments

@SuperHacker-liuan
Copy link
Contributor

Expected Behavior

In actix-web-2.0.0, NormalizePath will convert the paths as following:

  1. // => /
  2. //v1// => /v1/
  3. //v1//something => /v1/something

Current Behavior

When using TrailingSlash::Trim

  1. // => /
  2. //v1// => /v1
  3. //v1//something => /v1/something

When using TrailingSlash::Always

  1. // => /
  2. //v1// => /v1/
  3. //v1//something => /v1/something/

Possible Solution

Add an option called MergeOnly in TrailingSlash, which keeps the trailing slash's existance as it is.
This option should make NormalizePath compatible w/ actix-web-2.

Steps to Reproduce (for bugs)

#[actix_rt::test]
async fn keep_trailing_slash_unchange() {
    let mut app = init_service(
        App::new()
            .wrap(NormalizePath(TrailingSlash::MergeOnly))
            .service(web::resource("/").to(HttpResponse::Ok))
            .service(web::resource("/v1/something").to(HttpResponse::Ok))
            .service(web::resource("/v1/").to(HttpResponse::Ok)),
    )
    .await;

    let tests = vec![
        ("/", true), // root paths should still work
        ("/?query=test", true),
        ("///", true),
        ("/v1/something////", false),
        ("/v1/something/", false),
        ("//v1//something", true),
        ("/v1/", true),
        ("/v1", false),
        ("/v1////", true),
        ("//v1//", true),
        ("///v1", false),
    ];

    for (path, success) in tests {
        let req = TestRequest::with_uri(path).to_request();
        let res = call_service(&mut app, req).await;
        assert_eq!(res.status().is_success(), success);
    }
}

Context

I wanna migrate from actix-web-2.0 to 3.0, but failed to access some of my resource.

Your Environment

  • Rust Version: rustc 1.46.0 (04488afe3 2020-08-24)
  • Actix Web Version: 3.0.2
SuperHacker-liuan added a commit to SuperHacker-liuan/actix-web that referenced this issue Sep 18, 2020
… with actix-web 2.0

Behavior of `NormalizePath` in actix-web 2.0 would keep the trailing slash's existance as it is,
which is different from that in 3.0.

This patch add a new option called `MergeOnly` in `TrailingSlash`, which only merge the trailing
slashes into one if there exist. This option makes `NormalizePath` able to be compatible with
actix-web 2.0 and makes it eaiser for users who want to migrate from 2.0 to 3.0.

This will close actix#1694.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant