diff options
author | Ali MJ Al-Nasrawy <alimjalnasrawy@gmail.com> | 2021-05-12 11:53:29 +0000 |
---|---|---|
committer | Ali MJ Al-Nasrawy <alimjalnasrawy@gmail.com> | 2021-08-30 17:48:49 +0000 |
commit | 79963079fae23668cb0694834faa5d43d0d99f3b (patch) | |
tree | 033bce5f4ddbe68a639229598f1fca8737cf8257 /src/file_upload.rs | |
parent | Bump deps (diff) | |
download | miniserve-79963079fae23668cb0694834faa5d43d0d99f3b.tar.gz miniserve-79963079fae23668cb0694834faa5d43d0d99f3b.zip |
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 <alimjalnasrawy@gmail.com>
Diffstat (limited to 'src/file_upload.rs')
-rw-r--r-- | src/file_upload.rs | 167 |
1 files changed, 25 insertions, 142 deletions
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()) } |