From 61afe4dad94539804a7c606b001b1bbf9e38a752 Mon Sep 17 00:00:00 2001 From: Wyatt Herkamp Date: Mon, 15 Apr 2024 16:24:42 -0400 Subject: [PATCH] Added ExtendedFileOptions --- Cargo.toml | 32 ++- benches/read_entry.rs | 6 +- benches/read_metadata.rs | 6 +- examples/write_dir.rs | 33 ++-- examples/write_sample.rs | 8 +- fuzz/fuzz_targets/fuzz_write.rs | 60 ++++-- src/read.rs | 22 ++- src/types.rs | 22 ++- src/unstable.rs | 4 +- src/write.rs | 337 ++++++++++++++++++++------------ tests/end_to_end.rs | 14 +- tests/zip_crypto.rs | 2 +- 12 files changed, 344 insertions(+), 202 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d0e3d3299..ef8d16dc5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,8 +1,12 @@ [package] name = "zip_next" version = "1.0.1" -authors = ["Mathijs van de Nes ", "Marli Frost ", "Ryan Levick ", -"Chris Hennick "] +authors = [ + "Mathijs van de Nes ", + "Marli Frost ", + "Ryan Levick ", + "Chris Hennick ", +] 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" diff --git a/benches/read_entry.rs b/benches/read_entry.rs index 4ee20b02f..101f6510f 100644 --- a/benches/read_entry.rs +++ b/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 { 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]; diff --git a/benches/read_metadata.rs b/benches/read_metadata.rs index f9be2ec3b..262137dac 100644 --- a/benches/read_metadata.rs +++ b/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 { 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(); } diff --git a/examples/write_dir.rs b/examples/write_dir.rs index d71808dc5..61f269fc1 100644 --- a/examples/write_dir.rs +++ b/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( 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); diff --git a/examples/write_sample.rs b/examples/write_sample.rs index bb9739d09..700962fd1 100644 --- a/examples/write_sample.rs +++ b/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()?; diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs index b09b7ef90..e1cdf8187 100644 --- a/fuzz/fuzz_targets/fuzz_write.rs +++ b/fuzz/fuzz_targets/fuzz_write.rs @@ -1,27 +1,27 @@ #![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>, - options: zip_next::write::FileOptions, + options: zip_next::write::FullFileOptions, }, - WriteDirectory(zip_next::write::FileOptions), + WriteDirectory(zip_next::write::FullFileOptions), WriteSymlinkWithTarget { target: Box, - options: zip_next::write::FileOptions, + options: zip_next::write::FullFileOptions, }, ShallowCopy(Box), DeepCopy(Box), } -#[derive(Arbitrary,Clone,Debug)] +#[derive(Arbitrary, Clone, Debug)] pub struct FileOperation { basic: BasicFileOperation, name: String, @@ -29,7 +29,7 @@ pub struct FileOperation { // '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, operations: Vec<(FileOperation, bool)>, @@ -47,14 +47,25 @@ impl FileOperation { } } -fn do_operation(writer: &mut RefCell>, - operation: FileOperation, - abort: bool, flush_on_finish_file: bool) -> Result<(), Box> - where T: Read + Write + Seek { - writer.borrow_mut().set_flush_on_finish_file(flush_on_finish_file); +fn do_operation( + writer: &mut RefCell>, + operation: FileOperation, + abort: bool, + flush_on_finish_file: bool, +) -> Result<(), Box> +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::(); if uncompressed_size >= u32::MAX as usize { options = options.large_file(true); @@ -67,8 +78,10 @@ fn do_operation(writer: &mut RefCell>, 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(writer: &mut RefCell>, } 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()); -}); \ No newline at end of file +}); diff --git a/src/read.rs b/src/read.rs index 698f01496..fe5c37d19 100644 --- a/src/read.rs +++ b/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( 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( } 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::()?; let len = reader.read_u16::()?; 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, /// Extra field usually used for storage expansion - pub extra_field: Arc>, + pub extra_field: Option>>, /// Extra field only written to central directory - pub central_extra_field: Arc>, + pub central_extra_field: Option>>, /// File comment pub file_comment: Box, /// Specifies where the local header of the file starts @@ -458,6 +458,20 @@ impl ZipFileData { _ => 20, } } + #[inline(always)] + pub(crate) fn extra_field_len(&self) -> usize { + self.extra_field + .as_ref() + .map(|v| v.len()) + .unwrap_or_default() + } + #[inline(always)] + pub(crate) fn central_extra_field_len(&self) -> usize { + self.central_extra_field + .as_ref() + .map(|v| v.len()) + .unwrap_or_default() + } } /// The encryption specification used to encrypt a file with AES. @@ -521,8 +535,8 @@ mod test { uncompressed_size: 0, file_name: file_name.clone().into_boxed_str(), file_name_raw: file_name.into_bytes().into_boxed_slice(), - extra_field: Arc::new(vec![]), - central_extra_field: Arc::new(vec![]), + extra_field: None, + central_extra_field: None, file_comment: String::with_capacity(0).into_boxed_str(), header_start: 0, data_start: OnceLock::new(), diff --git a/src/unstable.rs b/src/unstable.rs index cc03ff9a0..beb35a414 100644 --- a/src/unstable.rs +++ b/src/unstable.rs @@ -4,7 +4,7 @@ pub mod stream { } /// Types for creating ZIP archives. pub mod write { - use crate::write::FileOptions; + use crate::write::{FileOptionExtension, FileOptions}; /// Unstable methods for [`FileOptions`]. pub trait FileOptionsExt { /// Write the file with the given password using the deprecated ZipCrypto algorithm. @@ -12,7 +12,7 @@ pub mod write { /// This is not recommended for new archives, as ZipCrypto is not secure. fn with_deprecated_encryption(self, password: &[u8]) -> Self; } - impl FileOptionsExt for FileOptions { + impl FileOptionsExt for FileOptions { fn with_deprecated_encryption(self, password: &[u8]) -> Self { self.with_deprecated_encryption(password) } diff --git a/src/write.rs b/src/write.rs index 7196bf6ab..4f9966ff4 100644 --- a/src/write.rs +++ b/src/write.rs @@ -100,13 +100,13 @@ pub(crate) mod zip_writer { /// # { /// # use zip_next::ZipWriter; /// use std::io::Write; - /// use zip_next::write::FileOptions; + /// use zip_next::write::SimpleFileOptions; /// /// // We use a buffer here, though you'd normally use a `File` /// let mut buf = [0; 65536]; /// let mut zip = ZipWriter::new(std::io::Cursor::new(&mut buf[..])); /// - /// let options = FileOptions::default().compression_method(zip_next::CompressionMethod::Stored); + /// let options = SimpleFileOptions::default().compression_method(zip_next::CompressionMethod::Stored); /// zip.start_file("hello_world.txt", options)?; /// zip.write(b"Hello, World!")?; /// @@ -129,6 +129,8 @@ pub(crate) mod zip_writer { pub(super) flush_on_finish_file: bool, } } +#[doc(inline)] +pub use self::sealed::FileOptionExtension; use crate::result::ZipError::InvalidArchive; #[cfg(feature = "lzma")] use crate::result::ZipError::UnsupportedArchive; @@ -149,38 +151,80 @@ struct ZipRawValues { compressed_size: u64, uncompressed_size: u64, } +mod sealed { + use std::sync::Arc; + + use super::ExtendedFileOptions; + + pub trait Sealed {} + /// File options Extensions + #[doc(hidden)] + pub trait FileOptionExtension: Default + Sealed { + /// Extra Data + fn extra_data(&self) -> Option<&Arc>>; + /// Central Extra Data + fn central_extra_data(&self) -> Option<&Arc>>; + } + impl Sealed for () {} + impl FileOptionExtension for () { + fn extra_data(&self) -> Option<&Arc>> { + None + } + fn central_extra_data(&self) -> Option<&Arc>> { + None + } + } + impl Sealed for ExtendedFileOptions {} + + impl FileOptionExtension for ExtendedFileOptions { + fn extra_data(&self) -> Option<&Arc>> { + Some(&self.extra_data) + } + fn central_extra_data(&self) -> Option<&Arc>> { + Some(&self.central_extra_data) + } + } +} /// Metadata for a file to be written -#[derive(Clone, Debug)] -pub struct FileOptions { +#[derive(Clone, Debug, Copy)] +pub struct FileOptions { pub(crate) compression_method: CompressionMethod, pub(crate) compression_level: Option, pub(crate) last_modified_time: DateTime, pub(crate) permissions: Option, pub(crate) large_file: bool, encrypt_with: Option, - extra_data: Arc>, - central_extra_data: Arc>, + extended_options: T, alignment: u16, #[cfg(feature = "deflate-zopfli")] pub(super) zopfli_buffer_size: Option, } +/// Simple File Options. Can be copied and good for simple writing zip files +pub type SimpleFileOptions = FileOptions<()>; +/// Adds Extra Data and Central Extra Data. It does not implement copy. +pub type FullFileOptions = FileOptions; +/// The Extension for Extra Data and Central Extra Data +#[derive(Clone, Debug, Default)] +pub struct ExtendedFileOptions { + extra_data: Arc>, + central_extra_data: Arc>, +} #[cfg(fuzzing)] -impl arbitrary::Arbitrary<'_> for FileOptions { +impl arbitrary::Arbitrary<'_> for FileOptions { fn arbitrary(u: &mut arbitrary::Unstructured) -> arbitrary::Result { - let mut options = FileOptions { + let mut options = FullFileOptions { compression_method: CompressionMethod::arbitrary(u)?, compression_level: None, last_modified_time: DateTime::arbitrary(u)?, permissions: Option::::arbitrary(u)?, large_file: bool::arbitrary(u)?, encrypt_with: Option::::arbitrary(u)?, - extra_data: Arc::new(vec![]), - central_extra_data: Arc::new(vec![]), alignment: u16::arbitrary(u)?, #[cfg(feature = "deflate-zopfli")] zopfli_buffer_size: None, + ..Default::default() }; match options.compression_method { #[cfg(feature = "deflate-zopfli")] @@ -218,7 +262,7 @@ impl arbitrary::Arbitrary<'_> for FileOptions { } } -impl FileOptions { +impl FileOptions { /// Set the compression method for the new file /// /// The default is `CompressionMethod::Deflated` if it is enabled. If not, @@ -226,7 +270,7 @@ impl FileOptions { /// is enabled, `CompressionMethod::Zlib` is the default. If all else fails, /// `CompressionMethod::Stored` becomes the default and files are written uncompressed. #[must_use] - pub const fn compression_method(mut self, method: CompressionMethod) -> FileOptions { + pub const fn compression_method(mut self, method: CompressionMethod) -> Self { self.compression_method = method; self } @@ -242,7 +286,7 @@ impl FileOptions { /// * `Zstd`: -7 - 22, with zero being mapped to default level. Default is 3 /// * others: only `None` is allowed #[must_use] - pub const fn compression_level(mut self, level: Option) -> FileOptions { + pub const fn compression_level(mut self, level: Option) -> Self { self.compression_level = level; self } @@ -252,7 +296,7 @@ impl FileOptions { /// The default is the current timestamp if the 'time' feature is enabled, and 1980-01-01 /// otherwise #[must_use] - pub const fn last_modified_time(mut self, mod_time: DateTime) -> FileOptions { + pub const fn last_modified_time(mut self, mod_time: DateTime) -> Self { self.last_modified_time = mod_time; self } @@ -267,7 +311,7 @@ impl FileOptions { /// higher file mode bits. So it cannot be used to denote an entry as a directory, /// symlink, or other special file type. #[must_use] - pub const fn unix_permissions(mut self, mode: u32) -> FileOptions { + pub const fn unix_permissions(mut self, mode: u32) -> Self { self.permissions = Some(mode & 0o777); self } @@ -278,15 +322,38 @@ impl FileOptions { /// aborted. If set to `true`, readers will require ZIP64 support and if the file does not /// exceed the limit, 20 B are wasted. The default is `false`. #[must_use] - pub const fn large_file(mut self, large: bool) -> FileOptions { + pub const fn large_file(mut self, large: bool) -> Self { self.large_file = large; self } - pub(crate) fn with_deprecated_encryption(mut self, password: &[u8]) -> FileOptions { + pub(crate) fn with_deprecated_encryption(mut self, password: &[u8]) -> Self { self.encrypt_with = Some(ZipCryptoKeys::derive(password)); self } + /// Sets the size of the buffer used to hold the next block that Zopfli will compress. The + /// larger the buffer, the more effective the compression, but the more memory is required. + /// A value of `None` indicates no buffer, which is recommended only when all non-empty writes + /// are larger than about 32 KiB. + #[must_use] + #[cfg(feature = "deflate-zopfli")] + pub const fn with_zopfli_buffer(mut self, size: Option) -> Self { + self.zopfli_buffer_size = size; + self + } + + /// Returns the compression level currently set. + pub const fn get_compression_level(&self) -> Option { + self.compression_level + } + /// Sets the alignment to the given number of bytes. + #[must_use] + pub const fn with_alignment(mut self, alignment: u16) -> Self { + self.alignment = alignment; + self + } +} +impl FileOptions { /// Adds an extra data field. pub fn add_extra_data( &mut self, @@ -296,15 +363,19 @@ impl FileOptions { ) -> ZipResult<()> { validate_extra_data(header_id, data)?; let len = data.len() + 4; - if self.extra_data.len() + self.central_extra_data.len() + len > u16::MAX as usize { + if self.extended_options.extra_data.len() + + self.extended_options.central_extra_data.len() + + len + > u16::MAX as usize + { Err(InvalidArchive( "Extra data field would be longer than allowed", )) } else { let field = if central_only { - &mut self.central_extra_data + &mut self.extended_options.central_extra_data } else { - &mut self.extra_data + &mut self.extended_options.extra_data }; let vec = Arc::get_mut(field); let vec = match vec { @@ -324,41 +395,17 @@ impl FileOptions { /// Removes the extra data fields. #[must_use] - pub fn clear_extra_data(mut self) -> FileOptions { - if self.extra_data.len() > 0 { - self.extra_data = Arc::new(vec![]); + pub fn clear_extra_data(mut self) -> Self { + if self.extended_options.extra_data.len() > 0 { + self.extended_options.extra_data = Arc::new(vec![]); } - if self.central_extra_data.len() > 0 { - self.central_extra_data = Arc::new(vec![]); + if self.extended_options.central_extra_data.len() > 0 { + self.extended_options.central_extra_data = Arc::new(vec![]); } self } - - /// Sets the alignment to the given number of bytes. - #[must_use] - pub const fn with_alignment(mut self, alignment: u16) -> FileOptions { - self.alignment = alignment; - self - } - - /// Sets the size of the buffer used to hold the next block that Zopfli will compress. The - /// larger the buffer, the more effective the compression, but the more memory is required. - /// A value of `None` indicates no buffer, which is recommended only when all non-empty writes - /// are larger than about 32 KiB. - #[must_use] - #[cfg(feature = "deflate-zopfli")] - pub const fn with_zopfli_buffer(mut self, size: Option) -> FileOptions { - self.zopfli_buffer_size = size; - self - } - - /// Returns the compression level currently set. - pub const fn get_compression_level(&self) -> Option { - self.compression_level - } } - -impl Default for FileOptions { +impl Default for FileOptions { /// Construct a new FileOptions object fn default() -> Self { Self { @@ -371,8 +418,7 @@ impl Default for FileOptions { permissions: None, large_file: false, encrypt_with: None, - extra_data: Arc::new(vec![]), - central_extra_data: Arc::new(vec![]), + extended_options: T::default(), alignment: 1, #[cfg(feature = "deflate-zopfli")] zopfli_buffer_size: Some(1 << 15), @@ -481,23 +527,7 @@ impl ZipWriter { let compressed_size = src_data.compressed_size; debug_assert!(compressed_size <= write_position - data_start); let uncompressed_size = src_data.uncompressed_size; - let mut options = FileOptions { - compression_method: src_data.compression_method, - compression_level: src_data.compression_level, - last_modified_time: src_data.last_modified_time, - permissions: src_data.unix_mode(), - large_file: src_data.large_file, - encrypt_with: None, - extra_data: src_data.extra_field.clone(), - central_extra_data: src_data.central_extra_field.clone(), - alignment: 1, - #[cfg(feature = "deflate-zopfli")] - zopfli_buffer_size: None, - }; - if let Some(perms) = src_data.unix_mode() { - options = options.unix_permissions(perms); - } - Self::normalize_options(&mut options); + let raw_values = ZipRawValues { crc32: src_data.crc32, compressed_size, @@ -513,7 +543,47 @@ impl ZipWriter { self.inner .get_plain() .seek(SeekFrom::Start(write_position))?; - self.start_entry(dest_name, options, Some(raw_values))?; + if src_data.extra_field.is_some() || src_data.central_extra_field.is_some() { + let mut options = FileOptions:: { + compression_method: src_data.compression_method, + compression_level: src_data.compression_level, + last_modified_time: src_data.last_modified_time, + permissions: src_data.unix_mode(), + large_file: src_data.large_file, + encrypt_with: None, + extended_options: ExtendedFileOptions { + extra_data: src_data.extra_field.clone().unwrap_or_default(), + central_extra_data: src_data.central_extra_field.clone().unwrap_or_default(), + }, + alignment: 1, + #[cfg(feature = "deflate-zopfli")] + zopfli_buffer_size: None, + }; + if let Some(perms) = src_data.unix_mode() { + options = options.unix_permissions(perms); + } + Self::normalize_options(&mut options); + self.start_entry(dest_name, options, Some(raw_values))?; + } else { + let mut options = FileOptions::<()> { + compression_method: src_data.compression_method, + compression_level: src_data.compression_level, + last_modified_time: src_data.last_modified_time, + permissions: src_data.unix_mode(), + large_file: src_data.large_file, + encrypt_with: None, + extended_options: (), + alignment: 1, + #[cfg(feature = "deflate-zopfli")] + zopfli_buffer_size: None, + }; + if let Some(perms) = src_data.unix_mode() { + options = options.unix_permissions(perms); + } + Self::normalize_options(&mut options); + self.start_entry(dest_name, options, Some(raw_values))?; + } + self.writing_to_file = true; self.writing_raw = true; if let Err(e) = self.write_all(©) { @@ -578,10 +648,10 @@ impl ZipWriter { } /// Start a new file for with the requested options. - fn start_entry( + fn start_entry( &mut self, name: S, - options: FileOptions, + options: FileOptions, raw_values: Option, ) -> ZipResult<()> where @@ -612,8 +682,8 @@ impl ZipWriter { uncompressed_size: raw_values.uncompressed_size, file_name: name.into(), file_name_raw: vec![].into_boxed_slice(), // Never used for saving - extra_field: options.extra_data, - central_extra_field: options.central_extra_data, + extra_field: options.extended_options.extra_data().cloned(), + central_extra_field: options.extended_options.central_extra_data().cloned(), file_comment: String::with_capacity(0).into_boxed_str(), header_start, data_start: OnceLock::new(), @@ -654,11 +724,11 @@ impl ZipWriter { // file name length writer.write_u16::(file.file_name.as_bytes().len() as u16)?; // extra field length - let mut extra_field_length = file.extra_field.len(); + let mut extra_field_length = file.extra_field_len(); if file.large_file { extra_field_length += 20; } - if extra_field_length + file.central_extra_field.len() > u16::MAX as usize { + if extra_field_length + file.central_extra_field_len() > u16::MAX as usize { let _ = self.abort_file(); return Err(InvalidArchive("Extra data field is too large")); } @@ -670,7 +740,9 @@ impl ZipWriter { if file.large_file { write_local_zip64_extra_field(writer, file)?; } - writer.write_all(&file.extra_field)?; + if let Some(extra_field) = &file.extra_field { + writer.write_all(extra_field)?; + } let mut header_end = writer.stream_position()?; if options.alignment > 1 { let align = options.alignment as u64; @@ -822,7 +894,11 @@ impl ZipWriter { /// same name as a file already in the archive. /// /// The data should be written using the [`Write`] implementation on this [`ZipWriter`] - pub fn start_file(&mut self, name: S, mut options: FileOptions) -> ZipResult<()> + pub fn start_file( + &mut self, + name: S, + mut options: FileOptions, + ) -> ZipResult<()> where S: Into>, { @@ -842,12 +918,12 @@ impl ZipWriter { Ok(()) } - fn normalize_options(options: &mut FileOptions) { + fn normalize_options(options: &mut FileOptions) { if options.permissions.is_none() { options.permissions = Some(0o644); } if !options.last_modified_time.is_valid() { - options.last_modified_time = FileOptions::default().last_modified_time; + options.last_modified_time = FileOptions::::default().last_modified_time; } *options.permissions.as_mut().unwrap() |= ffi::S_IFREG; } @@ -860,10 +936,10 @@ impl ZipWriter { since = "0.5.7", note = "by stripping `..`s from the path, the meaning of paths can change. Use `start_file` instead." )] - pub fn start_file_from_path( + pub fn start_file_from_path( &mut self, path: &std::path::Path, - options: FileOptions, + options: FileOptions, ) -> ZipResult<()> { self.start_file(path_to_string(path), options) } @@ -898,7 +974,7 @@ impl ZipWriter { where S: Into>, { - let mut options = FileOptions::default() + let mut options = SimpleFileOptions::default() .large_file(file.compressed_size().max(file.size()) > spec::ZIP64_BYTES_THR) .last_modified_time(file.last_modified()) .compression_method(file.compression()); @@ -953,7 +1029,11 @@ impl ZipWriter { /// Add a directory entry. /// /// As directories have no content, you must not call [`ZipWriter::write`] before adding a new file. - pub fn add_directory(&mut self, name: S, mut options: FileOptions) -> ZipResult<()> + pub fn add_directory( + &mut self, + name: S, + mut options: FileOptions, + ) -> ZipResult<()> where S: Into, { @@ -984,10 +1064,10 @@ impl ZipWriter { since = "0.5.7", note = "by stripping `..`s from the path, the meaning of paths can change. Use `add_directory` instead." )] - pub fn add_directory_from_path( + pub fn add_directory_from_path( &mut self, path: &std::path::Path, - options: FileOptions, + options: FileOptions, ) -> ZipResult<()> { self.add_directory(path_to_string(path), options) } @@ -1014,11 +1094,11 @@ impl ZipWriter { /// implementations may materialize a symlink as a regular file, possibly with the /// content incorrectly set to the symlink target. For maximum portability, consider /// storing a regular file instead. - pub fn add_symlink( + pub fn add_symlink( &mut self, name: N, target: T, - mut options: FileOptions, + mut options: FileOptions, ) -> ZipResult<()> where N: Into>, @@ -1471,8 +1551,8 @@ fn write_central_directory_header(writer: &mut T, file: &ZipFileData) // extra field length writer.write_u16::( zip64_extra_field_length - + file.extra_field.len() as u16 - + file.central_extra_field.len() as u16, + + file.extra_field_len() as u16 + + file.central_extra_field_len() as u16, )?; // file comment length writer.write_u16::(0)?; @@ -1489,8 +1569,12 @@ fn write_central_directory_header(writer: &mut T, file: &ZipFileData) // zip64 extra field writer.write_all(&zip64_extra_field[..zip64_extra_field_length as usize])?; // extra field - writer.write_all(&file.extra_field)?; - writer.write_all(&file.central_extra_field)?; + if let Some(extra_field) = &file.extra_field { + writer.write_all(extra_field)?; + } + if let Some(central_extra_field) = &file.central_extra_field { + writer.write_all(central_extra_field)?; + } // file comment // @@ -1621,10 +1705,10 @@ mod test { use crate::compression::CompressionMethod; use crate::result::ZipResult; use crate::types::DateTime; + use crate::write::SimpleFileOptions; use crate::ZipArchive; use std::io; use std::io::{Read, Write}; - use std::sync::Arc; #[test] fn write_empty_zip() { @@ -1641,7 +1725,7 @@ mod test { #[test] fn unix_permissions_bitmask() { // unix_permissions() throws away upper bits. - let options = FileOptions::default().unix_permissions(0o120777); + let options = SimpleFileOptions::default().unix_permissions(0o120777); assert_eq!(options.permissions, Some(0o777)); } @@ -1651,7 +1735,7 @@ mod test { writer .add_directory( "test", - FileOptions::default().last_modified_time( + SimpleFileOptions::default().last_modified_time( DateTime::from_date_and_time(2018, 8, 15, 20, 45, 6).unwrap(), ), ) @@ -1680,7 +1764,7 @@ mod test { .add_symlink( "name", "target", - FileOptions::default().last_modified_time( + SimpleFileOptions::default().last_modified_time( DateTime::from_date_and_time(2018, 8, 15, 20, 45, 6).unwrap(), ), ) @@ -1709,7 +1793,7 @@ mod test { .add_symlink( "directory\\link", "/absolute/symlink\\with\\mixed/slashes", - FileOptions::default().last_modified_time( + SimpleFileOptions::default().last_modified_time( DateTime::from_date_and_time(2018, 8, 15, 20, 45, 6).unwrap(), ), ) @@ -1744,8 +1828,7 @@ mod test { permissions: Some(33188), large_file: false, encrypt_with: None, - extra_data: Arc::new(vec![]), - central_extra_data: Arc::new(vec![]), + extended_options: (), alignment: 1, #[cfg(feature = "deflate-zopfli")] zopfli_buffer_size: None, @@ -1786,8 +1869,7 @@ mod test { permissions: Some(33188), large_file: false, encrypt_with: None, - extra_data: Arc::new(vec![]), - central_extra_data: Arc::new(vec![]), + extended_options: (), alignment: 0, #[cfg(feature = "deflate-zopfli")] zopfli_buffer_size: None, @@ -1838,8 +1920,7 @@ mod test { permissions: Some(33188), large_file: false, encrypt_with: None, - extra_data: Arc::new(vec![]), - central_extra_data: Arc::new(vec![]), + extended_options: (), alignment: 0, #[cfg(feature = "deflate-zopfli")] zopfli_buffer_size: None, @@ -1881,13 +1962,13 @@ mod test { fn duplicate_filenames() { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer - .start_file("foo/bar/test", FileOptions::default()) + .start_file("foo/bar/test", SimpleFileOptions::default()) .unwrap(); writer .write_all("The quick brown 🦊 jumps over the lazy 🐕".as_bytes()) .unwrap(); writer - .start_file("foo/bar/test", FileOptions::default()) + .start_file("foo/bar/test", SimpleFileOptions::default()) .expect_err("Expected duplicate filename not to be allowed"); } @@ -1897,7 +1978,7 @@ mod test { writer .start_file( "PK\u{6}\u{7}\0\0\0\u{11}\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -1910,7 +1991,7 @@ mod test { writer .start_file( "PK\u{6}\u{6}\0\0\0\0\0\0\0\0\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -1924,7 +2005,7 @@ mod test { writer .start_file( "PK\u{6}\u{6}PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -1936,12 +2017,12 @@ mod test { fn test_filename_looks_like_zip64_locator_3() { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer - .start_file("\0PK\u{6}\u{6}", FileOptions::default()) + .start_file("\0PK\u{6}\u{6}", SimpleFileOptions::default()) .unwrap(); writer .start_file( "\0\u{4}\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{3}", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -1953,18 +2034,22 @@ mod test { fn test_filename_looks_like_zip64_locator_4() { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer - .start_file("PK\u{6}\u{6}", FileOptions::default()) + .start_file("PK\u{6}\u{6}", SimpleFileOptions::default()) + .unwrap(); + writer + .start_file("\0\0\0\0\0\0", SimpleFileOptions::default()) + .unwrap(); + writer + .start_file("\0", SimpleFileOptions::default()) .unwrap(); + writer.start_file("", SimpleFileOptions::default()).unwrap(); writer - .start_file("\0\0\0\0\0\0", FileOptions::default()) + .start_file("\0\0", SimpleFileOptions::default()) .unwrap(); - writer.start_file("\0", FileOptions::default()).unwrap(); - writer.start_file("", FileOptions::default()).unwrap(); - writer.start_file("\0\0", FileOptions::default()).unwrap(); writer .start_file( "\0\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -1976,7 +2061,7 @@ mod test { fn test_filename_looks_like_zip64_locator_5() -> ZipResult<()> { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer - .add_directory("", FileOptions::default().with_alignment(21)) + .add_directory("", SimpleFileOptions::default().with_alignment(21)) .unwrap(); let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); writer.shallow_copy_file("/", "").unwrap(); @@ -1984,13 +2069,13 @@ mod test { writer.shallow_copy_file("\0", "PK\u{6}\u{6}").unwrap(); let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); writer - .start_file("\0\0\0\0\0\0", FileOptions::default()) + .start_file("\0\0\0\0\0\0", SimpleFileOptions::default()) .unwrap(); let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); writer .start_file( "#PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", - FileOptions::default(), + SimpleFileOptions::default(), ) .unwrap(); let zip = writer.finish().unwrap(); @@ -2003,7 +2088,7 @@ mod test { fn remove_shallow_copy_keeps_original() -> ZipResult<()> { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer - .start_file("original", FileOptions::default()) + .start_file("original", SimpleFileOptions::default()) .unwrap(); writer.write_all(RT_TEST_TEXT.as_bytes()).unwrap(); writer @@ -2021,20 +2106,20 @@ mod test { #[test] fn remove_encrypted_file() -> ZipResult<()> { let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - let first_file_options = FileOptions::default() + let first_file_options = SimpleFileOptions::default() .with_alignment(65535) .with_deprecated_encryption(b"Password"); writer.start_file("", first_file_options).unwrap(); writer.abort_file().unwrap(); let zip = writer.finish().unwrap(); let mut writer = ZipWriter::new(zip); - writer.start_file("", FileOptions::default()).unwrap(); + writer.start_file("", SimpleFileOptions::default()).unwrap(); Ok(()) } #[test] fn remove_encrypted_aligned_symlink() -> ZipResult<()> { - let mut options = FileOptions::default(); + let mut options = SimpleFileOptions::default(); options = options.with_deprecated_encryption(b"Password"); options.alignment = 65535; let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); @@ -2043,14 +2128,14 @@ mod test { let zip = writer.finish().unwrap(); println!("{:0>2x?}", zip.get_ref()); let mut writer = ZipWriter::new_append(zip).unwrap(); - writer.start_file("", FileOptions::default()).unwrap(); + writer.start_file("", SimpleFileOptions::default()).unwrap(); Ok(()) } #[cfg(feature = "deflate-zopfli")] #[test] fn zopfli_empty_write() -> ZipResult<()> { - let mut options = FileOptions::default(); + let mut options = SimpleFileOptions::default(); options = options .compression_method(CompressionMethod::default()) .compression_level(Some(264)); @@ -2065,7 +2150,7 @@ mod test { fn crash_with_no_features() -> ZipResult<()> { const ORIGINAL_FILE_NAME: &str = "PK\u{6}\u{6}\0\0\0\0\0\0\0\0\0\u{2}g\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\u{1}\0\0\0\0\0\0\0\0\0\0PK\u{6}\u{7}\0\0\0\0\0\0\0\0\0\0\0\0\u{7}\0\t'"; let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); - let mut options = FileOptions::default(); + let mut options = SimpleFileOptions::default(); options = options .with_alignment(3584) .compression_method(CompressionMethod::Stored); diff --git a/tests/end_to_end.rs b/tests/end_to_end.rs index a1b9eebce..f8cadef4d 100644 --- a/tests/end_to_end.rs +++ b/tests/end_to_end.rs @@ -3,7 +3,9 @@ use std::collections::HashSet; use std::io::prelude::*; use std::io::Cursor; use zip_next::result::ZipResult; +use zip_next::write::ExtendedFileOptions; use zip_next::write::FileOptions; +use zip_next::write::SimpleFileOptions; use zip_next::{CompressionMethod, ZipWriter, SUPPORTED_COMPRESSION_METHODS}; // This test asserts that after creating a zip file, then reading its contents back out, @@ -84,7 +86,7 @@ fn append() { let mut zip = ZipWriter::new_append(&mut file).unwrap(); zip.start_file( COPY_ENTRY_NAME, - FileOptions::default() + SimpleFileOptions::default() .compression_method(method) .unix_permissions(0o755), ) @@ -105,9 +107,10 @@ fn append() { fn write_test_archive(file: &mut Cursor>, method: CompressionMethod, shallow_copy: bool) { let mut zip = ZipWriter::new(file); - zip.add_directory("test/", Default::default()).unwrap(); + zip.add_directory("test/", SimpleFileOptions::default()) + .unwrap(); - let mut options = FileOptions::default() + let mut options = FileOptions::::default() .compression_method(method) .unix_permissions(0o755); @@ -159,7 +162,10 @@ fn check_test_archive(zip_file: R) -> ZipResult(0xbeef)?; extra_data.write_u16::(EXTRA_DATA.len() as u16)?; extra_data.write_all(EXTRA_DATA)?; - assert_eq!(file_with_extra_data.extra_data(), extra_data.as_slice()); + assert_eq!( + file_with_extra_data.extra_data(), + Some(extra_data.as_slice()) + ); } Ok(archive) diff --git a/tests/zip_crypto.rs b/tests/zip_crypto.rs index 7220dc0c4..d579c93f1 100644 --- a/tests/zip_crypto.rs +++ b/tests/zip_crypto.rs @@ -29,7 +29,7 @@ fn encrypting_file() { archive .start_file( "name", - zip_next::write::FileOptions::default().with_deprecated_encryption(b"password"), + zip_next::write::SimpleFileOptions::default().with_deprecated_encryption(b"password"), ) .unwrap(); archive.write_all(b"test").unwrap();