From 70a76ed58b63579802887051e01eb8580549bdaf Mon Sep 17 00:00:00 2001 From: Jose Quintana Date: Fri, 9 Jul 2021 22:54:59 +0200 Subject: [PATCH] refactor: optimize root path of static file module --- src/handler.rs | 4 ++-- src/server.rs | 3 ++- src/static_files.rs | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------- 3 files changed, 54 insertions(+), 44 deletions(-) diff --git a/src/handler.rs b/src/handler.rs index f3b06d3..f182e10 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -7,7 +7,7 @@ use crate::{Error, Result}; /// It defines options for a request handler. pub struct RequestHandlerOpts { - pub root_dir: PathBuf, + pub root_dir: Arc, pub compression: bool, pub dir_listing: bool, pub cors: Option>, @@ -28,7 +28,7 @@ impl RequestHandler { let method = req.method(); let headers = req.headers(); - let root_dir = self.opts.root_dir.as_path(); + let root_dir = self.opts.root_dir.as_ref(); let uri_path = req.uri().path(); let dir_listing = self.opts.dir_listing; diff --git a/src/server.rs b/src/server.rs index 6bf08cf..01a1f1f 100644 --- a/src/server.rs +++ b/src/server.rs @@ -2,6 +2,7 @@ use hyper::server::conn::AddrIncoming; use hyper::server::Server as HyperServer; use listenfd::ListenFd; use std::net::{IpAddr, SocketAddr, TcpListener}; +use std::sync::Arc; use structopt::StructOpt; use crate::handler::{RequestHandler, RequestHandlerOpts}; @@ -77,7 +78,7 @@ impl Server { } // Check for a valid root directory - let root_dir = helpers::get_valid_dirpath(&opts.root)?; + let root_dir = Arc::new(helpers::get_valid_dirpath(&opts.root)?); // Custom error pages content error_page::PAGE_404 diff --git a/src/static_files.rs b/src/static_files.rs index b2ab824..277c3a9 100644 --- a/src/static_files.rs +++ b/src/static_files.rs @@ -18,6 +18,7 @@ use std::io; use std::ops::Bound; use std::path::PathBuf; use std::pin::Pin; +use std::sync::Arc; use std::task::Poll; use std::time::{SystemTime, UNIX_EPOCH}; use std::{cmp, path::Path}; @@ -27,12 +28,22 @@ use tokio_util::io::poll_read_buf; use crate::Result; +/// Arc `PathBuf` reference wrapper since Arc doesn't implement AsRef. +#[derive(Clone, Debug)] +pub struct ArcPath(pub Arc); + +impl AsRef for ArcPath { + fn as_ref(&self) -> &Path { + (*self.0).as_ref() + } +} + /// Entry point to handle incoming requests which map to specific files /// on file system and return a file response. pub async fn handle( method: &Method, headers: &HeaderMap, - base: &Path, + path: impl Into, uri_path: &str, dir_listing: bool, ) -> Result, StatusCode> { @@ -41,13 +52,14 @@ pub async fn handle( return Err(StatusCode::METHOD_NOT_ALLOWED); } - let (filepath, meta, auto_index) = path_from_tail(base.to_owned(), uri_path).await?; + let base = Arc::new(path.into()); + let (filepath, meta, auto_index) = path_from_tail(base, uri_path).await?; // Directory listing // 1. Check if "directory listing" feature is enabled, // if current path is a valid directory and // if it does not contain an `index.html` file - if dir_listing && auto_index && !filepath.exists() { + if dir_listing && auto_index && !filepath.as_ref().exists() { // Redirect if current path does not end with a slash char if !uri_path.ends_with('/') { let uri = [uri_path, "/"].concat(); @@ -67,19 +79,19 @@ pub async fn handle( return Ok(resp); } - return directory_listing(method, uri_path, &filepath).await; + return directory_listing(method, uri_path, filepath.as_ref()).await; } - file_reply(headers, (filepath, meta, auto_index)).await + file_reply(headers, (filepath, &meta, auto_index)).await } /// Convert an incoming uri into a valid and sanitized path then returns a tuple // with the path as well as its file metadata and an auto index check if it's a directory. fn path_from_tail( - base: PathBuf, + base: Arc, tail: &str, -) -> impl Future> + Send { - future::ready(sanitize_path(base, tail)).and_then(|mut buf| async { +) -> impl Future> + Send { + future::ready(sanitize_path(base.as_ref(), tail)).and_then(|mut buf| async { match tokio::fs::metadata(&buf).await { Ok(meta) => { let mut auto_index = false; @@ -89,7 +101,7 @@ fn path_from_tail( auto_index = true; } tracing::trace!("dir: {:?}", buf); - Ok((buf, meta, auto_index)) + Ok((ArcPath(Arc::new(buf)), meta, auto_index)) } Err(err) => { tracing::debug!("file not found: {:?}", err); @@ -273,10 +285,10 @@ fn parse_last_modified(modified: SystemTime) -> Result, - res: (PathBuf, Metadata, bool), -) -> impl Future, StatusCode>> + Send { +fn file_reply<'a>( + headers: &'a HeaderMap, + res: (ArcPath, &'a Metadata, bool), +) -> impl Future, StatusCode>> + Send + 'a { let (path, meta, auto_index) = res; let conditionals = get_conditional_headers(headers); TkFile::open(path.clone()).then(move |res| match res { @@ -284,15 +296,19 @@ fn file_reply( Err(err) => { let status = match err.kind() { io::ErrorKind::NotFound => { - tracing::debug!("file not found: {:?}", path.display()); + tracing::debug!("file not found: {:?}", path.as_ref().display()); StatusCode::NOT_FOUND } io::ErrorKind::PermissionDenied => { - tracing::warn!("file permission denied: {:?}", path.display()); + tracing::warn!("file permission denied: {:?}", path.as_ref().display()); StatusCode::FORBIDDEN } _ => { - tracing::error!("file open error (path={:?}): {} ", path.display(), err); + tracing::error!( + "file open error (path={:?}): {} ", + path.as_ref().display(), + err + ); StatusCode::INTERNAL_SERVER_ERROR } }; @@ -315,7 +331,8 @@ fn get_conditional_headers(header_list: &HeaderMap) -> Conditionals } } -fn sanitize_path(mut buf: PathBuf, tail: &str) -> Result { +fn sanitize_path(base: impl AsRef, tail: &str) -> Result { + let mut buf = PathBuf::from(base.as_ref()); let p = match percent_decode_str(tail).decode_utf8() { Ok(p) => p, Err(err) => { @@ -400,30 +417,22 @@ impl Conditionals { } } -fn file_conditional( +async fn file_conditional( f: TkFile, - path: PathBuf, - meta: Metadata, + path: ArcPath, + meta: &Metadata, auto_index: bool, conditionals: Conditionals, -) -> impl Future, StatusCode>> + Send { - file_metadata(f, meta, auto_index) - .map_ok(|(file, meta)| response_body(file, &meta, path, conditionals)) -} - -async fn file_metadata( - f: TkFile, - meta: Metadata, - auto_index: bool, -) -> Result<(TkFile, Metadata), StatusCode> { +) -> Result, StatusCode> { if !auto_index { - return Ok((f, meta)); - } - match f.metadata().await { - Ok(meta) => Ok((f, meta)), - Err(err) => { - tracing::debug!("file metadata error: {}", err); - Err(StatusCode::INTERNAL_SERVER_ERROR) + Ok(response_body(f, meta, path, conditionals)) + } else { + match f.metadata().await { + Ok(meta) => Ok(response_body(f, &meta, path, conditionals)), + Err(err) => { + tracing::debug!("file metadata error: {}", err); + Err(StatusCode::INTERNAL_SERVER_ERROR) + } } } } @@ -431,7 +440,7 @@ async fn file_metadata( fn response_body( file: TkFile, meta: &Metadata, - path: PathBuf, + path: ArcPath, conditionals: Conditionals, ) -> Response { let mut len = meta.len(); @@ -623,14 +632,14 @@ mod tests { } assert_eq!( - sanitize_path(base.into(), "/foo.html").unwrap(), + sanitize_path(base, "/foo.html").unwrap(), p("/var/www/foo.html") ); // bad paths - sanitize_path(base.into(), "/../foo.html").expect_err("dot dot"); + sanitize_path(base, "/../foo.html").expect_err("dot dot"); - sanitize_path(base.into(), "/C:\\/foo.html").expect_err("C:\\"); + sanitize_path(base, "/C:\\/foo.html").expect_err("C:\\"); } #[test] -- libgit2 1.7.2