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

Add TrailingSlash::MergeOnly behavior #1695

Merged
merged 3 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Changes

## Unreleased - 2020-xx-xx
### Changed
* Add `TrailingSlash::MergeOnly` behaviour to `NormalizePath`, which allow `NormalizePath`
to keep the trailing slash's existance as it is. [#1695]


## 3.0.2 - 2020-09-15
Expand Down
2 changes: 1 addition & 1 deletion MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@

* `middleware::NormalizePath` can now also be configured to trim trailing slashes instead of always keeping one.
It will need `middleware::normalize::TrailingSlash` when being constructed with `NormalizePath::new(...)`,
or for an easier migration you can replace `wrap(middleware::NormalizePath)` with `wrap(middleware::NormalizePath::default())`.
or for an easier migration you can replace `wrap(middleware::NormalizePath)` with `wrap(middleware::NormalizePath::new(TrailingSlash::MergeOnly))`.

* `HttpServer::maxconn` is renamed to the more expressive `HttpServer::max_connections`.

Expand Down
40 changes: 39 additions & 1 deletion src/middleware/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ pub enum TrailingSlash {
/// Always add a trailing slash to the end of the path.
/// This will require all routes to end in a trailing slash for them to be accessible.
Always,
/// Only merge any present multiple trailing slashes.
///
/// Note: This option provides the best compatibility with the v2 version of this middlware.
MergeOnly,
/// Trim trailing slashes from the end of the path.
Trim,
}
Expand All @@ -33,7 +37,8 @@ impl Default for TrailingSlash {
/// Performs following:
///
/// - Merges multiple slashes into one.
/// - Appends a trailing slash if one is not present, or removes one if present, depending on the supplied `TrailingSlash`.
/// - Appends a trailing slash if one is not present, removes one if present, or keeps trailing
/// slashes as-is, depending on the supplied `TrailingSlash` variant.
///
/// ```rust
/// use actix_web::{web, http, middleware, App, HttpResponse};
Expand Down Expand Up @@ -108,6 +113,7 @@ where
// Either adds a string to the end (duplicates will be removed anyways) or trims all slashes from the end
let path = match self.trailing_slash_behavior {
TrailingSlash::Always => original_path.to_string() + "/",
TrailingSlash::MergeOnly => original_path.to_string(),
TrailingSlash::Trim => original_path.trim_end_matches('/').to_string(),
};

Expand Down Expand Up @@ -237,6 +243,38 @@ mod tests {
assert!(res4.status().is_success());
}

#[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);
}
}

#[actix_rt::test]
async fn test_in_place_normalization() {
let srv = |req: ServiceRequest| {
Expand Down