Allow copying File Options Again

Cargo.toml

@@ -1,8 +1,12 @@
 [package]
 name = "zip_next"
 version = "1.0.1"
-authors = ["Mathijs van de Nes <[email protected]>", "Marli Frost <[email protected]>", "Ryan Levick <[email protected]>",
-"Chris Hennick <[email protected]>"]
+authors = [
+    "Mathijs van de Nes <[email protected]>",
+    "Marli Frost <[email protected]>",
+    "Ryan Levick <[email protected]>",
+    "Chris Hennick <[email protected]>",
+]
 license = "MIT"
 repository = "https://github.com/Pr0methean/zip-next.git"
 keywords = ["zip", "archive"]
@@ -22,9 +26,11 @@ constant_time_eq = { version = "0.3.0", optional = true }
 crc32fast = "1.4.0"
 flate2 = { version = "1.0.28", default-features = false, optional = true }
 hmac = { version = "0.12.1", optional = true, features = ["reset"] }
-pbkdf2 = {version = "0.12.2", optional = true }
-sha1 = {version = "0.10.6", optional = true }
-time = { version = "0.3.34", optional = true, default-features = false, features = ["std"] }
+pbkdf2 = { version = "0.12.2", optional = true }
+sha1 = { version = "0.10.6", optional = true }
+time = { version = "0.3.34", optional = true, default-features = false, features = [
+    "std",
+] }
 zstd = { version = "0.13.1", optional = true, default-features = false }
 zopfli = { version = "0.8.0", optional = true }
 deflate64 = { version = "0.1.8", optional = true }
@@ -41,9 +47,9 @@ bencher = "0.1.5"
 getrandom = { version = "0.2.14", features = ["js"] }
 walkdir = "2.5.0"
 time = { version = "0.3.34", features = ["formatting", "macros"] }
-
+anyhow = "1"
 [features]
-aes-crypto = [ "aes", "constant_time_eq", "hmac", "pbkdf2", "sha1" ]
+aes-crypto = ["aes", "constant_time_eq", "hmac", "pbkdf2", "sha1"]
 chrono = ["chrono/default"]
 deflate = ["flate2/rust_backend"]
 deflate-miniz = ["flate2/default"]
@@ -52,7 +58,17 @@ deflate-zlib-ng = ["flate2/zlib-ng"]
 deflate-zopfli = ["zopfli"]
 lzma = ["lzma-rs/stream"]
 unreserved = []
-default = ["aes-crypto", "bzip2", "deflate", "deflate64", "deflate-zlib-ng", "deflate-zopfli", "lzma", "time", "zstd"]
+default = [
+    "aes-crypto",
+    "bzip2",
+    "deflate",
+    "deflate64",
+    "deflate-zlib-ng",
+    "deflate-zopfli",
+    "lzma",
+    "time",
+    "zstd",
+]
 
 [[bench]]
 name = "read_entry"

benches/read_entry.rs

@@ -4,13 +4,13 @@ use std::io::{Cursor, Read, Write};
 
 use bencher::Bencher;
 use getrandom::getrandom;
