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
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
5 changes: 5 additions & 0 deletions 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
Expand Down
4 changes: 4 additions & 0 deletions MIGRATION.md
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/middleware/condition.rs
Expand Up @@ -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<T> {
Expand Down
71 changes: 63 additions & 8 deletions src/middleware/normalize.rs
Expand Up @@ -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 {
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
/// 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()))
Expand All @@ -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<S, B> Transform<S> for NormalizePath
where
Expand All @@ -50,13 +74,15 @@ where
ok(NormalizePathNormalization {
service,
merge_slash: Regex::new("//+").unwrap(),
trailing_slash_behavior: self.0,
})
}
}

pub struct NormalizePathNormalization<S> {
service: S,
merge_slash: Regex,
trailing_slash_behavior: TrailingSlash,
}

impl<S, B> Service for NormalizePathNormalization<S>
Expand All @@ -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, "/");
Expand Down Expand Up @@ -150,14 +179,40 @@ 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 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| {
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 +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();
Expand All @@ -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();
Expand Down