Repository: svenstaro/miniserve
Pull request #849 (2022-07-20 09:49:00)

Minor refactoring
Get rid of some ugly code.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 4d94c59..d4dd21f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -464,9 +464,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"
 
 [[package]]
 name = "bytes"
-version = "1.2.0"
+version = "1.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f0b3de4a0c5e67e16066a0715723abd91edc2f9001d09c46e1dca929351e130e"
+checksum = "c4872d67bab6358e59559027aa3b9157c53d9358c51423c17554809a8858e0f8"
 
 [[package]]
 name = "bytesize"
@@ -543,9 +543,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "3.2.14"
+version = "3.2.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "54635806b078b7925d6e36810b1755f2a4b5b4d57560432c1ecf60bcbe10602b"
+checksum = "ab8b79fe3946ceb4a0b1c080b4018992b8d27e9ff363644c1c9b6387c854614d"
 dependencies = [
  "atty",
  "bitflags",
@@ -565,7 +565,7 @@ version = "3.2.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "ead064480dfc4880a10764488415a97fdd36a4cf1bb022d372f02e8faf8386e1"
 dependencies = [
- "clap 3.2.14",
+ "clap 3.2.12",
 ]
 
 [[package]]
@@ -596,7 +596,7 @@ version = "0.1.10"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "937efe4451c32be4ccb3e19549cbb1ea5cb85197f39b87a9662ca1d7b1765d1d"
 dependencies = [
- "clap 3.2.14",
+ "clap 3.2.12",
  "roff",
 ]
 
@@ -1411,7 +1411,7 @@ dependencies = [
  "bytesize",
  "chrono",
  "chrono-humanize",
- "clap 3.2.14",
+ "clap 3.2.12",
  "clap_complete",
  "clap_mangen",
  "futures",
@@ -2135,18 +2135,18 @@ checksum = "a2333e6df6d6598f2b1974829f853c2b4c5f4a6e503c10af918081aa6f8564e1"
 
 [[package]]
 name = "serde"
-version = "1.0.140"
+version = "1.0.139"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fc855a42c7967b7c369eb5860f7164ef1f6f81c20c7cc1141f2a604e18723b03"
+checksum = "0171ebb889e45aa68b44aee0859b3eede84c6f5f5c228e6f140c0b2a0a46cad6"
 dependencies = [
  "serde_derive",
 ]
 
 [[package]]
 name = "serde_derive"
-version = "1.0.140"
+version = "1.0.139"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6f2122636b9fe3b81f1cb25099fcf2d3f542cdb1d45940d56c713158884a05da"
+checksum = "dc1d3230c1de7932af58ad8ffbe1d784bd55efd5a9d84ac24f69c72d83543dfb"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -2226,12 +2226,9 @@ checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de"
 
 [[package]]
 name = "slab"
-version = "0.4.7"
+version = "0.4.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4614a76b2a8be0058caa9dbbaf66d988527d86d003c11a94fbd335d7661edcef"
-dependencies = [
- "autocfg",
-]
+checksum = "eb703cfe953bccee95685111adeedb76fabe4e97549a58d16f03ea7b9367bb32"
 
 [[package]]
 name = "smallvec"
diff --git a/src/errors.rs b/src/errors.rs
index 22351a3..b2ed459 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -103,7 +103,7 @@ impl ResponseError for ContextualError {
             ));
         }
 
-        resp.content_type("text/plain; charset=utf-8")
+        resp.content_type(mime::TEXT_PLAIN_UTF_8)
             .body(self.to_string())
     }
 }
@@ -125,7 +125,7 @@ where
 
         if (res.status().is_client_error() || res.status().is_server_error())
             && res.headers().get(header::CONTENT_TYPE).map(AsRef::as_ref)