-use zip_next::{ZipArchive, ZipWriter};
+use zip_next::{write::SimpleFileOptions, ZipArchive, ZipWriter};
 
 fn generate_random_archive(size: usize) -> Vec<u8> {
     let data = Vec::new();
     let mut writer = ZipWriter::new(Cursor::new(data));
-    let options = zip_next::write::FileOptions::default()
-        .compression_method(zip_next::CompressionMethod::Stored);
+    let options =
+        SimpleFileOptions::default().compression_method(zip_next::CompressionMethod::Stored);
 
     writer.start_file("random.dat", options).unwrap();
     let mut bytes = vec![0u8; size];

benches/read_metadata.rs

@@ -3,7 +3,7 @@ use bencher::{benchmark_group, benchmark_main};
 use std::io::{Cursor, Write};
 
 use bencher::Bencher;
-use zip_next::write::FileOptions;
+use zip_next::write::SimpleFileOptions;
 use zip_next::{CompressionMethod, ZipArchive, ZipWriter};
 
 const FILE_COUNT: usize = 15_000;
@@ -12,13 +12,13 @@ const FILE_SIZE: usize = 1024;
 fn generate_random_archive(count_files: usize, file_size: usize) -> Vec<u8> {
     let data = Vec::new();
     let mut writer = ZipWriter::new(Cursor::new(data));
-    let options = FileOptions::default().compression_method(CompressionMethod::Stored);
+    let options = SimpleFileOptions::default().compression_method(CompressionMethod::Stored);
 
     let bytes = vec![0u8; file_size];
 
     for i in 0..count_files {
         let name = format!("file_deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef_{i}.dat");
-        writer.start_file(name, options.clone()).unwrap();
+        writer.start_file(name, options).unwrap();
         writer.write_all(&bytes).unwrap();
     }
 

examples/write_dir.rs

@@ -1,6 +1,6 @@
+use anyhow::Context;
 use std::io::prelude::*;
-use zip_next::result::ZipError;
-use zip_next::write::FileOptions;
+use zip_next::{result::ZipError, write::SimpleFileOptions};
 
 use std::fs::File;
 use std::path::Path;
@@ -58,7 +58,7 @@ fn real_main() -> i32 {
         }
         match doit(src_dir, dst_file, method.unwrap()) {
             Ok(_) => println!("done: {src_dir} written to {dst_file}"),
-            Err(e) => println!("Error: {e:?}"),
+            Err(e) => eprintln!("Error: {e:?}"),
         }
     }
 
@@ -70,26 +70,30 @@ fn zip_dir<T>(
     prefix: &str,
     writer: T,
     method: zip_next::CompressionMethod,
-) -> zip_next::result::ZipResult<()>
+) -> anyhow::Result<()>
 where
     T: Write + Seek,
 {
     let mut zip = zip_next::ZipWriter::new(writer);
-    let options = FileOptions::default()
+    let options = SimpleFileOptions::default()
         .compression_method(method)
         .unix_permissions(0o755);
 
+    let prefix = Path::new(prefix);
     let mut buffer = Vec::new();
     for entry in it {
         let path = entry.path();
-        let name = path.strip_prefix(Path::new(prefix)).unwrap();
+        let name = path.strip_prefix(prefix).unwrap();
+        let path_as_string = name
+            .to_str()
+            .map(str::to_owned)
+            .with_context(|| format!("{name:?} Is a Non UTF-8 Path"))?;
 
         // Write file or directory explicitly
         // Some unzip tools unzip files with directory paths correctly, some do not!
         if path.is_file() {
             println!("adding file {path:?} as {name:?} ...");
-            #[allow(deprecated)]
-            zip.start_file_from_path(name, options.clone())?;
+            zip.start_file(path_as_string, options)?;
             let mut f = File::open(path)?;
 
             f.read_to_end(&mut buffer)?;
@@ -98,22 +102,17 @@ where
         } else if !name.as_os_str().is_empty() {
             // Only if not root! Avoids path spec / warning
             // and mapname conversion failed error on unzip
-            println!("adding dir {path:?} as {name:?} ...");
-            #[allow(deprecated)]
-            zip.add_directory_from_path(name, options.clone())?;
+            println!("adding dir {path_as_string:?} as {name:?} ...");
+            zip.add_directory(path_as_string, options)?;
         }
     }
     zip.finish()?;
     Ok(())
 }
 
