From 6551018818e39c1fc437dc11670ffb964d8f5888 Mon Sep 17 00:00:00 2001
From: Jose Quintana <joseluisquintana20@gmail.com>
Date: Fri, 12 Feb 2021 15:42:20 +0100
Subject: [PATCH] refactor: optimize static file middleware file searching

---
 src/staticfile_middleware/staticfile.rs | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------
 1 file changed, 68 insertions(+), 90 deletions(-)

diff --git a/src/staticfile_middleware/staticfile.rs b/src/staticfile_middleware/staticfile.rs
index 392e614..61ecb22 100644
--- a/src/staticfile_middleware/staticfile.rs
+++ b/src/staticfile_middleware/staticfile.rs
@@ -10,41 +10,41 @@ use iron::prelude::*;
 use iron::status;
 use std::fs::{File, Metadata};
 use std::path::{Path, PathBuf};
-use std::time::SystemTime;
-use std::time::UNIX_EPOCH;
+use std::time::{SystemTime, UNIX_EPOCH};
 use std::{error, io};
 
-use crate::helpers as utils;
+use crate::helpers;
 use crate::staticfile_middleware::partial_file::PartialFile;
 
 /// Recursively serves files from the specified root and assets directories.
 pub struct Staticfile {
     root: PathBuf,
     assets: PathBuf,
-    dir_list: bool,
+    dir_listing: bool,
 }
 
 impl Staticfile {
-    pub fn new<P: AsRef<Path>>(root: P, assets: P, dir_list: bool) -> io::Result<Staticfile>
+    pub fn new<P: AsRef<Path>>(
+        root_dir: P,
+        assets_dir: P,
+        dir_listing: bool,
+    ) -> io::Result<Staticfile>
     where
         PathBuf: From<P>,
     {
-        let root = root.into();
-        let assets = assets.into();
-
         Ok(Staticfile {
-            root,
-            assets,
-            dir_list,
+            root: root_dir.into(),
+            assets: assets_dir.into(),
+            dir_listing,
         })
     }
 
     fn resolve_path(&self, path: &[&str]) -> Result<PathBuf, Box<dyn error::Error>> {
-        let path_dirname = path[0];
+        let current_dirname = path[0];
         let asserts_dirname = self.assets.iter().last().unwrap().to_str().unwrap();
         let mut is_assets = false;
 
-        let resolved = if path_dirname == asserts_dirname {
+        let path_resolved = if current_dirname == asserts_dirname {
             // Assets path validation resolve
             is_assets = true;
 
@@ -64,18 +64,18 @@ impl Staticfile {
             res
         };
 
-        let resolved = PathBuf::from(utils::adjust_canonicalization(resolved));
+        let path_resolved = PathBuf::from(helpers::adjust_canonicalization(path_resolved));
         let base_path = if is_assets { &self.assets } else { &self.root };
 
         // Protect against path/directory traversal
-        if !resolved.starts_with(&base_path) {
+        if !path_resolved.starts_with(&base_path) {
             return Err(From::from(format!(
                 "Cannot leave {:?} base path",
                 &base_path
             )));
         }
 
-        Ok(resolved)
+        Ok(path_resolved)
     }
 }
 
@@ -87,17 +87,17 @@ impl Handler for Staticfile {
         }
 
         // Resolve path on file system
-        let file_path = match self.resolve_path(&req.url.path()) {
+        let path_resolved = match self.resolve_path(&req.url.path()) {
             Ok(file_path) => file_path,
             Err(_) => return Ok(Response::with(status::NotFound)),
         };
 
-        // 1. Check if directory listing feature is enabled,
+        // 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 self.dir_list && file_path.is_dir() && !file_path.join("index.html").exists() {
-            let encoding = Encoding::Identity;
-            let readir = match std::fs::read_dir(file_path) {
+        if self.dir_listing && path_resolved.is_dir() && !path_resolved.join("index.html").exists()
+        {
+            let read_dir = match std::fs::read_dir(path_resolved) {
                 Ok(dir) => dir,
                 Err(err) => {
                     error!("{}", err);
@@ -132,7 +132,7 @@ impl Handler for Staticfile {
                 entries_str =
                     String::from("<tr><td colspan=\"3\"><a href=\"../\">../</a></td></tr>");
             }
-            for entry in readir {
+            for entry in read_dir {
                 let entry = entry.unwrap();
                 let meta = entry.metadata().unwrap();
                 let mut filesize = meta.len().file_size(file_size_opts::DECIMAL).unwrap();
@@ -142,7 +142,7 @@ impl Handler for Staticfile {
                     filesize = String::from("-")
                 }
                 let uri = format!("{}{}", current_path, name);
-                let modified = get_last_modified(meta.modified().unwrap()).unwrap();
+                let modified = parse_last_modified(meta.modified().unwrap()).unwrap();
 
                 entries_str = format!(
                     "{}<tr><td><a href=\"{}\" title=\"{}\">{}</a></td><td style=\"width: 160px;\">{}</td><td align=\"right\" style=\"width: 140px;\">{}</td></tr>",
@@ -155,12 +155,12 @@ impl Handler for Staticfile {
                 );
             }
 
-            let page = format!(
+            let page_str = format!(
                 "<html><head><title>Index of {}</title></head><body><h1>Index of {}</h1><table style=\"min-width:680px;\"><tr><th colspan=\"3\"><hr></th></tr>{}<tr><th colspan=\"3\"><hr></th></tr></table></body></html>", current_path, current_path, entries_str
             );
-            let len = page.len() as u64;
-            let content_encoding = ContentEncoding(vec![encoding]);
-            let mut resp = Response::with((status::Ok, Header(content_encoding), page));
+            let len = page_str.len() as u64;
+            let content_encoding = ContentEncoding(vec![Encoding::Identity]);
+            let mut resp = Response::with((status::Ok, Header(content_encoding), page_str));
 
             // Empty current response body on HEAD requests,
             // just setting up the `content-length` header (size of the file in bytes)
@@ -173,10 +173,10 @@ impl Handler for Staticfile {
             return Ok(resp);
         }
 
-        // 2. Otherwise proceed with the normal file-response process
+        // 2. Otherwise proceed with the normal file-  process
 
-        // Get current file metadata
-        let file = match StaticFileWithMetadata::search(file_path.clone()) {
+        // Search a file and its metadata by the resolved path
+        let static_file = match StaticFileWithMeta::search(path_resolved.clone()) {
             Ok(f) => f,
             Err(e) => {
                 trace!("{}", e);
@@ -185,43 +185,27 @@ impl Handler for Staticfile {
         };
 
         // Apply last modified date time
-        let client_last_modified = req.headers.get::<IfModifiedSince>();
-        let last_modified = file.last_modified().ok().map(HttpDate);
+        let client_last_mod = req.headers.get::<IfModifiedSince>();
+        let last_mod = static_file.last_modified().ok().map(HttpDate);
 
-        if let (Some(client_last_modified), Some(last_modified)) =
-            (client_last_modified, last_modified)
-        {
+        if let (Some(client_last_mod), Some(last_mod)) = (client_last_mod, last_mod) {
             trace!(
                 "Comparing {} (file) <= {} (req)",
-                last_modified,
-                client_last_modified.0
+                last_mod,
+                client_last_mod.0
             );
 
-            if last_modified <= client_last_modified.0 {
+            if last_mod <= client_last_mod.0 {
                 return Ok(Response::with(status::NotModified));
             }
         }
 
-        // Add Encoding Gzip header
-        let encoding = if file.is_gz {
-            Encoding::Gzip
-        } else {
-            Encoding::Identity
-        };
-        let encoding = ContentEncoding(vec![encoding]);
-
-        // Prepare response
-        let mut resp = match last_modified {
-            Some(last_modified) => {
-                let last_modified = LastModified(last_modified);
-                Response::with((
-                    status::Ok,
-                    Header(last_modified),
-                    Header(encoding),
-                    file.file,
-                ))
+        // Prepare response object
+        let mut resp = match last_mod {
+            Some(last_mod) => {
+                Response::with((status::Ok, Header(LastModified(last_mod)), static_file.file))
             }
-            None => Response::with((status::Ok, Header(encoding), file.file)),
+            None => Response::with((status::Ok, static_file.file)),
         };
 
         // Empty current response body on HEAD requests,
@@ -229,7 +213,7 @@ impl Handler for Staticfile {
         // https://tools.ietf.org/html/rfc7231#section-4.3.2
         if req.method == Method::Head {
             resp.set_mut(vec![]);
-            resp.set_mut(Header(ContentLength(file.metadata.len())));
+            resp.set_mut(Header(ContentLength(static_file.meta.len())));
             return Ok(resp);
         }
 
@@ -241,7 +225,7 @@ impl Handler for Staticfile {
             None => resp,
             // Try to deliver partial content
             Some(Range::Bytes(v)) => {
-                if let Ok(partial_file) = PartialFile::from_path(&file_path, v) {
+                if let Ok(partial_file) = PartialFile::from_path(&path_resolved, v) {
                     Response::with((
                         status::Ok,
                         partial_file,
@@ -258,52 +242,46 @@ impl Handler for Staticfile {
     }
 }
 
-struct StaticFileWithMetadata {
+/// It represents a regular source file in file system with its metadata.
+struct StaticFileWithMeta {
     file: File,
-    metadata: Metadata,
-    is_gz: bool,
+    meta: Metadata,
 }
 
-impl StaticFileWithMetadata {
-    pub fn search(path: PathBuf) -> Result<StaticFileWithMetadata, Box<dyn error::Error>> {
-        let mut file_path = path;
-        trace!("Opening {}", file_path.display());
-        let metadata = std::fs::metadata(&file_path)?;
+impl StaticFileWithMeta {
+    /// Search for a regular source file in file system.
+    /// If source file is a directory then it attempts to search for an index.html.
+    pub fn search(mut src: PathBuf) -> Result<StaticFileWithMeta, Box<dyn error::Error>> {
+        trace!("Opening {}", src.display());
+
+        let mut auto_index = false;
+        let meta = std::fs::metadata(&src)?;
 
-        // Look for index.html inside of a directory
-        if metadata.is_dir() {
-            file_path.push("index.html");
-            trace!("Redirecting to index {}", file_path.display());
+        // Look for an `index.html` file inside of a directory
+        if meta.is_dir() {
+            src.push("index.html");
+            auto_index = true;
+            trace!("Redirecting to index {}", src.display());
         }
 
-        let file = StaticFileWithMetadata::open(&file_path)?;
-        if file.metadata.is_file() {
-            Ok(file)
+        // Attempt to open source file in read-only mode
+        let file = File::open(src)?;
+        let meta = if auto_index { file.metadata()? } else { meta };
+
+        if meta.is_file() {
+            Ok(StaticFileWithMeta { file, meta })
         } else {
             Err(From::from("Requested path was not a regular file"))
         }
     }
 
-    fn open<P>(path: P) -> Result<StaticFileWithMetadata, Box<dyn error::Error>>
-    where
-        P: AsRef<Path>,
-    {
-        let file = File::open(path)?;
-        let metadata = file.metadata()?;
-
-        Ok(StaticFileWithMetadata {
-            file,
-            metadata,
-            is_gz: false,
-        })
-    }
-
+    /// Get the last modification time of current file.
     pub fn last_modified(&self) -> Result<time::Tm, Box<dyn error::Error>> {
-        get_last_modified(self.metadata.modified()?)
+        parse_last_modified(self.meta.modified()?)
     }
 }
 
-fn get_last_modified(modified: SystemTime) -> Result<time::Tm, Box<dyn error::Error>> {
+fn parse_last_modified(modified: SystemTime) -> Result<time::Tm, Box<dyn error::Error>> {
     let since_epoch = modified.duration_since(UNIX_EPOCH)?;
     // HTTP times don't have nanosecond precision, so we truncate
     // the modification time.
--
libgit2 1.7.2