From 79963079fae23668cb0694834faa5d43d0d99f3b Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 12 May 2021 14:53:29 +0300 Subject: Fix clippy::too_many_arguments and rework error .. ... page rendering Too many arguments are moved around and many of them are already stored in MiniserveConfig. Many of these are used to render error pages. To fix this issue, it was necessary to rework error page rendering: 1. Implement `ResponseError` for `ContextualError` so that it can be returned from service handlers as is and will then be automatically logged to the console and converted into an error response. 2. At service handler level, all error responses are now rendered as plain text. 3. 'error_page_middleware' is now responsible for the rendering of the final error page from plain text reponses. Signed-off-by: Ali MJ Al-Nasrawy --- src/file_upload.rs | 167 ++++++++--------------------------------------------- 1 file changed, 25 insertions(+), 142 deletions(-) (limited to 'src/file_upload.rs') 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 { +) -> Result { let conf = req.app_data::().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::>() - .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, - sorting_order: Option, - 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()) } -- cgit v1.2.3