-fn doit(
-    src_dir: &str,
-    dst_file: &str,
-    method: zip_next::CompressionMethod,
-) -> zip_next::result::ZipResult<()> {
+fn doit(src_dir: &str, dst_file: &str, method: zip_next::CompressionMethod) -> anyhow::Result<()> {
     if !Path::new(src_dir).is_dir() {
-        return Err(ZipError::FileNotFound);
+        return Err(ZipError::FileNotFound.into());
     }
 
     let path = Path::new(dst_file);

examples/write_sample.rs

@@ -1,5 +1,5 @@
 use std::io::prelude::*;
-use zip_next::write::FileOptions;
+use zip_next::write::SimpleFileOptions;
 
 fn main() {
     std::process::exit(real_main());
@@ -27,15 +27,15 @@ fn doit(filename: &str) -> zip_next::result::ZipResult<()> {
 
     let mut zip = zip_next::ZipWriter::new(file);
 
-    zip.add_directory("test/", Default::default())?;
+    zip.add_directory("test/", SimpleFileOptions::default())?;
 
-    let options = FileOptions::default()
+    let options = SimpleFileOptions::default()
         .compression_method(zip_next::CompressionMethod::Stored)
         .unix_permissions(0o755);
     zip.start_file("test/☃.txt", options)?;
     zip.write_all(b"Hello, World!\n")?;
 
-    zip.start_file("test/lorem_ipsum.txt", Default::default())?;
+    zip.start_file("test/lorem_ipsum.txt", options)?;
     zip.write_all(LOREM_IPSUM)?;
 
     zip.finish()?;

fuzz/fuzz_targets/fuzz_write.rs

@@ -1,35 +1,35 @@
 #![no_main]
 
-use std::cell::RefCell;
-use libfuzzer_sys::fuzz_target;
 use arbitrary::Arbitrary;
+use libfuzzer_sys::fuzz_target;
+use std::cell::RefCell;
 use std::io::{Cursor, Read, Seek, Write};
-use std::path::{PathBuf};
+use std::path::PathBuf;
 
-#[derive(Arbitrary,Clone,Debug)]
+#[derive(Arbitrary, Clone, Debug)]
 pub enum BasicFileOperation {
     WriteNormalFile {
         contents: Vec<Vec<u8>>,
-        options: zip_next::write::FileOptions,
+        options: zip_next::write::FullFileOptions,
     },
-    WriteDirectory(zip_next::write::FileOptions),
+    WriteDirectory(zip_next::write::FullFileOptions),
     WriteSymlinkWithTarget {
         target: Box<PathBuf>,
-        options: zip_next::write::FileOptions,
+        options: zip_next::write::FullFileOptions,
     },
     ShallowCopy(Box<FileOperation>),
     DeepCopy(Box<FileOperation>),
 }
 
-#[derive(Arbitrary,Clone,Debug)]
+#[derive(Arbitrary, Clone, Debug)]
 pub struct FileOperation {
     basic: BasicFileOperation,
     name: String,
     reopen: bool,
     // 'abort' flag is separate, to prevent trying to copy an aborted file
 }
 
-#[derive(Arbitrary,Clone,Debug)]
+#[derive(Arbitrary, Clone, Debug)]
 pub struct FuzzTestCase {
     comment: Vec<u8>,
     operations: Vec<(FileOperation, bool)>,
@@ -47,14 +47,25 @@ impl FileOperation {
     }
 }
 
-fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
-                   operation: FileOperation,
-                   abort: bool, flush_on_finish_file: bool) -> Result<(), Box<dyn std::error::Error>>
-                   where T: Read + Write + Seek {
-    writer.borrow_mut().set_flush_on_finish_file(flush_on_finish_file);
+fn do_operation<T>(
+    writer: &mut RefCell<zip_next::ZipWriter<T>>,
+    operation: FileOperation,
+    abort: bool,
+    flush_on_finish_file: bool,
+) -> Result<(), Box<dyn std::error::Error>>
+where
+    T: Read + Write + Seek,
+{
+    writer
+        .borrow_mut()
+        .set_flush_on_finish_file(flush_on_finish_file);
     let name = operation.name;
     match operation.basic {
-        BasicFileOperation::WriteNormalFile {contents, mut options, ..} => {
+        BasicFileOperation::WriteNormalFile {
+            contents,
+            mut options,
+            ..
+        } => {
             let uncompressed_size = contents.iter().map(Vec::len).sum::<usize>();
             if uncompressed_size >= u32::MAX as usize {
                 options = options.large_file(true);
@@ -67,8 +78,10 @@ fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
         BasicFileOperation::WriteDirectory(options) => {
             writer.borrow_mut().add_directory(name, options)?;
         }
-        BasicFileOperation::WriteSymlinkWithTarget {target, options} => {
-            writer.borrow_mut().add_symlink(name, target.to_string_lossy(), options)?;
+        BasicFileOperation::WriteSymlinkWithTarget { target, options } => {
+            writer
+                .borrow_mut()
+                .add_symlink(name, target.to_string_lossy(), options)?;
         }
         BasicFileOperation::ShallowCopy(base) => {
             let base_name = base.referenceable_name();
@@ -86,8 +99,8 @@ fn do_operation<T>(writer: &mut RefCell<zip_next::ZipWriter<T>>,
     }
     if operation.reopen {
         let old_comment = writer.borrow().get_raw_comment().to_owned();
-        let new_writer = zip_next::ZipWriter::new_append(
-            writer.borrow_mut().finish().unwrap()).unwrap();
+        let new_writer =
+            zip_next::ZipWriter::new_append(writer.borrow_mut().finish().unwrap()).unwrap();
         assert_eq!(&old_comment, new_writer.get_raw_comment());
         *writer = new_writer.into();
     }
@@ -98,7 +111,12 @@ fuzz_target!(|test_case: FuzzTestCase| {
     let mut writer = RefCell::new(zip_next::ZipWriter::new(Cursor::new(Vec::new())));
     writer.borrow_mut().set_raw_comment(test_case.comment);
     for (operation, abort) in test_case.operations {
-        let _ = do_operation(&mut writer, operation, abort, test_case.flush_on_finish_file);
+        let _ = do_operation(
+            &mut writer,
+            operation,
+            abort,
+            test_case.flush_on_finish_file,
+        );
     }
     let _ = zip_next::ZipArchive::new(writer.borrow_mut().finish().unwrap());
-});
+});

src/read.rs

@@ -14,6 +14,7 @@ use byteorder::{LittleEndian, ReadBytesExt};
 use std::borrow::{Borrow, Cow};
 use std::collections::HashMap;
 use std::io::{self, prelude::*};
+use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, OnceLock};
 
@@ -828,8 +829,8 @@ fn central_header_to_zip_file_inner<R: Read>(
         uncompressed_size: uncompressed_size as u64,
         file_name,
         file_name_raw: file_name_raw.into(),
-        extra_field: Arc::new(extra_field),
-        central_extra_field: Arc::new(vec![]),
+        extra_field: Some(Arc::new(extra_field)),
+        central_extra_field: None,
         file_comment,
         header_start: offset,
         central_header_start,
@@ -861,9 +862,12 @@ fn central_header_to_zip_file_inner<R: Read>(
 }
 
 fn parse_extra_field(file: &mut ZipFileData) -> ZipResult<()> {
-    let mut reader = io::Cursor::new(file.extra_field.as_ref());
+    let Some(extra_field) = &file.extra_field else {
+        return Ok(());
+    };
+    let mut reader = io::Cursor::new(extra_field.as_ref());
 
-    while (reader.position() as usize) < file.extra_field.len() {
+    while (reader.position() as usize) < extra_field.len() {
         let kind = reader.read_u16::<LittleEndian>()?;
         let len = reader.read_u16::<LittleEndian>()?;
         let mut len_left = len as i64;
@@ -1068,8 +1072,8 @@ impl<'a> ZipFile<'a> {
     }
 
     /// Get the extra data of the zip header for this file
-    pub fn extra_data(&self) -> &[u8] {
-        &self.data.extra_field
+    pub fn extra_data(&self) -> Option<&[u8]> {
+        self.data.extra_field.as_ref().map(|v| v.deref().deref())
     }
 
     /// Get the starting offset of the data of the compressed file
@@ -1115,7 +1119,7 @@ impl<'a> Drop for ZipFile<'a> {
             loop {
                 match reader.read(&mut buffer) {
                     Ok(0) => break,
-                    Ok(_) => (),
+                    Ok(_read) => (),
                     Err(e) => {
                         panic!("Could not consume all of the output of the current ZipFile: {e:?}")
                     }
@@ -1188,8 +1192,8 @@ pub fn read_zipfile_from_stream<'a, R: Read>(reader: &'a mut R) -> ZipResult<Opt
         uncompressed_size: uncompressed_size as u64,
         file_name,
         file_name_raw: file_name_raw.into(),
-        extra_field: Arc::new(extra_field),
-        central_extra_field: Arc::new(vec![]),
+        extra_field: Some(Arc::new(extra_field)),
+        central_extra_field: None,
         file_comment: String::with_capacity(0).into_boxed_str(), // file comment is only available in the central directory
         // header_start and data start are not available, but also don't matter, since seeking is
         // not available.