Develop (#5)

* pass through Zip and Io errors (#4) * pass through Zip and Io Errors replaces all instances of unwrap() * release 0.6.0 - read::ZipArchiveExtensions now returns ZipResult<PathBuf> instead of PathBuf as the underlying operation can return an error. Co-authored-by: Matthias Friedrich * Adds tests; extends try_is_zip method so that it can detect different archive formats. Updates README.md Co-authored-by: T.C. Hollingsworth <[email protected]>
matzefriedrich · Nov 30, 2020 · d1b02e9 · d1b02e9
1 parent 062b2ae
commit d1b02e9
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 50 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "zip-extensions"
-version = "0.5.0"
+version = "0.6.0"
 authors = ["Matthias Friedrich <[email protected]>"]
 edition = "2018"
 description = "An extension create for zip."
@@ -15,4 +15,4 @@ exclude = [
 ]
 
 [dependencies]
-zip = "0.5.5"
+zip = "0.5.8"
diff --git a/README.md b/README.md
@@ -15,8 +15,8 @@ Add the following dependencies to the `Cargo.toml` file.
 
 ````toml
 [dependencies]
-zip = "0.5.5"
-zip-extensions = "0.5.0"
+zip = "0.5.8"
+zip-extensions = "0.6.0"
 ````
 
 See https://github.com/mvdnes/zip-rs fur further information about `zip` dependencies.

diff --git a/src/file_utils.rs b/src/file_utils.rs
@@ -8,8 +8,8 @@ pub fn file_write_all_bytes(path: PathBuf, bytes: &[u8], overwrite: bool) -> io:
     if path.exists() && overwrite == false {
         return Err(Error::new(ErrorKind::AlreadyExists, "The specified file already exists."));
     }
-    let mut file = File::create(path).unwrap();
-    file.set_len(0).unwrap();
+    let mut file = File::create(path)?;
+    file.set_len(0)?;
     file.write(bytes)
 }
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -6,3 +6,30 @@ pub use crate::write::*;
 mod file_utils;
 pub mod read;
 pub mod write;
+
+#[cfg(test)]
+mod tests {
+    use crate::{is_zip, zip_create_from_directory};
+    use std::path::PathBuf;
+    use std::str::FromStr;
+    use std::fs::File;
+
+    #[test]
+    fn is_zip_returns_false_if_file_does_not_exists() {
+        let archive_file = PathBuf::from_str("missing.zip").unwrap();
+        let actual = is_zip(&archive_file);
+        assert_eq!(actual, false)
+    }
+
+    #[test]
+    fn is_zip_returns_true() {
+        let archive_file = PathBuf::from_str("empty.zip").unwrap();
+        let file = File::create(&archive_file.as_path()).unwrap();
+        let mut zip_writer = zip::ZipWriter::new(file);
+        zip_writer.set_comment("This is an empty ZIP file.");
+        zip_writer.finish().unwrap();
+        let actual = is_zip(&archive_file);
+        std::fs::remove_file(&archive_file.as_path());
+        assert_eq!(actual, true)
+    }
+}
diff --git a/src/read.rs b/src/read.rs
@@ -9,16 +9,16 @@ use std::fs::File;
 
 /// Extracts a ZIP file to the given directory.
 pub fn zip_extract(archive_file: &PathBuf, target_dir: &PathBuf) -> ZipResult<()> {
-    let file = File::open(archive_file).unwrap();
-    let mut archive = zip::ZipArchive::new(file).unwrap();
+    let file = File::open(archive_file)?;
+    let mut archive = zip::ZipArchive::new(file)?;
     archive.extract(target_dir)
 }
 
 /// Extracts and entry in the ZIP archive to the given directory.
 pub fn zip_extract_file(archive_file: &PathBuf, entry_path: &PathBuf, target_dir: &PathBuf, overwrite: bool) -> ZipResult<()> {
-    let file = File::open(archive_file).unwrap();
-    let mut archive = zip::ZipArchive::new(file).unwrap();
-    let file_number: usize = match archive.file_number(entry_path){
+    let file = File::open(archive_file)?;
+    let mut archive = zip::ZipArchive::new(file)?;
+    let file_number: usize = match archive.file_number(entry_path) {
         Some(index) => index,
         None => return Err(ZipError::FileNotFound)
     };
@@ -28,30 +28,44 @@ pub fn zip_extract_file(archive_file: &PathBuf, entry_path: &PathBuf, target_dir
 
 /// Extracts an entry in the ZIP archive to the given memory buffer.
 pub fn zip_extract_file_to_memory(archive_file: &PathBuf, entry_path: &PathBuf, buffer: &mut Vec<u8>) -> ZipResult<()> {
-    let file = File::open(archive_file).unwrap();
-    let mut archive = zip::ZipArchive::new(file).unwrap();
-    let file_number: usize = match archive.file_number(entry_path){
+    let file = File::open(archive_file)?;
+    let mut archive = zip::ZipArchive::new(file)?;
+    let file_number: usize = match archive.file_number(entry_path) {
         Some(index) => index,
         None => return Err(ZipError::FileNotFound)
     };
     return archive.extract_file_to_memory(file_number, buffer);
 }
 
 /// Determines whether the specified file is a ZIP file, or not.
-pub fn is_zip(file: &PathBuf) -> bool {
-    const ZIP_SIGNATURE: [u8; 4] = [0x50, 0x4b, 0x03, 0x04];
-    let mut file = File::open(file).unwrap();
+pub fn try_is_zip(file: &PathBuf) -> ZipResult<bool> {
+    const ZIP_SIGNATURE: [u8; 2] = [0x50, 0x4b];
+    const ZIP_ARCHIVE_FORMAT: [u8; 6] = [0x03, 0x04, 0x05, 0x06, 0x07, 0x08];
+    let mut file = File::open(file)?;
     let mut buffer: [u8; 4] = [0; 4];
-    let bytes_read = file.read(&mut buffer).unwrap();
-    if bytes_read == buffer.len() && bytes_read == ZIP_SIGNATURE.len() {
+    let bytes_read = file.read(&mut buffer)?;
+    if bytes_read == buffer.len() {
         for i in 0..ZIP_SIGNATURE.len() {
             if buffer[i] != ZIP_SIGNATURE[i] {
-                return false;
+                return Ok(false);
             }
         }
-        return true;
+
+        for i in (0..ZIP_ARCHIVE_FORMAT.len()).step_by(2) {
+            if buffer[2] == ZIP_ARCHIVE_FORMAT[i] || buffer[3] == ZIP_ARCHIVE_FORMAT[i + 1] {
+                return Ok(true);
+            }
+        }
+    }
+    Ok(false)
+}
+
+/// Determines whether the specified file is a ZIP file, or not.
+pub fn is_zip(file: &PathBuf) -> bool {
+    match try_is_zip(file) {
+        Ok(r) => r,
+        Err(_) => false,
     }
-    false
 }
 
 pub trait ZipArchiveExtensions {
@@ -65,7 +79,7 @@ pub trait ZipArchiveExtensions {
     fn extract_file_to_memory(&mut self, file_number: usize, buffer: &mut Vec<u8>) -> ZipResult<()>;
 
     /// Gets an entry´s path.
-    fn entry_path(&mut self, file_number: usize) -> PathBuf;
+    fn entry_path(&mut self, file_number: usize) -> ZipResult<PathBuf>;
 
     /// Finds the index of the specified entry.
     fn file_number(&mut self, entry_path: &PathBuf) -> Option<usize>;
@@ -78,16 +92,16 @@ impl<R: Read + io::Seek> ZipArchiveExtensions for ZipArchive<R> {
         }
 
         for file_number in 0..self.len() {
-            let mut next: ZipFile = self.by_index(file_number).unwrap();
+            let mut next: ZipFile = self.by_index(file_number)?;
             let sanitized_name = next.sanitized_name();
             if next.is_dir() {
                 let extracted_folder_path = target_directory.join(sanitized_name);
-                std::fs::create_dir_all(extracted_folder_path).unwrap();
+                std::fs::create_dir_all(extracted_folder_path)?;
             } else if next.is_file() {
                 let mut buffer: Vec<u8> = Vec::new();
-                let _bytes_read = next.read_to_end(&mut buffer).unwrap();
+                let _bytes_read = next.read_to_end(&mut buffer)?;
                 let extracted_file_path = target_directory.join(sanitized_name);
-                file_write_all_bytes(extracted_file_path, buffer.as_ref(), true).unwrap();
+                file_write_all_bytes(extracted_file_path, buffer.as_ref(), true)?;
             }
         }
 
@@ -96,31 +110,32 @@ impl<R: Read + io::Seek> ZipArchiveExtensions for ZipArchive<R> {
 
     fn extract_file(&mut self, file_number: usize, destination_file_path: &PathBuf, overwrite: bool) -> ZipResult<()> {
         let mut buffer: Vec<u8> = Vec::new();
-        self.extract_file_to_memory(file_number, &mut buffer).unwrap();
-        file_write_all_bytes(destination_file_path.to_path_buf(), buffer.as_ref(), overwrite).unwrap();
+        self.extract_file_to_memory(file_number, &mut buffer)?;
+        file_write_all_bytes(destination_file_path.to_path_buf(), buffer.as_ref(), overwrite)?;
         return Ok(());
     }
 
     fn extract_file_to_memory(&mut self, file_number: usize, buffer: &mut Vec<u8>) -> ZipResult<()> {
-        let mut next: ZipFile = self.by_index(file_number).unwrap();
+        let mut next: ZipFile = self.by_index(file_number)?;
         if next.is_file() {
-            let _bytes_read = next.read_to_end(buffer).unwrap();
+            let _bytes_read = next.read_to_end(buffer)?;
             return Ok(());
         }
         return Err(ZipError::Io(Error::new(ErrorKind::InvalidInput, "The specified index does not indicate a file entry.")));
     }
 
-    fn entry_path(&mut self, file_number: usize) -> PathBuf {
-        let next: ZipFile = self.by_index(file_number).unwrap();
-        return next.sanitized_name();
+    fn entry_path(&mut self, file_number: usize) -> ZipResult<PathBuf> {
+        let next: ZipFile = self.by_index(file_number)?;
+        return Ok(next.sanitized_name());
     }
 
     fn file_number(&mut self, entry_path: &PathBuf) -> Option<usize> {
         for file_number in 0..self.len() {
-            let next: ZipFile = self.by_index(file_number).unwrap();
-            let sanitized_name = next.sanitized_name();
-            if sanitized_name == *entry_path{
-                return Some(file_number);
+            if let Ok(next) = self.by_index(file_number) {
+                let sanitized_name = next.sanitized_name();
+                if sanitized_name == *entry_path {
+                    return Some(file_number);
+                }
             }
         }
         None

diff --git a/src/write.rs b/src/write.rs
@@ -15,7 +15,7 @@ pub fn zip_create_from_directory(archive_file: &PathBuf, directory: &PathBuf) ->
 
 /// Creates a zip archive that contains the files and directories from the specified directory, uses the specified compression level.
 pub fn zip_create_from_directory_with_options(archive_file: &PathBuf, directory: &PathBuf, options: FileOptions) -> ZipResult<()> {
-    let file = File::create(archive_file).unwrap();
+    let file = File::create(archive_file)?;
     let mut zip_writer = zip::ZipWriter::new(file);
     zip_writer.create_from_directory_with_options(directory, options)
 }
@@ -40,29 +40,28 @@ impl<W: Write + io::Seek> ZipWriterExtensions for ZipWriter<W> {
 
         let mut buffer = Vec::new();
 
-        while paths_queue.len() > 0 {
-            let next = paths_queue.pop().unwrap();
-            let directory_entry_iterator = std::fs::read_dir(next).unwrap();
+        while let Some(next) = paths_queue.pop() {
+            let directory_entry_iterator = std::fs::read_dir(next)?;
 
             for entry in directory_entry_iterator {
-                let entry_path = entry.unwrap().path();
-                let entry_metadata = std::fs::metadata(entry_path.clone()).unwrap();
+                let entry_path = entry?.path();
+                let entry_metadata = std::fs::metadata(entry_path.clone())?;
                 if entry_metadata.is_file() {
-                    let mut f = File::open(&entry_path).unwrap();
-                    f.read_to_end(&mut buffer).unwrap();
+                    let mut f = File::open(&entry_path)?;
+                    f.read_to_end(&mut buffer)?;
                     let relative_path = make_relative_path(&directory, &entry_path);
-                    self.start_file_from_path(&relative_path, options).unwrap();
-                    self.write_all(buffer.as_ref()).unwrap();
+                    self.start_file_from_path(&relative_path, options)?;
+                    self.write_all(buffer.as_ref())?;
                     buffer.clear();
                 } else if entry_metadata.is_dir() {
                     let relative_path = make_relative_path(&directory, &entry_path);
-                    self.add_directory_from_path(&relative_path, options).unwrap();
+                    self.add_directory_from_path(&relative_path, options)?;
                     paths_queue.push(entry_path.clone());
                 }
             }
         }
 
-        self.finish().unwrap();
+        self.finish()?;
         Ok(())
     }
 }