From 60e7e52276ee60d46336563a91a8ef72be2ad699 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?LIU=20An=20=28=E5=8A=89=E5=AE=89=29?= <liuan@sgcc.com.cn>
Date: Fri, 25 Sep 2020 19:50:59 +0800
Subject: [PATCH] Add TrailingSlash::MergeOnly behavior (#1695)

Co-authored-by: Rob Ede <robjtede@icloud.com>
---
 CHANGES.md                  |  3 +++
 MIGRATION.md                |  2 +-
 src/middleware/normalize.rs | 40 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 15d3c81ce..608c237b2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -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
diff --git a/MIGRATION.md b/MIGRATION.md
index 98b22ae4e..5c4650194 100644
--- a/MIGRATION.md
+++ b/MIGRATION.md
@@ -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`.
 
diff --git a/src/middleware/normalize.rs b/src/middleware/normalize.rs
index 8452c9083..e0ecd90dc 100644
--- a/src/middleware/normalize.rs
+++ b/src/middleware/normalize.rs
@@ -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,
 }
@@ -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};
@@ -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(),
         };
 
@@ -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| {