-                == Some(b"text/plain; charset=utf-8")
+                == Some(mime::TEXT_PLAIN_UTF_8.essence_str().as_bytes())
         {
             let req = res.request().clone();
             Ok(res.map_body(|head, body| map_error_page(&req, head, body)))
@@ -155,7 +155,7 @@ fn map_error_page(req: &HttpRequest, head: &mut ResponseHead, body: BoxBody) ->
 
     head.headers.insert(
         header::CONTENT_TYPE,
-        header::HeaderValue::from_static("text/html; charset=utf-8"),
+        mime::TEXT_HTML_UTF_8.essence_str().try_into().unwrap(),
     );
 
     BoxBody::new(render_error(error_msg, head.status, conf, return_address).into_string())
diff --git a/src/listing.rs b/src/listing.rs
index 887fa8b..25f50fa 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -323,7 +323,7 @@ pub fn directory_listing(
             return Ok(ServiceResponse::new(
                 req.clone(),
                 HttpResponse::Forbidden()
-                    .content_type("text/plain; charset=utf-8")
+                    .content_type(mime::TEXT_PLAIN_UTF_8)
                     .body("Archive creation is disabled."),
             ));
         }
@@ -369,20 +369,18 @@ pub fn directory_listing(
     } else {
         Ok(ServiceResponse::new(
             req.clone(),
-            HttpResponse::Ok()
-                .content_type("text/html; charset=utf-8")
-                .body(
-                    renderer::page(
-                        entries,
-                        is_root,
-                        query_params,
-                        breadcrumbs,
-                        &encoded_dir,
-                        conf,
-                        current_user,
-                    )
-                    .into_string(),
-                ),
+            HttpResponse::Ok().content_type(mime::TEXT_HTML_UTF_8).body(
+                renderer::page(
+                    entries,
+                    is_root,
+                    query_params,
+                    breadcrumbs,
+                    &encoded_dir,
+                    conf,
+                    current_user,
+                )
+                .into_string(),
+            ),
         ))
     }
 }
diff --git a/src/main.rs b/src/main.rs
index 9d3f9ac..b46262d 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,7 +1,6 @@
 use std::io;
 use std::io::Write;
 use std::net::{IpAddr, SocketAddr, TcpListener};
-use std::path::{Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 
@@ -69,33 +68,19 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
         simplelog::LevelFilter::Warn
     };
 
-    if simplelog::TermLogger::init(
+    simplelog::TermLogger::init(
         log_level,
         simplelog::Config::default(),
         simplelog::TerminalMode::Mixed,
         simplelog::ColorChoice::Auto,
     )
-    .is_err()
-    {
-        simplelog::SimpleLogger::init(log_level, simplelog::Config::default())
-            .expect("Couldn't initialize logger")
-    }
+    .or_else(|_| simplelog::SimpleLogger::init(log_level, simplelog::Config::default()))
+    .expect("Couldn't initialize logger");
 
-    if miniserve_config.no_symlinks {
-        let is_symlink = miniserve_config
-            .path
-            .symlink_metadata()
-            .map_err(|e| {
-                ContextualError::IoError("Failed to retrieve symlink's metadata".to_string(), e)
-            })?
-            .file_type()
-            .is_symlink();
-
-        if is_symlink {
-            return Err(ContextualError::NoSymlinksOptionWithSymlinkServePath(
-                miniserve_config.path.to_string_lossy().to_string(),
-            ));
-        }
+    if miniserve_config.no_symlinks && miniserve_config.path.is_symlink() {
+        return Err(ContextualError::NoSymlinksOptionWithSymlinkServePath(
+            miniserve_config.path.to_string_lossy().to_string(),
+        ));
     }
 
     let inside_config = miniserve_config.clone();
@@ -104,7 +89,15 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
         ContextualError::IoError("Failed to resolve path to be served".to_string(), e)
     })?;
 
-    check_file_exists(&canon_path, &miniserve_config.index);
+    // warn if --index is specified but not found
+    if let Some(ref index) = miniserve_config.index {
+        if !canon_path.join(index).exists() {
+            warn!(
+                "The file '{}' provided for option --index could not be found.",
+                index.to_string_lossy(),
+            );
+        }
+    }
 
     let path_string = canon_path.to_string_lossy();
 
@@ -266,19 +259,6 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
         .map_err(|e| ContextualError::IoError("".to_owned(), e))
 }
 
