Refactor tar code

src/errors.rs

@@ -1,5 +1,7 @@
 //! Representations of various client errors
 
+use std::path::PathBuf;
+
 use hyper::{self, http, StatusCode};
 use serde_json::Error as SerdeError;
 use std::{error::Error as StdError, fmt, string::FromUtf8Error};
@@ -23,6 +25,7 @@ pub enum Error {
         message: String,
     },
     ConnectionNotUpgraded,
+    PathNotUtf8(PathBuf),
 }
 
 impl From<SerdeError> for Error {
@@ -82,6 +85,7 @@ impl fmt::Display for Error {
                 f,
                 "expected the docker host to upgrade the HTTP connection but it did not"
             ),
+            Error::PathNotUtf8(pb) => write!(f, "Path is not UTF-8: '{}'", pb.display()),
         }
     }
 }

src/tarball.rs

@@ -1,48 +1,28 @@
 use flate2::{write::GzEncoder, Compression};
 use std::{
     fs::{self, File},
-    io::{self, Write},
+    io::Write,
     path::{Path, MAIN_SEPARATOR},
 };
 use tar::Builder;
 
+use crate::errors::Error;
+use crate::errors::Result;
+
 // todo: this is pretty involved. (re)factor this into its own crate
 pub fn dir<W>(
     buf: W,
     path: &str,
-) -> io::Result<()>
+) -> Result<()>
 where
     W: Write,
 {
     let mut archive = Builder::new(GzEncoder::new(buf, Compression::best()));
-    fn bundle<F>(
-        dir: &Path,
-        f: &mut F,
-        bundle_dir: bool,
-    ) -> io::Result<()>
-    where
-        F: FnMut(&Path) -> io::Result<()>,
-    {
-        if fs::metadata(dir)?.is_dir() {
-            if bundle_dir {
-                f(&dir)?;
-            }
-            for entry in fs::read_dir(dir)? {
-                let entry = entry?;
-                if fs::metadata(entry.path())?.is_dir() {
-                    bundle(&entry.path(), f, true)?;
-                } else {
-                    f(&entry.path().as_path())?;
-                }
-            }
-        }
-        Ok(())
-    }
 
     {
         let base_path = Path::new(path).canonicalize()?;
-        // todo: don't unwrap
-        let mut base_path_str = base_path.to_str().unwrap().to_owned();
+
+        let mut base_path_str = base_path.to_str().ok_or_else(|| Error::PathNotUtf8(base_path.to_path_buf()))?.to_owned();
         if let Some(last) = base_path_str.chars().last() {
             if last != MAIN_SEPARATOR {
                 base_path_str.push(MAIN_SEPARATOR)
@@ -51,10 +31,9 @@ where
 
         let mut append = |path: &Path| {
             let canonical = path.canonicalize()?;
-            // todo: don't unwrap
             let relativized = canonical
                 .to_str()
-                .unwrap()
+                .ok_or_else(|| Error::PathNotUtf8(canonical.to_path_buf()))?
                 .trim_start_matches(&base_path_str[..]);
             if path.is_dir() {
                 archive.append_dir(Path::new(relativized), &canonical)?
@@ -69,3 +48,27 @@ where
 
     Ok(())
 }
+
+fn bundle<F>(
+    dir: &Path,
+    f: &mut F,
+    bundle_dir: bool,
+) -> Result<()>
+where
+    F: FnMut(&Path) -> Result<()>,
+{
+    if fs::metadata(dir)?.is_dir() {
+        if bundle_dir {
+            f(&dir)?;
+        }
+        for entry in fs::read_dir(dir)? {
+            let entry = entry?;
+            if fs::metadata(entry.path())?.is_dir() {
+                bundle(&entry.path(), f, true)?;
+            } else {
+                f(&entry.path().as_path())?;
+            }
+        }
+    }
+    Ok(())
+}