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

Configurable trailing slash behaviour for NormalizePath #1639

Merged
merged 5 commits into from Aug 19, 2020
Merged
Changes from 1 commit
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
70 changes: 61 additions & 9 deletions src/middleware/normalize.rs
Expand Up @@ -10,20 +10,29 @@ use regex::Regex;
use crate::service::{ServiceRequest, ServiceResponse};
use crate::Error;

#[derive(Default, Clone, Copy)]
/// To be used when constructing `NormalizePath` to define it's behavior.
#[derive(Clone, Copy)]
pub enum TrailingSlash {
robjtede marked this conversation as resolved.
Show resolved Hide resolved
/// Always add a trailing slash to the end of the path.
robjtede marked this conversation as resolved.
Show resolved Hide resolved
Always,
/// Trim trailing slashes from the end of the path.
Trim,
}

#[derive(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()))
Expand All @@ -32,7 +41,20 @@ 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 Default for NormalizePath {
fn default() -> Self {
NormalizePath::new(TrailingSlash::Always)
}
}
robjtede marked this conversation as resolved.
Show resolved Hide resolved

impl<S, B> Transform<S> for NormalizePath
where
Expand All @@ -50,13 +72,18 @@ where
ok(NormalizePathNormalization {
service,
merge_slash: Regex::new("//+").unwrap(),
trim_last_slash: match self.0 {
TrailingSlash::Trim => true,
TrailingSlash::Always => false,
},
})
}
}

pub struct NormalizePathNormalization<S> {
service: S,
merge_slash: Regex,
trim_last_slash: bool,
robjtede marked this conversation as resolved.
Show resolved Hide resolved
}

impl<S, B> Service for NormalizePathNormalization<S>
Expand All @@ -78,8 +105,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.trim_last_slash {
false => original_path.to_string() + "/",
true => original_path.trim_end_matches(|c| c == '/').to_string(),
robjtede marked this conversation as resolved.
Show resolved Hide resolved
};

// normalize multiple /'s to one /
let path = self.merge_slash.replace_all(&path, "/");
Expand Down Expand Up @@ -150,14 +180,36 @@ mod tests {
assert!(res4.status().is_success());
}

#[actix_rt::test]
async fn trim_trailing_slashes() {
robjtede marked this conversation as resolved.
Show resolved Hide resolved
let mut app = init_service(
App::new()
.wrap(NormalizePath(TrailingSlash::Trim))
.service(web::resource("/v1/something").to(HttpResponse::Ok)),
)
.await;

let req4 = TestRequest::with_uri("/v1/something////").to_request();
let res4 = call_service(&mut app, req4).await;
assert!(res4.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());

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| {
assert_eq!("/v1/something/", req.path());
ok(req.into_response(HttpResponse::Ok().finish()))
};

let mut normalize = NormalizePath
let mut normalize = NormalizePath::default()
.new_transform(srv.into_service())
.await
.unwrap();
Expand Down Expand Up @@ -188,7 +240,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();
Expand All @@ -207,7 +259,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();
Expand Down