From f2129318428e007ebeb6679ca6024fed4f79f641 Mon Sep 17 00:00:00 2001 From: _ <> Date: Sun, 29 Nov 2020 19:47:40 +0000 Subject: [PATCH] :recycle: Remove more unwraps --- crates/ptth_server/src/file_server/errors.rs | 27 ++++++++ crates/ptth_server/src/file_server/mod.rs | 66 +++++++++----------- crates/ptth_server/src/file_server/tests.rs | 6 +- 3 files changed, 61 insertions(+), 38 deletions(-) diff --git a/crates/ptth_server/src/file_server/errors.rs b/crates/ptth_server/src/file_server/errors.rs index de32bf9..a4ae14b 100644 --- a/crates/ptth_server/src/file_server/errors.rs +++ b/crates/ptth_server/src/file_server/errors.rs @@ -1,7 +1,34 @@ use thiserror::Error; +#[derive (Debug, Error, PartialEq)] +pub enum MarkdownError { + #[error ("File is too big to process")] + TooBig, + + #[error ("File is not UTF-8")] + NotUtf8, +} + #[derive (Debug, Error)] pub enum FileServerError { #[error ("Handlebars render error")] Handlebars (#[from] handlebars::RenderError), + + #[error ("I/O error")] + Io (#[from] std::io::Error), + + #[error ("Request path is not UTF-8")] + PathNotUtf8 (std::str::Utf8Error), + + #[error ("Can't get file metadata")] + CantGetFileMetadata (std::io::Error), + + #[error ("No file name requested")] + NoFileNameRequested, + + #[error ("File path is not UTF-8")] + FilePathNotUtf8, + + #[error ("Markdown error")] + Markdown (#[from] MarkdownError), } diff --git a/crates/ptth_server/src/file_server/mod.rs b/crates/ptth_server/src/file_server/mod.rs index 38dcfcf..74aec33 100644 --- a/crates/ptth_server/src/file_server/mod.rs +++ b/crates/ptth_server/src/file_server/mod.rs @@ -52,7 +52,10 @@ use ptth_core::{ }; pub mod errors; -use errors::FileServerError; +use errors::{ + FileServerError, + MarkdownError, +}; #[derive (Debug, Serialize)] pub struct ServerInfo { @@ -262,7 +265,7 @@ async fn serve_dir ( server_info: &ServerInfo, path: Cow <'_, str>, mut dir: ReadDir -) -> Response +) -> Result { let mut entries = vec! []; @@ -270,15 +273,15 @@ async fn serve_dir ( entries.push (read_dir_entry (entry).await); } - entries.sort_unstable_by (|a, b| a.file_name.partial_cmp (&b.file_name).unwrap ()); + entries.sort_unstable_by (|a, b| a.file_name.cmp (&b.file_name)); let s = handlebars.render ("file_server_dir", &TemplateDirPage { path, entries, server_info, - }).unwrap (); + })?; - serve_html (s) + Ok (serve_html (s)) } #[instrument (level = "debug", skip (f))] @@ -288,7 +291,7 @@ async fn serve_file ( range: Range , range_requested: bool ) - -> Response +-> Result { let (tx, rx) = channel (1); let body = if should_send_body { @@ -303,11 +306,10 @@ async fn serve_file ( let content_length = range.end - range.start; let seek = SeekFrom::Start (range.start); + f.seek (seek).await?; if should_send_body { tokio::spawn (async move { - f.seek (seek).await.unwrap (); - let mut tx = tx; let mut bytes_sent = 0; let mut bytes_left = content_length; @@ -373,7 +375,7 @@ async fn serve_file ( response.body (body); } - response + Ok (response) } fn serve_error ( @@ -443,13 +445,6 @@ struct ServeFileParams { file: AlwaysEqual , } -#[derive (Debug, PartialEq)] -enum MarkdownError { - TooBig, - // NotMarkdown, - NotUtf8, -} - #[derive (Debug, PartialEq)] enum InternalResponse { Favicon, @@ -475,7 +470,7 @@ async fn internal_serve_all ( headers: &HashMap >, hidden_path: Option <&Path> ) - -> InternalResponse +-> Result { use std::str::FromStr; use InternalResponse::*; @@ -483,7 +478,7 @@ async fn internal_serve_all ( info! ("Client requested {}", uri); let uri = match hyper::Uri::from_str (uri) { - Err (_) => return InvalidUri, + Err (_) => return Ok (InvalidUri), Ok (x) => x, }; @@ -492,28 +487,28 @@ async fn internal_serve_all ( Method::Head => false, m => { debug! ("Unsupported method {:?}", m); - return MethodNotAllowed; + return Ok (MethodNotAllowed); } }; if uri.path () == "/favicon.ico" { - return Favicon; + return Ok (Favicon); } let path = match prefix_match ("/files", uri.path ()) { Some (x) => x, - None => return Root, + None => return Ok (Root), }; if path == "" { - return Redirect ("files/".to_string ()); + return Ok (Redirect ("files/".to_string ())); } // TODO: There is totally a dir traversal attack in here somewhere let encoded_path = &path [1..]; - let path_s = percent_decode (encoded_path.as_bytes ()).decode_utf8 ().unwrap (); + let path_s = percent_decode (encoded_path.as_bytes ()).decode_utf8 ().map_err (FileServerError::PathNotUtf8)?; let path = Path::new (&*path_s); let full_path = root.join (path); @@ -522,19 +517,20 @@ async fn internal_serve_all ( if let Some (hidden_path) = hidden_path { if full_path == hidden_path { - return Forbidden; + return Ok (Forbidden); } } let has_trailing_slash = path_s.is_empty () || path_s.ends_with ('/'); - if let Ok (dir) = read_dir (&full_path).await { + Ok (if let Ok (dir) = read_dir (&full_path).await { if ! has_trailing_slash { - return Redirect (format! ("{}/", path.file_name ().unwrap ().to_str ().unwrap ())); + let file_name = path.file_name ().ok_or (FileServerError::NoFileNameRequested)?; + return Ok (Redirect (format! ("{}/", file_name.to_str ().ok_or (FileServerError::FilePathNotUtf8)?))); } if uri.query ().is_some () { - return InvalidQuery; + return Ok (InvalidQuery); } let dir = dir.into (); @@ -547,10 +543,10 @@ async fn internal_serve_all ( else if let Ok (mut file) = File::open (&full_path).await { use std::os::unix::fs::PermissionsExt; - let file_md = file.metadata ().await.unwrap (); + let file_md = file.metadata ().await.map_err (FileServerError::CantGetFileMetadata)?; if file_md.permissions ().mode () == super::load_toml::CONFIG_PERMISSIONS_MODE { - return Forbidden; + return Ok (Forbidden); } let file_len = file_md.len (); @@ -567,10 +563,10 @@ async fn internal_serve_all ( } else { let mut buffer = vec! [0_u8; MAX_BUF_SIZE.try_into ().unwrap ()]; - let bytes_read = file.read (&mut buffer).await.unwrap (); + let bytes_read = file.read (&mut buffer).await?; buffer.truncate (bytes_read); - MarkdownPreview (render_markdown_styled (&buffer).unwrap ()) + MarkdownPreview (render_markdown_styled (&buffer)?) } } else { @@ -603,7 +599,7 @@ async fn internal_serve_all ( } else { NotFound - } + }) } #[instrument (level = "debug", skip (handlebars, headers))] @@ -620,7 +616,7 @@ pub async fn serve_all ( { use InternalResponse::*; - Ok (match internal_serve_all (root, method, uri, headers, hidden_path).await { + Ok (match internal_serve_all (root, method, uri, headers, hidden_path).await? { Favicon => serve_error (StatusCode::NotFound, ""), Forbidden => serve_error (StatusCode::Forbidden, "403 Forbidden"), InvalidUri => serve_error (StatusCode::BadRequest, "Invalid URI"), @@ -639,13 +635,13 @@ pub async fn serve_all ( ServeDir (ServeDirParams { path, dir, - }) => serve_dir (handlebars, server_info, path.to_string_lossy (), dir.into_inner ()).await, + }) => serve_dir (handlebars, server_info, path.to_string_lossy (), dir.into_inner ()).await?, ServeFile (ServeFileParams { file, send_body, range, range_requested, - }) => serve_file (file.into_inner (), send_body, range, range_requested).await, + }) => serve_file (file.into_inner (), send_body, range, range_requested).await?, MarkdownErr (e) => match e { MarkdownError::TooBig => serve_error (StatusCode::InternalServerError, "File is too big to preview as Markdown"), //MarkdownError::NotMarkdown => serve_error (StatusCode::BadRequest, "File is not Markdown"), diff --git a/crates/ptth_server/src/file_server/tests.rs b/crates/ptth_server/src/file_server/tests.rs index f85361a..d81407d 100644 --- a/crates/ptth_server/src/file_server/tests.rs +++ b/crates/ptth_server/src/file_server/tests.rs @@ -158,7 +158,7 @@ fn file_server () { None ).await; - assert_eq! (resp, expected); + assert_eq! (resp.unwrap (), expected); } let resp = internal_serve_all ( @@ -171,7 +171,7 @@ fn file_server () { None ).await; - assert_eq! (resp, RangeNotSatisfiable (1_048_576)); + assert_eq! (resp.unwrap (), RangeNotSatisfiable (1_048_576)); let resp = internal_serve_all ( &file_server_root, @@ -181,7 +181,7 @@ fn file_server () { None ).await; - assert_eq! (resp, ServeFile (ServeFileParams { + assert_eq! (resp.unwrap (), ServeFile (ServeFileParams { send_body: false, range: 0..1_048_576, range_requested: false,