From c3187bfe914990d925da3765a946160205f7b5d5 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 29 May 2020 17:46:56 -0700 Subject: [PATCH] Add 'Options::NormalizeDirs' to 'StaticFiles'. Closes #1198. Co-authored-by: Keith Wansbrough --- contrib/lib/src/serve.rs | 31 +++++++++++++++++--- contrib/lib/tests/static_files.rs | 48 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/contrib/lib/src/serve.rs b/contrib/lib/src/serve.rs index 8e1c12c6..da099343 100644 --- a/contrib/lib/src/serve.rs +++ b/contrib/lib/src/serve.rs @@ -17,9 +17,9 @@ use std::path::{PathBuf, Path}; use rocket::{Request, Data, Route}; -use rocket::http::{Method, uri::Segments}; +use rocket::http::{Method, uri::Segments, ext::IntoOwned}; use rocket::handler::{Handler, Outcome}; -use rocket::response::NamedFile; +use rocket::response::{NamedFile, Redirect}; /// A bitset representing configurable options for the [`StaticFiles`] handler. /// @@ -38,7 +38,8 @@ pub struct Options(u8); #[allow(non_upper_case_globals, non_snake_case)] impl Options { /// `Options` representing the empty set. No dotfiles or index pages are - /// rendered. This is different than the _default_, which enables `Index`. + /// rendered. This is different than the [`Options::default()`], which + /// enables `Index`. pub const None: Options = Options(0b0000); /// `Options` enabling responding to requests for a directory with the @@ -53,6 +54,16 @@ impl Options { /// directories beginning with `.`. This is _not_ enabled by default. pub const DotFiles: Options = Options(0b0010); + /// `Options` that normalizes directory requests by redirecting requests to + /// directory paths without a trailing slash to ones with a trailing slash. + /// + /// When enabled, the [`StaticFiles`] handler will respond to requests for a + /// directory without a trailing `/` with a permanent redirect (308) to the + /// same path with a trailing `/`. This ensures relative URLs within any + /// document served for that directory will be interpreted relative to that + /// directory, rather than its parent. This is _not_ enabled by default. + pub const NormalizeDirs: Options = Options(0b0100); + /// Returns `true` if `self` is a superset of `other`. In other words, /// returns `true` if all of the options in `other` are also in `self`. /// @@ -79,6 +90,7 @@ impl Options { } } +/// The default set of options: `Options::Index`. impl Default for Options { fn default() -> Self { Options::Index @@ -263,7 +275,10 @@ impl StaticFiles { impl Into> for StaticFiles { fn into(self) -> Vec { let non_index = Route::ranked(self.rank, Method::Get, "/", self.clone()); - if self.options.contains(Options::Index) { + // `Index` requires routing the index for obvious reasons. + // `NormalizeDirs` requires routing the index so a `.mount("/foo")` with + // a request `/foo`, can be redirected to `/foo/`. + if self.options.contains(Options::Index) || self.options.contains(Options::NormalizeDirs) { let index = Route::ranked(self.rank, Method::Get, "/", self); vec![index, non_index] } else { @@ -275,6 +290,14 @@ impl Into> for StaticFiles { impl Handler for StaticFiles { fn handle<'r>(&self, req: &'r Request<'_>, data: Data) -> Outcome<'r> { fn handle_dir<'r>(opt: Options, r: &'r Request<'_>, d: Data, path: &Path) -> Outcome<'r> { + if opt.contains(Options::NormalizeDirs) && !r.uri().path().ends_with('/') { + let new_path = r.uri().map_path(|p| p.to_owned() + "/") + .expect("adding a trailing slash to a known good path results in a valid path") + .into_owned(); + + return Outcome::from_or_forward(r, d, Redirect::permanent(new_path)); + } + if !opt.contains(Options::Index) { return Outcome::forward(d); } diff --git a/contrib/lib/tests/static_files.rs b/contrib/lib/tests/static_files.rs index 18b4b1f6..b65acf09 100644 --- a/contrib/lib/tests/static_files.rs +++ b/contrib/lib/tests/static_files.rs @@ -24,6 +24,8 @@ mod static_tests { .mount("/dots", StaticFiles::new(&root, Options::DotFiles)) .mount("/index", StaticFiles::new(&root, Options::Index)) .mount("/both", StaticFiles::new(&root, Options::DotFiles | Options::Index)) + .mount("/redir", StaticFiles::new(&root, Options::NormalizeDirs)) + .mount("/redir_index", StaticFiles::new(&root, Options::NormalizeDirs | Options::Index)) } static REGULAR_FILES: &[&str] = &[ @@ -143,4 +145,50 @@ mod static_tests { assert_all(&client, "both", HIDDEN_FILES, true); assert_all(&client, "both", INDEXED_DIRECTORIES, true); } + + #[test] + fn test_redirection() { + let client = Client::new(rocket()).expect("valid rocket"); + + // Redirection only happens if enabled, and doesn't affect index behaviour. + let response = client.get("/no_index/inner").dispatch(); + assert_eq!(response.status(), Status::NotFound); + + let response = client.get("/index/inner").dispatch(); + assert_eq!(response.status(), Status::Ok); + + let response = client.get("/redir/inner").dispatch(); + assert_eq!(response.status(), Status::PermanentRedirect); + assert_eq!(response.headers().get("Location").next(), Some("/redir/inner/")); + + let response = client.get("/redir/inner?foo=bar").dispatch(); + assert_eq!(response.status(), Status::PermanentRedirect); + assert_eq!(response.headers().get("Location").next(), Some("/redir/inner/?foo=bar")); + + let response = client.get("/redir_index/inner").dispatch(); + assert_eq!(response.status(), Status::PermanentRedirect); + assert_eq!(response.headers().get("Location").next(), Some("/redir_index/inner/")); + + // Paths with trailing slash are unaffected. + let response = client.get("/redir/inner/").dispatch(); + assert_eq!(response.status(), Status::NotFound); + + let response = client.get("/redir_index/inner/").dispatch(); + assert_eq!(response.status(), Status::Ok); + + // Root of route is also redirected. + let response = client.get("/no_index").dispatch(); + assert_eq!(response.status(), Status::NotFound); + + let response = client.get("/index").dispatch(); + assert_eq!(response.status(), Status::Ok); + + let response = client.get("/redir").dispatch(); + assert_eq!(response.status(), Status::PermanentRedirect); + assert_eq!(response.headers().get("Location").next(), Some("/redir/")); + + let response = client.get("/redir_index").dispatch(); + assert_eq!(response.status(), Status::PermanentRedirect); + assert_eq!(response.headers().get("Location").next(), Some("/redir_index/")); + } }