-fn check_file_exists(canon_path: &Path, file_option: &Option<PathBuf>) {
-    if let Some(file_path) = file_option {
-        let file_path: &Path = file_path.as_ref();
-        let has_file: std::path::PathBuf = [canon_path, file_path].iter().collect();
-        if !has_file.exists() {
-            error!(
-                "The file '{}' provided for option --index could not be found.",
-                file_path.to_string_lossy(),
-            );
-        }
-    }
-}
-
 /// Allows us to set low-level socket options
 ///
 /// This mainly used to set `set_only_v6` socket option
@@ -307,67 +287,50 @@ fn configure_header(conf: &MiniserveConfig) -> middleware::DefaultHeaders {
 ///
 /// This is where we configure the app to serve an index file, the file listing, or a single file.
 fn configure_app(app: &mut web::ServiceConfig, conf: &MiniserveConfig) {
-    let files_service = || {
-        let files = actix_files::Files::new("", &conf.path);
-        let files = match &conf.index {
-            Some(index_file) => {
-                if conf.spa {
-                    files
-                        .index_file(index_file.to_string_lossy())
-                        .default_handler(
-                            NamedFile::open(&conf.path.join(index_file))
-                                .expect("Cant open SPA index file."),
-                        )
-                } else {
-                    files.index_file(index_file.to_string_lossy())
-                }
+    let dir_service = || {
+        let mut files = actix_files::Files::new("", &conf.path);
+
+        // Use specific index file if one was provided.
+        if let Some(ref index_file) = conf.index {
+            files = files.index_file(index_file.to_string_lossy());
+            // Handle SPA option.
+            //
+            // Note: --spa requires --index in clap.
+            if conf.spa {
+                files = files.default_handler(
+                    NamedFile::open(&conf.path.join(index_file))
+                        .expect("Cant open SPA index file."),
+                );
             }
-            None => {
-                if conf.spa {
-                    unreachable!("This can't be reached since we require --index to be provided if --spa is given via clap");
-                } else {
-                    files
-                }
-            }
-        };
-        let files = match conf.show_hidden {
-            true => files.use_hidden_files(),
-            false => files,
-        };
+        }
+
+        if conf.show_hidden {
+            files = files.use_hidden_files();
+        }
 
         let base_path = conf.path.clone();
-        let symlinks_may_be_followed = !conf.no_symlinks;
+        let no_symlinks = conf.no_symlinks;
         files
             .show_files_listing()
             .files_listing_renderer(listing::directory_listing)
             .prefer_utf8(true)
             .redirect_to_slash_directory()
             .path_filter(move |path, _| {
-                // Only allow symlinks to be followed in case conf.no_symlinks is false.
-                let path_is_symlink = base_path
-                    .join(path)
-                    .symlink_metadata()
-                    .map(|m| m.file_type().is_symlink())
-                    .unwrap_or(false);
-
-                if path_is_symlink {
-                    symlinks_may_be_followed
-                } else {
-                    true
-                }
+                // deny symlinks if conf.no_symlinks
+                !(no_symlinks && base_path.join(path).is_symlink())
             })
     };
 
-    if !conf.path.is_file() {
+    if conf.path.is_file() {
+        // Handle single files
+        app.service(web::resource(["", "/"]).route(web::to(listing::file_handler)));
+    } else {
         if conf.file_upload {
             // Allow file upload
             app.service(web::resource("/upload").route(web::post().to(file_upload::upload_file)));
         }
         // Handle directories
-        app.service(files_service());
-    } else {
-        // Handle single files
-        app.service(web::resource(["", "/"]).route(web::to(listing::file_handler)));
+        app.service(dir_service());
     }
 }
 


Test patch:
diff --git a/tests/serve_request.rs b/tests/serve_request.rs
index 18cc9e4..450a8f5 100644
--- a/tests/serve_request.rs
+++ b/tests/serve_request.rs
@@ -230,7 +230,7 @@ fn serves_requests_custom_index_notice(tmpdir: TempDir, port: u16) -> Result<(),
 
     child.kill()?;
     let output = child.wait_with_output().expect("Failed to read stdout");
-    let all_text = String::from_utf8(output.stderr);
+    let all_text = String::from_utf8(output.stdout);
 
     assert!(
         all_text?.contains("The file 'not.html' provided for option --index could not be found.")