From 75d86a6bebe2829f8e4bc3f54945b08073fb1710 Mon Sep 17 00:00:00 2001 From: LJ <5854483+ljoonal@users.noreply.github.com> Date: Wed, 19 Aug 2020 11:21:52 +0000 Subject: [PATCH] Configurable trailing slash behaviour for NormalizePath (#1639) Co-authored-by: ljoonal --- CHANGES.md | 5 +++ MIGRATION.md | 4 +++ src/middleware/condition.rs | 2 +- src/middleware/normalize.rs | 71 ++++++++++++++++++++++++++++++++----- 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a44b04d4040..0a7b26d7dc5 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changes +## Unreleased +### Added +* `middleware::NormalizePath` now has configurable behaviour for either always having a trailing slash, + or as the new addition, always trimming trailing slashes. + ## 3.0.0-beta.3 - 2020-08-17 ### Changed * Update `rustls` to 0.18 diff --git a/MIGRATION.md b/MIGRATION.md index 7459b5b2d76..0e73b7d4767 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -32,6 +32,10 @@ } ``` +* `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())`. + ## 2.0.0 * `HttpServer::start()` renamed to `HttpServer::run()`. It also possible to diff --git a/src/middleware/condition.rs b/src/middleware/condition.rs index 7ff81437b23..ab1c69746f8 100644 --- a/src/middleware/condition.rs +++ b/src/middleware/condition.rs @@ -17,7 +17,7 @@ use futures_util::future::{ok, Either, FutureExt, LocalBoxFuture}; /// # fn main() { /// let enable_normalize = std::env::var("NORMALIZE_PATH") == Ok("true".into()); /// let app = App::new() -/// .wrap(Condition::new(enable_normalize, NormalizePath)); +/// .wrap(Condition::new(enable_normalize, NormalizePath::default())); /// # } /// ``` pub struct Condition { diff --git a/src/middleware/normalize.rs b/src/middleware/normalize.rs index a1021ed14be..1b689479691 100644 --- a/src/middleware/normalize.rs +++ b/src/middleware/normalize.rs @@ -10,20 +10,37 @@ use regex::Regex; use crate::service::{ServiceRequest, ServiceResponse}; use crate::Error; +/// To be used when constructing `NormalizePath` to define it's behavior. +#[non_exhaustive] +#[derive(Clone, Copy)] +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, + /// Trim trailing slashes from the end of the path. + Trim, +} + +impl Default for TrailingSlash { + fn default() -> Self { + TrailingSlash::Always + } +} + #[derive(Default, Clone, Copy)] /// `Middleware` to normalize request's URI in place /// /// Performs following: /// /// - Merges multiple slashes into one. -/// - Appends a trailing slash if one is not present. +/// - Appends a trailing slash if one is not present, or removes one if present, depending on the supplied `TrailingSlash`. /// /// ```rust /// use actix_web::{web, http, middleware, App, HttpResponse}; /// /// # fn main() { /// let app = App::new() -/// .wrap(middleware::NormalizePath) +/// .wrap(middleware::NormalizePath::default()) /// .service( /// web::resource("/test") /// .route(web::get().to(|| HttpResponse::Ok())) @@ -32,7 +49,14 @@ use crate::Error; /// # } /// ``` -pub struct NormalizePath; +pub struct NormalizePath(TrailingSlash); + +impl NormalizePath { + /// Create new `NormalizePath` middleware with the specified trailing slash style. + pub fn new(trailing_slash_style: TrailingSlash) -> Self { + NormalizePath(trailing_slash_style) + } +} impl Transform for NormalizePath where @@ -50,6 +74,7 @@ where ok(NormalizePathNormalization { service, merge_slash: Regex::new("//+").unwrap(), + trailing_slash_behavior: self.0, }) } } @@ -57,6 +82,7 @@ where pub struct NormalizePathNormalization { service: S, merge_slash: Regex, + trailing_slash_behavior: TrailingSlash, } impl Service for NormalizePathNormalization @@ -78,8 +104,11 @@ where let original_path = head.uri.path(); - // always add trailing slash, might be an extra one - let path = original_path.to_string() + "/"; + // 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::Trim => original_path.trim_end_matches('/').to_string(), + }; // normalize multiple /'s to one / let path = self.merge_slash.replace_all(&path, "/"); @@ -150,6 +179,32 @@ mod tests { assert!(res4.status().is_success()); } + #[actix_rt::test] + async fn trim_trailing_slashes() { + let mut app = init_service( + App::new() + .wrap(NormalizePath(TrailingSlash::Trim)) + .service(web::resource("/v1/something").to(HttpResponse::Ok)), + ) + .await; + + let req = TestRequest::with_uri("/v1/something////").to_request(); + let res = call_service(&mut app, req).await; + assert!(res.status().is_success()); + + let req2 = TestRequest::with_uri("/v1/something/").to_request(); + let res2 = call_service(&mut app, req2).await; + assert!(res2.status().is_success()); + + let req3 = TestRequest::with_uri("//v1//something//").to_request(); + let res3 = call_service(&mut app, req3).await; + assert!(res3.status().is_success()); + + let req4 = TestRequest::with_uri("//v1//something").to_request(); + let res4 = call_service(&mut app, req4).await; + assert!(res4.status().is_success()); + } + #[actix_rt::test] async fn test_in_place_normalization() { let srv = |req: ServiceRequest| { @@ -157,7 +212,7 @@ mod tests { ok(req.into_response(HttpResponse::Ok().finish())) }; - let mut normalize = NormalizePath + let mut normalize = NormalizePath::default() .new_transform(srv.into_service()) .await .unwrap(); @@ -188,7 +243,7 @@ mod tests { ok(req.into_response(HttpResponse::Ok().finish())) }; - let mut normalize = NormalizePath + let mut normalize = NormalizePath::default() .new_transform(srv.into_service()) .await .unwrap(); @@ -207,7 +262,7 @@ mod tests { ok(req.into_response(HttpResponse::Ok().finish())) }; - let mut normalize = NormalizePath + let mut normalize = NormalizePath::default() .new_transform(srv.into_service()) .await .unwrap();