Merge pull request #529 from aliemjay/src-refactor

[Refactor] Fix clippy::too_many_arguments and rework error page rendering

6 files changed, 235 insertions, 467 deletions

diff --git a/src/auth.rs b/src/auth.rs
index 7c77758..0d97f11 100644
--- a/src/auth.rs
+++ b/src/auth.rs
@@ -1,12 +1,10 @@
 use actix_web::dev::{Service, ServiceRequest, ServiceResponse};
-use actix_web::http::{header, StatusCode};
-use actix_web::{HttpRequest, HttpResponse};
+use actix_web::{HttpRequest, ResponseError};
 use futures::future::Either;
 use sha2::{Digest, Sha256, Sha512};
 use std::future::{ready, Future};
 
-use crate::errors::{self, ContextualError};
-use crate::renderer;
+use crate::errors::ContextualError;
 
 #[derive(Clone, Debug)]
 /// HTTP Basic authentication parameters
@@ -77,86 +75,38 @@ pub fn get_hash<T: Digest>(text: &str) -> Vec<u8> {
     hasher.finalize().to_vec()
 }
 
-/// When authentication succedes, return the request to be passed to downstream services.
-/// Otherwise, return an error response
-fn handle_auth(req: ServiceRequest) -> Result<ServiceRequest, ServiceResponse> {
-    let (req, pl) = req.into_parts();
+fn handle_auth(req: &HttpRequest) -> Result<(), ContextualError> {
     let required_auth = &req.app_data::<crate::MiniserveConfig>().unwrap().auth;
 
     if required_auth.is_empty() {
         // auth is disabled by configuration
-        return Ok(ServiceRequest::from_parts(req, pl));
-    } else if let Ok(cred) = BasicAuthParams::try_from_request(&req) {
-        if match_auth(cred, required_auth) {
-            return Ok(ServiceRequest::from_parts(req, pl));
-        }
+        return Ok(());
     }
 
-    // auth failed; render and return the error response
-    let resp = HttpResponse::Unauthorized()
-        .append_header((
-            header::WWW_AUTHENTICATE,
-            header::HeaderValue::from_static("Basic realm=\"miniserve\""),
-        ))
-        .body(build_unauthorized_response(
-            &req,
-            ContextualError::InvalidHttpCredentials,
-            true,
-            StatusCode::UNAUTHORIZED,
-        ));
-
-    Err(ServiceResponse::new(req, resp))
+    match BasicAuthParams::try_from_request(req) {
+        Ok(cred) => match match_auth(cred, required_auth) {
+            true => Ok(()),
+            false => Err(ContextualError::InvalidHttpCredentials),
+        },
+        Err(_) => Err(ContextualError::RequireHttpCredentials),
+    }
 }
 
 pub fn auth_middleware<S>(
-    req: ServiceRequest,
+    mut req: ServiceRequest,
     srv: &S,
 ) -> impl Future<Output = actix_web::Result<ServiceResponse>> + 'static
 where
     S: Service<ServiceRequest, Response = ServiceResponse, Error = actix_web::Error>,
     S::Future: 'static,
 {
-    match handle_auth(req) {
-        Ok(req) => Either::Left(srv.call(req)),
-        Err(resp) => Either::Right(ready(Ok(resp))),
-    }
-}
-
-/// Builds the unauthorized response body
-/// The reason why log_error_chain is optional is to handle cases where the auth pop-up appears and when the user clicks Cancel.
-/// In those case, we do not log the error to the terminal since it does not really matter.
-fn build_unauthorized_response(
-    req: &HttpRequest,
-    error: ContextualError,
-    log_error_chain: bool,
-    error_code: StatusCode,
-) -> String {
-    let state = req.app_data::<crate::MiniserveConfig>().unwrap();
-    let error = ContextualError::HttpAuthenticationError(Box::new(error));
-
-    if log_error_chain {
-        errors::log_error_chain(error.to_string());
+    match handle_auth(req.parts_mut().0) {
+        Ok(_) => Either::Left(srv.call(req)),
+        Err(err) => {
+            let resp = req.into_response(err.error_response());
+            Either::Right(ready(Ok(resp)))
+        }
     }
-    let return_path = match state.random_route {
-        Some(ref random_route) => format!("/{}", random_route),
-        None => "/".to_string(),
-    };
-
-    renderer::render_error(
-        &error.to_string(),
-        error_code,
-        &return_path,
-        None,
-        None,
-        false,
-        false,
-        &state.favicon_route,
-        &state.css_route,
-        &state.default_color_scheme,
-        &state.default_color_scheme_dark,
-        state.hide_version_footer,
-    )
-    .into_string()
 }
 
 #[rustfmt::skip]
diff --git a/src/errors.rs b/src/errors.rs
index f079657..25d0529 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -1,3 +1,11 @@
+use crate::{renderer::render_error, MiniserveConfig};
+use actix_web::{
+    body::AnyBody,
+    dev::{ResponseHead, Service, ServiceRequest, ServiceResponse},
+    http::{header, StatusCode},
+    HttpRequest, HttpResponse, ResponseError,
+};
+use futures::prelude::*;
 use thiserror::Error;
 
 #[derive(Debug, Error)]
@@ -50,9 +58,9 @@ pub enum ContextualError {
     #[error("{0}")]
     ArchiveCreationDetailError(String),
 
-    /// Might occur when the HTTP authentication fails
-    #[error("An error occured during HTTP authentication\ncaused by: {0}")]
-    HttpAuthenticationError(Box<ContextualError>),
+    /// Might occur when the HTTP credentials are not provided
+    #[error("Access requires HTTP authentication")]
+    RequireHttpCredentials,
 
     /// Might occur when the HTTP credentials are not correct
     #[error("Invalid credentials for HTTP authentication")]
@@ -76,6 +84,89 @@ Please set an explicit serve path like: `miniserve /my/path`")]
     NoSymlinksOptionWithSymlinkServePath(String),
 }
 
+impl ResponseError for ContextualError {
+    fn status_code(&self) -> StatusCode {
+        match self {
+            Self::ArchiveCreationError(_, err) => err.status_code(),
+            Self::RouteNotFoundError(_) => StatusCode::NOT_FOUND,
+            Self::InsufficientPermissionsError(_) => StatusCode::FORBIDDEN,
+            Self::InvalidHttpCredentials | Self::RequireHttpCredentials => StatusCode::UNAUTHORIZED,
+            Self::InvalidHttpRequestError(_) => StatusCode::BAD_REQUEST,
+            _ => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
+
+    fn error_response(&self) -> HttpResponse {
+        if let Self::RequireHttpCredentials = self {
+        } else {
+            log_error_chain(self.to_string());
+        }
+
+        let mut resp = HttpResponse::build(self.status_code());
+        if let Self::RequireHttpCredentials | Self::InvalidHttpCredentials = self {
+            resp.append_header((
+                header::WWW_AUTHENTICATE,
+                header::HeaderValue::from_static("Basic realm=\"miniserve\""),
+            ));
+        }
+
+        resp.content_type("text/plain; charset=utf-8")
+            .body(self.to_string())
+    }
+}
+
+/// Middleware to convert plain-text error responses to user-friendly web pages
+pub fn error_page_middleware<S>(
+    req: ServiceRequest,
+    srv: &S,
+) -> impl Future<Output = actix_web::Result<ServiceResponse>> + 'static
+where
+    S: Service<ServiceRequest, Response = ServiceResponse, Error = actix_web::Error>,
+    S::Future: 'static,
+{
+    let fut = srv.call(req);
+
+    async {
+        let res = fut.await?;
+
+        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")
+        {
+            let req = res.request().clone();
+            Ok(res.map_body(|head, body| map_error_page(&req, head, body)))
+        } else {
+            Ok(res)
+        }
+    }
+}
+
+fn map_error_page(req: &HttpRequest, head: &mut ResponseHead, body: AnyBody) -> AnyBody {
+    let error_msg = match &body {
+        AnyBody::Bytes(bytes) => match std::str::from_utf8(bytes) {
+            Ok(msg) => msg,
+            _ => return body,
+        },
+        _ => return body,
+    };
+
+    let conf = req.app_data::<MiniserveConfig>().unwrap();
+    let return_address = req
+        .headers()
+        .get(header::REFERER)
+        .and_then(|h| h.to_str().ok())
+        .unwrap_or("/");
+
+    head.headers.insert(
+        header::CONTENT_TYPE,
+        header::HeaderValue::from_static("text/html; charset=utf-8"),
+    );
+
+    render_error(error_msg, head.status, conf, return_address)
+        .into_string()
+        .into()
+}
+
 pub fn log_error_chain(description: String) {
     for cause in description.lines() {
         log::error!("{}", cause);
diff --git a/src/file_upload.rs b/src/file_upload.rs
index 6fa99ef..5f9738c 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -1,16 +1,12 @@
-use actix_web::{
-    http::{header, StatusCode},
-    HttpRequest, HttpResponse,
-};
+use actix_web::{http::header, HttpRequest, HttpResponse};
 use futures::TryStreamExt;
 use std::{
     io::Write,
     path::{Component, PathBuf},
 };
 
-use crate::errors::{self, ContextualError};
-use crate::listing::{self, SortingMethod, SortingOrder};
-use crate::renderer;
+use crate::errors::ContextualError;
+use crate::listing::{self};
 
 /// Create future to save file.
 async fn save_file(
@@ -76,17 +72,10 @@ async fn handle_multipart(
 /// server root directory. Any path which will go outside of this directory is considered
 /// invalid.
 /// This method returns future.
-#[allow(clippy::too_many_arguments)]
 pub async fn upload_file(
     req: HttpRequest,
     payload: actix_web::web::Payload,
-    uses_random_route: bool,
-    favicon_route: String,
-    css_route: String,
-    default_color_scheme: String,
-    default_color_scheme_dark: String,
-    hide_version_footer: bool,
-) -> Result<HttpResponse, actix_web::Error> {
+) -> Result<HttpResponse, ContextualError> {
     let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
     let return_path = if let Some(header) = req.headers().get(header::REFERER) {
         header.to_str().unwrap_or("/").to_owned()
@@ -95,138 +84,32 @@ pub async fn upload_file(
     };
 
     let query_params = listing::extract_query_parameters(&req);
-    let upload_path = match query_params.path.clone() {
-        Some(path) => match path.strip_prefix(Component::RootDir) {
-            Ok(stripped_path) => stripped_path.to_owned(),
-            Err(_) => path.clone(),
-        },
-        None => {
-            let err = ContextualError::InvalidHttpRequestError(
-                "Missing query parameter 'path'".to_string(),
-            );
-            return Ok(create_error_response(
-                &err.to_string(),
-                StatusCode::BAD_REQUEST,
-                &return_path,
-                query_params.sort,
-                query_params.order,
-                uses_random_route,
-                &favicon_route,
-                &css_route,
-                &default_color_scheme,
-                &default_color_scheme_dark,
-                hide_version_footer,
-            ));
-        }
-    };
+    let upload_path = query_params.path.as_ref().ok_or_else(|| {
+        ContextualError::InvalidHttpRequestError("Missing query parameter 'path'".to_string())
+    })?;
+    let upload_path = upload_path
+        .strip_prefix(Component::RootDir)
+        .unwrap_or(upload_path);
 
-    let app_root_dir = match conf.path.canonicalize() {
-        Ok(dir) => dir,
-        Err(e) => {
-            let err = ContextualError::IoError(
-                "Failed to resolve path served by miniserve".to_string(),
-                e,
-            );
-            return Ok(create_error_response(
-                &err.to_string(),
-                StatusCode::INTERNAL_SERVER_ERROR,
-                &return_path,
-                query_params.sort,
-                query_params.order,
-                uses_random_route,
-                &favicon_route,
-                &css_route,
-                &default_color_scheme,
-                &default_color_scheme_dark,
-                hide_version_footer,
-            ));
-        }
-    };
+    let app_root_dir = conf.path.canonicalize().map_err(|e| {
+        ContextualError::IoError("Failed to resolve path served by miniserve".to_string(), e)
+    })?;
 
     // If the target path is under the app root directory, save the file.
-    let target_dir = match &app_root_dir.join(upload_path).canonicalize() {
-        Ok(path) if path.starts_with(&app_root_dir) => path.clone(),
-        _ => {
-            let err = ContextualError::InvalidHttpRequestError(
-                "Invalid value for 'path' parameter".to_string(),
-            );
-            return Ok(create_error_response(
-                &err.to_string(),
-                StatusCode::BAD_REQUEST,
-                &return_path,
-                query_params.sort,
-                query_params.order,
-                uses_random_route,
-                &favicon_route,
-                &css_route,
-                &default_color_scheme,
-                &default_color_scheme_dark,
-                hide_version_footer,
-            ));
-        }
-    };
-    let overwrite_files = conf.overwrite_files;
-    let default_color_scheme = conf.default_color_scheme.clone();
-    let default_color_scheme_dark = conf.default_color_scheme_dark.clone();
+    let target_dir = match app_root_dir.join(upload_path).canonicalize() {
+        Ok(path) if path.starts_with(&app_root_dir) => Ok(path),
+        _ => Err(ContextualError::InvalidHttpRequestError(
+            "Invalid value for 'path' parameter".to_string(),
+        )),
+    }?;
 
-    match actix_multipart::Multipart::new(req.headers(), payload)
+    actix_multipart::Multipart::new(req.headers(), payload)
         .map_err(ContextualError::MultipartError)
-        .and_then(move |field| handle_multipart(field, target_dir.clone(), overwrite_files))
+        .and_then(|field| handle_multipart(field, target_dir.clone(), conf.overwrite_files))
         .try_collect::<Vec<u64>>()
-        .await
-    {
-        Ok(_) => Ok(HttpResponse::SeeOther()
-            .append_header((header::LOCATION, return_path))
-            .finish()),
-        Err(e) => Ok(create_error_response(
-            &e.to_string(),
-            StatusCode::INTERNAL_SERVER_ERROR,
-            &return_path,
-            query_params.sort,
-            query_params.order,
-            uses_random_route,
-            &favicon_route,
-            &css_route,
-            &default_color_scheme,
-            &default_color_scheme_dark,
-            hide_version_footer,
-        )),
-    }
-}
+        .await?;
 
-/// Convenience method for creating response errors, if file upload fails.
-#[allow(clippy::too_many_arguments)]
-fn create_error_response(
-    description: &str,
-    error_code: StatusCode,
-    return_path: &str,
-    sorting_method: Option<SortingMethod>,
-    sorting_order: Option<SortingOrder>,
-    uses_random_route: bool,
-    favicon_route: &str,
-    css_route: &str,
-    default_color_scheme: &str,
-    default_color_scheme_dark: &str,
-    hide_version_footer: bool,
-) -> HttpResponse {
-    errors::log_error_chain(description.to_string());
-    HttpResponse::BadRequest()
-        .content_type("text/html; charset=utf-8")
-        .body(
-            renderer::render_error(
-                description,
-                error_code,
-                return_path,
-                sorting_method,
-                sorting_order,
-                true,
-                !uses_random_route,
-                favicon_route,
-                css_route,
-                default_color_scheme,
-                default_color_scheme_dark,
-                hide_version_footer,
-            )
-            .into_string(),
-        )
+    Ok(HttpResponse::SeeOther()
+        .append_header((header::LOCATION, return_path))
+        .finish())
 }
diff --git a/src/listing.rs b/src/listing.rs
index 8147113..b2730de 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -1,6 +1,5 @@
 use actix_web::body::Body;
 use actix_web::dev::ServiceResponse;
-use actix_web::http::StatusCode;
 use actix_web::web::Query;
 use actix_web::{HttpRequest, HttpResponse};
 use bytesize::ByteSize;
@@ -149,33 +148,16 @@ pub async fn file_handler(req: HttpRequest) -> actix_web::Result<actix_files::Na
 
 /// List a directory and renders a HTML file accordingly
 /// Adapted from https://docs.rs/actix-web/0.7.13/src/actix_web/fs.rs.html#564
-#[allow(clippy::too_many_arguments)]
 pub fn directory_listing(
     dir: &actix_files::Directory,
     req: &HttpRequest,
-    skip_symlinks: bool,
-    show_hidden: bool,
-    file_upload: bool,
-    random_route: Option<String>,
-    favicon_route: String,
-    css_route: String,
-    default_color_scheme: &str,
-    default_color_scheme_dark: &str,
-    show_qrcode: bool,
-    upload_route: String,
-    tar_enabled: bool,
-    tar_gz_enabled: bool,
-    zip_enabled: bool,
-    dirs_first: bool,
-    hide_version_footer: bool,
-    show_symlink_info: bool,
-    title: Option<String>,
 ) -> io::Result<ServiceResponse> {
     use actix_web::dev::BodyEncoding;
+    let conf = req.app_data::<crate::MiniserveConfig>().unwrap();
     let serve_path = req.path();
 
     let base = Path::new(serve_path);
-    let random_route_abs = format!("/{}", random_route.clone().unwrap_or_default());
+    let random_route_abs = format!("/{}", conf.random_route.clone().unwrap_or_default());
     let is_root = base.parent().is_none() || Path::new(&req.path()) == Path::new(&random_route_abs);
 
     let encoded_dir = match base.strip_prefix(random_route_abs) {
@@ -186,13 +168,18 @@ pub fn directory_listing(
     .to_string();
 
     let breadcrumbs = {
-        let title = title.unwrap_or_else(|| req.connection_info().host().into());
+        let title = conf
+            .title
+            .clone()
+            .unwrap_or_else(|| req.connection_info().host().into());
 
         let decoded = percent_decode_str(&encoded_dir).decode_utf8_lossy();
 
         let mut res: Vec<Breadcrumb> = Vec::new();
-        let mut link_accumulator =
-            format!("/{}", random_route.map(|r| r + "/").unwrap_or_default());
+        let mut link_accumulator = match &conf.random_route {
+            Some(random_route) => format!("/{}/", random_route),
+            None => "/".to_string(),
+        };
 
         let mut components = Path::new(&*decoded).components().peekable();
 
@@ -242,7 +229,7 @@ pub fn directory_listing(
     let mut entries: Vec<Entry> = Vec::new();
 
     for entry in dir.path.read_dir()? {
-        if dir.is_visible(&entry) || show_hidden {
+        if dir.is_visible(&entry) || conf.show_hidden {
             let entry = entry?;
             // show file url as relative to static path
             let file_name = entry.file_name().to_string_lossy().to_string();
@@ -253,11 +240,10 @@ pub fn directory_listing(
                 }
                 res => (false, res),
             };
-            let symlink_dest = if is_symlink && show_symlink_info {
-                Some(std::fs::read_link(entry.path())?)
-            } else {
-                None
-            };
+            let symlink_dest = (is_symlink && conf.show_symlink_info)
+                .then(|| entry.path())
+                .and_then(|path| std::fs::read_link(path).ok())
+                .map(|path| path.to_string_lossy().into_owned());
             let file_url = base
                 .join(&utf8_percent_encode(&file_name, PATH_SEGMENT).to_string())
                 .to_string_lossy()
@@ -265,7 +251,7 @@ pub fn directory_listing(
 
             // if file is a directory, add '/' to the end of the name
             if let Ok(metadata) = metadata {
-                if skip_symlinks && is_symlink {
+                if conf.no_symlinks && is_symlink {
                     continue;
                 }
                 let last_modification_date = match metadata.modified() {
@@ -280,7 +266,7 @@ pub fn directory_listing(
                         file_url,
                         None,
                         last_modification_date,
-                        symlink_dest.and_then(|s| s.into_os_string().into_string().ok()),
+                        symlink_dest,
                     ));
                 } else if metadata.is_file() {
                     entries.push(Entry::new(
@@ -289,7 +275,7 @@ pub fn directory_listing(
                         file_url,
                         Some(ByteSize::b(metadata.len())),
                         last_modification_date,
-                        symlink_dest.and_then(|s| s.into_os_string().into_string().ok()),
+                        symlink_dest,
                     ));
                 }
             } else {
@@ -323,33 +309,17 @@ pub fn directory_listing(
     }
 
     // List directories first
-    if dirs_first {
+    if conf.dirs_first {
         entries.sort_by_key(|e| !e.is_dir());
     }
 
     if let Some(archive_method) = query_params.download {
-        if !archive_method.is_enabled(tar_enabled, tar_gz_enabled, zip_enabled) {
+        if !archive_method.is_enabled(conf.tar_enabled, conf.tar_gz_enabled, conf.zip_enabled) {
             return Ok(ServiceResponse::new(
                 req.clone(),
                 HttpResponse::Forbidden()
-                    .content_type("text/html; charset=utf-8")
-                    .body(
-                        renderer::render_error(
-                            "Archive creation is disabled.",
-                            StatusCode::FORBIDDEN,
-                            "/",
-                            None,
-                            None,
-                            false,
-                            false,
-                            &favicon_route,
-                            &css_route,
-                            default_color_scheme,
-                            default_color_scheme_dark,
-                            hide_version_footer,
-                        )
-                        .into_string(),
-                    ),
+                    .content_type("text/plain; charset=utf-8")
+                    .body("Archive creation is disabled."),
             ));
         }
         log::info!(
@@ -372,6 +342,7 @@ pub fn directory_listing(
 
         // Start the actual archive creation in a separate thread.
         let dir = dir.path.to_path_buf();
+        let skip_symlinks = conf.no_symlinks;
         std::thread::spawn(move || {
             if let Err(err) = archive_method.create_archive(dir, skip_symlinks, pipe) {
                 log::error!("Error during archive creation: {:?}", err);
@@ -399,21 +370,10 @@ pub fn directory_listing(
                     renderer::page(
                         entries,
                         is_root,
-                        query_params.sort,
-                        query_params.order,
-                        show_qrcode,
-                        file_upload,
-                        &upload_route,
-                        &favicon_route,
-                        &css_route,
-                        default_color_scheme,
-                        default_color_scheme_dark,
-                        &encoded_dir,
+                        query_params,
                         breadcrumbs,
-                        tar_enabled,
-                        tar_gz_enabled,
-                        zip_enabled,
-                        hide_version_footer,
+                        &encoded_dir,
+                        conf,
                     )
                     .into_string(),
                 ),
diff --git a/src/main.rs b/src/main.rs
index 5f68cd2..e5cc596 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -5,10 +5,7 @@ use std::thread;
 use std::time::Duration;
 
 use actix_web::web;
-use actix_web::{
-    http::{header::ContentType, StatusCode},
-    Responder,
-};
+use actix_web::{http::header::ContentType, Responder};
 use actix_web::{middleware, App, HttpRequest, HttpResponse};
 use anyhow::{bail, Result};
 use clap::{crate_version, Clap, IntoApp};
@@ -205,18 +202,22 @@ async fn run(miniserve_config: MiniserveConfig) -> Result<(), ContextualError> {
         App::new()
             .wrap(configure_header(&inside_config.clone()))
             .app_data(inside_config.clone())
-            // we should use `actix_web_httpauth::middleware::HttpAuthentication`
-            // but it is unfortuantrly broken
-            // see: https://github.com/actix/actix-extras/issues/127
-            // TODO replace this when fixed upstream
-            .wrap_fn(auth::auth_middleware)
+            .wrap_fn(errors::error_page_middleware)
             .wrap(middleware::Logger::default())
             .route(
                 &format!("/{}", inside_config.favicon_route),
                 web::get().to(favicon),
             )
             .route(&format!("/{}", inside_config.css_route), web::get().to(css))
-            .configure(|c| configure_app(c, &inside_config))
+            .service(
+                web::scope(inside_config.random_route.as_deref().unwrap_or(""))
+                    // we should use `actix_web_httpauth::middleware::HttpAuthentication`
+                    // but it is unfortuantrly broken
+                    // see: https://github.com/actix/actix-extras/issues/127
+                    // TODO replace this when fixed upstream
+                    .wrap_fn(auth::auth_middleware)
+                    .configure(|c| configure_app(c, &inside_config)),
+            )
             .default_service(web::get().to(error_404))
     });
 
@@ -289,145 +290,39 @@ fn configure_header(conf: &MiniserveConfig) -> middleware::DefaultHeaders {
 
 /// Configures the Actix application
 fn configure_app(app: &mut web::ServiceConfig, conf: &MiniserveConfig) {
-    let random_route = conf.random_route.clone().unwrap_or_default();
-    let uses_random_route = conf.random_route.clone().is_some();
-    let full_route = format!("/{}", random_route);
-
-    let upload_route = if let Some(random_route) = conf.random_route.clone() {
-        format!("/{}/upload", random_route)
-    } else {
-        "/upload".to_string()
-    };
-
-    let serve_path = {
-        let path = &conf.path;
-        let no_symlinks = conf.no_symlinks;
-        let show_hidden = conf.show_hidden;
-        let random_route = conf.random_route.clone();
-        let favicon_route = conf.favicon_route.clone();
-        let css_route = conf.css_route.clone();
-        let default_color_scheme = conf.default_color_scheme.clone();
-        let default_color_scheme_dark = conf.default_color_scheme_dark.clone();
-        let show_qrcode = conf.show_qrcode;
-        let file_upload = conf.file_upload;
-        let tar_enabled = conf.tar_enabled;
-        let tar_gz_enabled = conf.tar_gz_enabled;
-        let zip_enabled = conf.zip_enabled;
-        let dirs_first = conf.dirs_first;
-        let hide_version_footer = conf.hide_version_footer;
-        let cmd_enable_symlink_dest = conf.show_symlink_info;
-        let title = conf.title.clone();
-
-        if path.is_file() {
-            None
-        } else {
-            let u_r = upload_route.clone();
-
-            // build `Files` service using configuraion parameters
-            let files = actix_files::Files::new(&full_route, path);
-            let files = match &conf.index {
-                Some(index_file) => files.index_file(index_file.to_string_lossy()),
-                None => files,
-            };
-            let files = match show_hidden {
-                true => files.use_hidden_files(),
-                false => files,
-            };
-            let files = files
-                .show_files_listing()
-                .files_listing_renderer(move |dir, req| {
-                    listing::directory_listing(
-                        dir,
-                        req,
-                        no_symlinks,
-                        show_hidden,
-                        file_upload,
-                        random_route.clone(),
-                        favicon_route.clone(),
-                        css_route.clone(),
-                        &default_color_scheme,
-                        &default_color_scheme_dark,
-                        show_qrcode,
-                        u_r.clone(),
-                        tar_enabled,
-                        tar_gz_enabled,
-                        zip_enabled,
-                        dirs_first,
-                        hide_version_footer,
-                        cmd_enable_symlink_dest,
-                        title.clone(),
-                    )
-                })
-                .prefer_utf8(true)
-                .redirect_to_slash_directory()
-                .default_handler(web::to(error_404));
-            Some(files)
-        }
+    let files_service = || {
+        let files = actix_files::Files::new("", &conf.path);
+        let files = match &conf.index {
+            Some(index_file) => files.index_file(index_file.to_string_lossy()),
+            None => files,
+        };
+        let files = match conf.show_hidden {
+            true => files.use_hidden_files(),
+            false => files,
+        };
+        files
+            .show_files_listing()
+            .files_listing_renderer(listing::directory_listing)
+            .prefer_utf8(true)
+            .redirect_to_slash_directory()
+            .default_handler(web::to(error_404))
     };
 
-    let favicon_route = conf.favicon_route.clone();
-    let css_route = conf.css_route.clone();
-
-    let default_color_scheme = conf.default_color_scheme.clone();
-    let default_color_scheme_dark = conf.default_color_scheme_dark.clone();
-    let hide_version_footer = conf.hide_version_footer;
-
-    if let Some(serve_path) = serve_path {
+    if !conf.path.is_file() {
         if conf.file_upload {
             // Allow file upload
-            app.service(
-                web::resource(&upload_route).route(web::post().to(move |req, payload| {
-                    file_upload::upload_file(
-                        req,
-                        payload,
-                        uses_random_route,
-                        favicon_route.clone(),
-                        css_route.clone(),
-                        default_color_scheme.clone(),
-                        default_color_scheme_dark.clone(),
-                        hide_version_footer,
-                    )
-                })),
-            )
-            // Handle directories
-            .service(serve_path);
-        } else {
-            // Handle directories
-            app.service(serve_path);
+            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(&full_route).route(web::to(listing::file_handler)));
+        app.service(web::resource(["", "/"]).route(web::to(listing::file_handler)));
     }
 }
 
-async fn error_404(req: HttpRequest) -> HttpResponse {
-    let err_404 = ContextualError::RouteNotFoundError(req.path().to_string());
-    let conf = req.app_data::<MiniserveConfig>().unwrap();
-    let uses_random_route = conf.random_route.is_some();
-    let favicon_route = conf.favicon_route.clone();
-    let css_route = conf.css_route.clone();
-    let query_params = listing::extract_query_parameters(&req);
-
-    errors::log_error_chain(err_404.to_string());
-
-    actix_web::HttpResponse::NotFound().body(
-        renderer::render_error(
-            &err_404.to_string(),
-            StatusCode::NOT_FOUND,
-            "/",
-            query_params.sort,
-            query_params.order,
-            false,
-            !uses_random_route,
-            &favicon_route,
-            &css_route,
-            &conf.default_color_scheme,
-            &conf.default_color_scheme_dark,
-            conf.hide_version_footer,
-        )
-        .into_string(),
-    )
+async fn error_404(req: HttpRequest) -> Result<HttpResponse, ContextualError> {
+    Err(ContextualError::RouteNotFoundError(req.path().to_string()))
 }
 
 async fn favicon() -> impl Responder {
diff --git a/src/renderer.rs b/src/renderer.rs
index 3360504..d1821dd 100644
--- a/src/renderer.rs
+++ b/src/renderer.rs
@@ -6,31 +6,24 @@ use maud::{html, Markup, PreEscaped, DOCTYPE};
 use std::time::SystemTime;
 use strum::IntoEnumIterator;
 
-use crate::archive::ArchiveMethod;
-use crate::listing::{Breadcrumb, Entry, SortingMethod, SortingOrder};
+use crate::listing::{Breadcrumb, Entry, QueryParameters, SortingMethod, SortingOrder};
+use crate::{archive::ArchiveMethod, MiniserveConfig};
 
 /// Renders the file listing
-#[allow(clippy::too_many_arguments)]
 pub fn page(
     entries: Vec<Entry>,
     is_root: bool,
-    sort_method: Option<SortingMethod>,
-    sort_order: Option<SortingOrder>,
-    show_qrcode: bool,
-    file_upload: bool,
-    upload_route: &str,
-    favicon_route: &str,
-    css_route: &str,
-    default_color_scheme: &str,
-    default_color_scheme_dark: &str,
-    encoded_dir: &str,
+    query_params: QueryParameters,
     breadcrumbs: Vec<Breadcrumb>,
-    tar_enabled: bool,
-    tar_gz_enabled: bool,
-    zip_enabled: bool,
-    hide_version_footer: bool,
+    encoded_dir: &str,
+    conf: &MiniserveConfig,
 ) -> Markup {
-    let upload_action = build_upload_action(upload_route, encoded_dir, sort_method, sort_order);
+    let upload_route = match conf.random_route {
+        Some(ref random_route) => format!("/{}/upload", random_route),
+        None => "/upload".to_string(),
+    };
+    let (sort_method, sort_order) = (query_params.sort, query_params.order);
+    let upload_action = build_upload_action(&upload_route, encoded_dir, sort_method, sort_order);
 
     let title_path = breadcrumbs
         .iter()
@@ -41,11 +34,11 @@ pub fn page(
     html! {
         (DOCTYPE)
         html {
-            (page_header(&title_path, file_upload, favicon_route, css_route))
+            (page_header(&title_path, conf.file_upload, &conf.favicon_route, &conf.css_route))
 
             body#drop-container
-                .(format!("default_theme_{}", default_color_scheme))
-                .(format!("default_theme_dark_{}", default_color_scheme_dark)) {
+                .(format!("default_theme_{}", conf.default_color_scheme))
+                .(format!("default_theme_dark_{}", conf.default_color_scheme_dark)) {
 
                 (PreEscaped(r#"
                     <script>
@@ -71,14 +64,14 @@ pub fn page(
                     </script>
                     "#))
 
-                @if file_upload {
+                @if conf.file_upload {
                     div.drag-form {
                         div.drag-title {
                             h1 { "Drop your file here to upload it" }
                         }
                     }
                 }
-                (color_scheme_selector(show_qrcode))
+                (color_scheme_selector(conf.show_qrcode))
                 div.container {
                     span#top { }
                     h1.title dir="ltr" {
@@ -95,16 +88,16 @@ pub fn page(
                         }
                     }
                     div.toolbar {
-                        @if tar_enabled || tar_gz_enabled || zip_enabled {
+                        @if conf.tar_enabled || conf.tar_gz_enabled || conf.zip_enabled {
                             div.download {
                                 @for archive_method in ArchiveMethod::iter() {
-                                    @if archive_method.is_enabled(tar_enabled, tar_gz_enabled, zip_enabled) {
+                                    @if archive_method.is_enabled(conf.tar_enabled, conf.tar_gz_enabled, conf.zip_enabled) {
                                         (archive_button(archive_method, sort_method, sort_order))
                                     }
                                 }
                             }
                         }
-                        @if file_upload {
+                        @if conf.file_upload {
                             div.upload {
                                 form id="file_submit" action=(upload_action) method="POST" enctype="multipart/form-data" {
                                     p { "Select a file to upload or drag it anywhere into the window" }
@@ -141,7 +134,7 @@ pub fn page(
                     a.back href="#top" {
                         (arrow_up())
                     }
-                    @if !hide_version_footer {
+                    @if !conf.hide_version_footer {
                         (version_footer())
                     }
                 }
@@ -478,48 +471,44 @@ fn humanize_systemtime(time: Option<SystemTime>) -> Option<String> {
 }
 
 /// Renders an error on the webpage
-#[allow(clippy::too_many_arguments)]
 pub fn render_error(
     error_description: &str,
     error_code: StatusCode,
+    conf: &MiniserveConfig,
     return_address: &str,
-    sort_method: Option<SortingMethod>,
-    sort_order: Option<SortingOrder>,
-    has_referer: bool,
-    display_back_link: bool,
-    favicon_route: &str,
-    css_route: &str,
-    default_color_scheme: &str,
-    default_color_scheme_dark: &str,
-    hide_version_footer: bool,
 ) -> Markup {
-    let link = if has_referer {
-        return_address.to_string()
-    } else {
-        parametrized_link(return_address, sort_method, sort_order)
-    };
-
     html! {
         (DOCTYPE)
         html {
-            (page_header(&error_code.to_string(), false, favicon_route, css_route))
+            (page_header(&error_code.to_string(), false, &conf.favicon_route, &conf.css_route))
+
+            body.(format!("default_theme_{}", conf.default_color_scheme))
+                .(format!("default_theme_dark_{}", conf.default_color_scheme_dark)) {
 
-            body.(format!("default_theme_{}", default_color_scheme))
-                .(format!("default_theme_dark_{}", default_color_scheme_dark)) {
+                (PreEscaped(r#"
+                    <script>
+                        // read theme from local storage and apply it to body
+                        var theme = localStorage.getItem('theme');
+                        if (theme != null && theme != 'default') {
+                            document.body.classList.add('theme_' + theme);
+                        }
+                    </script>
+                    "#))
 
                 div.error {
                     p { (error_code.to_string()) }
                     @for error in error_description.lines() {
                         p { (error) }
                     }
-                    @if display_back_link {
+                    // WARN don't expose random route!
+                    @if !conf.random_route.is_some() {
                         div.error-nav {
-                            a.error-back href=(link) {
+                            a.error-back href=(return_address) {
                                 "Go back to file listing"
                             }
                         }
                     }
-                    @if !hide_version_footer {
+                    @if !conf.hide_version_footer {
                         (version_footer())
                     }
                 }