Fix tests

5 files changed, 73 insertions, 49 deletions

diff --git a/Cargo.lock b/Cargo.lock
index cd68f79..b941c4b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -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)",
 ]
 
diff --git a/Cargo.toml b/Cargo.toml
index 3010b07..facbecc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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()?;