diff options
| -rw-r--r-- | Cargo.lock | 1 | ||||
| -rw-r--r-- | Cargo.toml | 1 | ||||
| -rw-r--r-- | src/listing.rs | 26 | ||||
| -rw-r--r-- | src/renderer.rs | 22 | ||||
| -rw-r--r-- | tests/navigation.rs | 72 |
5 files changed, 73 insertions, 49 deletions
@@ -1457,6 +1457,7 @@ dependencies = [ "strum 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "strum_macros 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "tar 0.4.26 (registry+https://github.com/rust-lang/crates.io-index)", + "url 1.7.2 (registry+https://github.com/rust-lang/crates.io-index)", "yansi 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -56,3 +56,4 @@ select = "0.4" port_check = "0.1" rstest = "0.3" pretty_assertions = "0.6" +url = "1.7" diff --git a/src/listing.rs b/src/listing.rs index 4a69108..c5ee59e 100644 --- a/src/listing.rs +++ b/src/listing.rs @@ -135,6 +135,19 @@ pub fn directory_listing<S>( upload_route: String, ) -> Result<HttpResponse, io::Error> { let serve_path = req.path(); + + // In case the current path is a directory, we want to make sure that the current URL ends + // on a slash ("/"). + if !serve_path.ends_with('/') { + let query = match req.query_string() { + "" => String::new(), + _ => format!("?{}", req.query_string()), + }; + return Ok(HttpResponse::MovedPermanenty() + .header("Location", format!("{}/{}", serve_path, query)) + .body("301")); + } + let base = Path::new(serve_path); let random_route = format!("/{}", random_route.unwrap_or_default()); let is_root = base.parent().is_none() || req.path() == random_route; @@ -284,19 +297,6 @@ pub fn directory_listing<S>( .chunked() .body(Body::Streaming(Box::new(rx)))) } else { - // Redirect to directory - if !renderer::has_trailing(&serve_path) { - let query = match req.query_string() { - "" => String::new(), - _ => format!("?{}", req.query_string()) - }; - return Ok( - HttpResponse::MovedPermanenty() - .header("Location", format!("{}/{}", serve_path, query)) - .body("301") - ); - } - Ok(HttpResponse::Ok() .content_type("text/html; charset=utf-8") .body( diff --git a/src/renderer.rs b/src/renderer.rs index af8f5d7..cbad557 100644 --- a/src/renderer.rs +++ b/src/renderer.rs @@ -215,19 +215,11 @@ fn archive_button( } } -// Is there a trailing "/" -pub fn has_trailing(s: &str) -> bool { - match s.chars().last() { - Some(d) => d == '/', - None => false - } -} - -// Add trailing "/" if conditions permit -fn add_trailing(link: &str) -> String { - if has_trailing(&link) { +/// Ensure that there's always a trailing slash behind the `link`. +fn make_link_with_trailing_slash(link: &str) -> String { + if link.ends_with('/') { link.to_string() - }else { + } else { format!("{}/", link) } } @@ -244,7 +236,7 @@ fn parametrized_link( if let Some(order) = sort_order { let parametrized_link = format!( "{}?sort={}&order={}", - add_trailing(&link), + make_link_with_trailing_slash(&link), method, order ); @@ -260,12 +252,12 @@ fn parametrized_link( if color_scheme != default_color_scheme { return format!( "{}?theme={}", - add_trailing(&link), + make_link_with_trailing_slash(&link), color_scheme.to_slug() ); } - add_trailing(&link) + make_link_with_trailing_slash(&link) } /// Partial: table header link diff --git a/tests/navigation.rs b/tests/navigation.rs index d847f3e..a78653e 100644 --- a/tests/navigation.rs +++ b/tests/navigation.rs @@ -3,13 +3,36 @@ mod utils; use assert_cmd::prelude::*; use assert_fs::fixture::TempDir; -use fixtures::{port, tmpdir, Error, DIRECTORIES, DEEPLY_NESTED_FILE}; +use fixtures::{port, tmpdir, Error, DEEPLY_NESTED_FILE, DIRECTORIES}; use rstest::rstest; use select::document::Document; use std::process::{Command, Stdio}; use std::thread::sleep; use std::time::Duration; use utils::get_link_from_text; +use url::Url; + +#[rstest] +/// The index directory gets a trailing slash. +fn index_gets_trailing_slash(tmpdir: TempDir, port: u16) -> Result<(), Error> { + let mut child = Command::cargo_bin("miniserve")? + .arg("-p") + .arg(port.to_string()) + .arg(tmpdir.path()) + .stdout(Stdio::null()) + .spawn()?; + + sleep(Duration::from_secs(1)); + + let base_url = Url::parse(&format!("http://localhost:{}", port))?; + let expected_url = format!("{}", base_url); + let resp = reqwest::get(base_url.as_str())?; + assert_eq!(resp.url().as_str(), expected_url); + + child.kill()?; + + Ok(()) +} #[rstest] /// We can navigate into directories and back using shown links. @@ -23,20 +46,20 @@ fn can_navigate_into_dirs_and_back(tmpdir: TempDir, port: u16) -> Result<(), Err sleep(Duration::from_secs(1)); - let original_location = format!("http://localhost:{}/", port); - let body = reqwest::get(&original_location)?.error_for_status()?; - let parsed = Document::from_read(body)?; + let base_url = Url::parse(&format!("http://localhost:{}/", port))?; + let initial_body = reqwest::get(base_url.as_str())?.error_for_status()?; + let initial_parsed = Document::from_read(initial_body)?; for &directory in DIRECTORIES { - let dir_elem = get_link_from_text(&parsed, &directory).expect("Dir should have been here."); - let dir_body = - reqwest::get(format!("http://localhost:{}/{}", port, dir_elem).as_str())?.error_for_status()?; - let dir_parsed = Document::from_read(dir_body)?; - let back_link = get_link_from_text(&dir_parsed, "Parent directory").expect("Back link should be there."); - let back_location = format!("http://localhost:{}{}", port, back_link); + let dir_elem = get_link_from_text(&initial_parsed, &directory).expect("Dir not found."); + let body = reqwest::get(&format!("{}{}", base_url, dir_elem))?.error_for_status()?; + let parsed = Document::from_read(body)?; + let back_link = + get_link_from_text(&parsed, "Parent directory").expect("Back link not found."); + let resp = reqwest::get(&format!("{}{}", base_url, back_link))?; // Now check that we can actually get back to the original location we came from using the // link. - assert_eq!(back_location, original_location); + assert_eq!(resp.url(), &base_url); } child.kill()?; @@ -59,30 +82,37 @@ fn can_navigate_deep_into_dirs_and_back(tmpdir: TempDir, port: u16) -> Result<() // Create a vector of directory names. We don't need to fetch the file and so we'll // remove that part. let dir_names = { - let mut comps = DEEPLY_NESTED_FILE.split("/").map(|d| format!("{}/", d)).collect::<Vec<String>>(); + let mut comps = DEEPLY_NESTED_FILE + .split("/") + .map(|d| format!("{}/", d)) + .collect::<Vec<String>>(); comps.pop(); comps }; - let base_url = format!("http://localhost:{}", port); + let base_url = Url::parse(&format!("http://localhost:{}/", port))?; // First we'll go forwards through the directory tree and then we'll go backwards. // In the end, we'll have to end up where we came from. let mut next_url = base_url.clone(); for dir_name in dir_names.iter() { - let body = reqwest::get(&next_url)?.error_for_status()?; + let resp = reqwest::get(next_url.as_str())?; + let body = resp.error_for_status()?; let parsed = Document::from_read(body)?; - let dir_elem = get_link_from_text(&parsed, &dir_name).expect("Dir should have been here."); - next_url = format!("{}{}", base_url, dir_elem); + let dir_elem = get_link_from_text(&parsed, &dir_name).expect("Dir not found."); + next_url = next_url.join(&dir_elem)?; } + assert_ne!(base_url, next_url); // Now try to get out the tree again using links only. - let start_url = format!("{}/", base_url); - while next_url != start_url { - let body = reqwest::get(&next_url)?.error_for_status()?; + while next_url != base_url { + let resp = reqwest::get(next_url.as_str())?; + let body = resp.error_for_status()?; let parsed = Document::from_read(body)?; - let dir_elem = get_link_from_text(&parsed, "Parent directory").expect("Back link not found."); - next_url = format!("{}{}", base_url, dir_elem); + let dir_elem = + get_link_from_text(&parsed, "Parent directory").expect("Back link not found."); + next_url = next_url.join(&dir_elem)?; } + assert_eq!(base_url, next_url); child.kill()?; |