From 255cfaf26105de0d647410c6fd31d4fb6457d66c Mon Sep 17 00:00:00 2001 From: Chris Hennick Date: Fri, 26 May 2023 17:22:45 -0700 Subject: [PATCH] Add flush_on_finish_file parameter --- CHANGELOG.md | 8 +++- Cargo.toml | 2 +- README.md | 6 +-- benches/read_entry.rs | 2 +- benches/read_metadata.rs | 2 +- examples/write_dir.rs | 2 +- examples/write_sample.rs | 2 +- fuzz/fuzz_targets/fuzz_write.rs | 32 ++++++++------ src/write.rs | 76 ++++++++++++++++++++------------- tests/end_to_end.rs | 6 +-- tests/zip_crypto.rs | 2 +- 11 files changed, 86 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac7b7924..f033712d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -178,4 +178,10 @@ ### Added - - `ZipWriter` methods `get_comment()` and `get_raw_comment()`. \ No newline at end of file + - `ZipWriter` methods `get_comment()` and `get_raw_comment()`. + +## [0.9.0] + +### Added + + - `flush_on_finish_file` parameter for `ZipWriter`. \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml index 94037d24..12395874 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zip_next" -version = "0.8.3" +version = "0.9.0" authors = ["Mathijs van de Nes ", "Marli Frost ", "Ryan Levick ", "Chris Hennick "] license = "MIT" diff --git a/README.md b/README.md index 6446e45f..f8aa7671 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ zip_next [![Build Status](https://github.com/Pr0methean/zip-next/actions/workflows/ci.yaml/badge.svg)](https://github.com/Pr0methean/zip-next/actions?query=branch%3Amaster+workflow%3ACI) [![Crates.io version](https://img.shields.io/crates/v/zip_next.svg)](https://crates.io/crates/zip_next) -[Documentation](https://docs.rs/zip_next/0.8.3/zip_next/) +[Documentation](https://docs.rs/zip_next/0.9.0/zip_next/) Info ---- @@ -32,14 +32,14 @@ With all default features: ```toml [dependencies] -zip_next = "0.8.3" +zip_next = "0.9.0" ``` Without the default features: ```toml [dependencies] -zip_next = { version = "0.8.3", default-features = false } +zip_next = { version = "0.9.0", default-features = false } ``` The features available are: diff --git a/benches/read_entry.rs b/benches/read_entry.rs index 4ee20b02..dd4bced7 100644 --- a/benches/read_entry.rs +++ b/benches/read_entry.rs @@ -8,7 +8,7 @@ use zip_next::{ZipArchive, ZipWriter}; fn generate_random_archive(size: usize) -> Vec { let data = Vec::new(); - let mut writer = ZipWriter::new(Cursor::new(data)); + let mut writer = ZipWriter::new(Cursor::new(data), false); let options = zip_next::write::FileOptions::default() .compression_method(zip_next::CompressionMethod::Stored); diff --git a/benches/read_metadata.rs b/benches/read_metadata.rs index f9be2ec3..eee2d713 100644 --- a/benches/read_metadata.rs +++ b/benches/read_metadata.rs @@ -11,7 +11,7 @@ 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 mut writer = ZipWriter::new(Cursor::new(data), false); let options = FileOptions::default().compression_method(CompressionMethod::Stored); let bytes = vec![0u8; file_size]; diff --git a/examples/write_dir.rs b/examples/write_dir.rs index f0d9efcd..61535c34 100644 --- a/examples/write_dir.rs +++ b/examples/write_dir.rs @@ -73,7 +73,7 @@ fn zip_dir( where T: Write + Seek, { - let mut zip = zip_next::ZipWriter::new(writer); + let mut zip = zip_next::ZipWriter::new(writer, false); let options = FileOptions::default() .compression_method(method) .unix_permissions(0o755); diff --git a/examples/write_sample.rs b/examples/write_sample.rs index bb9739d0..0834d473 100644 --- a/examples/write_sample.rs +++ b/examples/write_sample.rs @@ -25,7 +25,7 @@ fn doit(filename: &str) -> zip_next::result::ZipResult<()> { let path = std::path::Path::new(filename); let file = std::fs::File::create(path).unwrap(); - let mut zip = zip_next::ZipWriter::new(file); + let mut zip = zip_next::ZipWriter::new(file, false); zip.add_directory("test/", Default::default())?; diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs index 580105f6..6961e9df 100644 --- a/fuzz/fuzz_targets/fuzz_write.rs +++ b/fuzz/fuzz_targets/fuzz_write.rs @@ -6,7 +6,7 @@ use arbitrary::Arbitrary; use std::io::{Cursor, Read, Seek, Write}; use std::path::{PathBuf}; -#[derive(Arbitrary,Debug)] +#[derive(Arbitrary,Clone,Debug)] pub enum BasicFileOperation { WriteNormalFile { contents: Vec>, @@ -21,7 +21,7 @@ pub enum BasicFileOperation { DeepCopy(Box), } -#[derive(Arbitrary,Debug)] +#[derive(Arbitrary,Clone,Debug)] pub struct FileOperation { basic: BasicFileOperation, name: String, @@ -29,6 +29,13 @@ pub struct FileOperation { // 'abort' flag is separate, to prevent trying to copy an aborted file } +#[derive(Arbitrary,Clone,Debug)] +pub struct FuzzTestCase { + comment: Vec, + operations: Vec<(FileOperation, bool)>, + flush_on_finish_file: bool, +} + impl FileOperation { fn referenceable_name(&self) -> String { if let BasicFileOperation::WriteDirectory(_) = self.basic { @@ -42,7 +49,7 @@ impl FileOperation { fn do_operation(writer: &mut RefCell>, operation: FileOperation, - abort: bool) -> Result<(), Box> + abort: bool, flush_on_finish_file: bool) -> Result<(), Box> where T: Read + Write + Seek { let name = operation.name; match operation.basic { @@ -63,12 +70,12 @@ fn do_operation(writer: &mut RefCell>, } BasicFileOperation::ShallowCopy(base) => { let base_name = base.referenceable_name(); - do_operation(writer, *base, false)?; + do_operation(writer, *base, false, flush_on_finish_file)?; writer.borrow_mut().shallow_copy_file(&base_name, &name)?; } BasicFileOperation::DeepCopy(base) => { let base_name = base.referenceable_name(); - do_operation(writer, *base, false)?; + do_operation(writer, *base, false, flush_on_finish_file)?; writer.borrow_mut().deep_copy_file(&base_name, &name)?; } } @@ -77,19 +84,20 @@ 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(), flush_on_finish_file).unwrap(); assert_eq!(&old_comment, new_writer.get_raw_comment()); *writer = new_writer.into(); } Ok(()) } -fuzz_target!(|data: (Vec, Vec<(FileOperation, bool)>)| { - let (comment, operations) = data; - let mut writer = RefCell::new(zip_next::ZipWriter::new(Cursor::new(Vec::new()))); - writer.borrow_mut().set_raw_comment(comment); - for (operation, abort) in operations { - let _ = do_operation(&mut writer, operation, abort); +fuzz_target!(|test_case: FuzzTestCase| { + let mut writer = RefCell::new(zip_next::ZipWriter::new(Cursor::new(Vec::new()), + test_case.flush_on_finish_file)); + 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 _ = zip_next::ZipArchive::new(writer.borrow_mut().finish().unwrap()); }); \ No newline at end of file diff --git a/src/write.rs b/src/write.rs index d6076065..ccd359d6 100644 --- a/src/write.rs +++ b/src/write.rs @@ -84,7 +84,7 @@ pub(crate) mod zip_writer { /// /// // 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 mut zip = ZipWriter::new(std::io::Cursor::new(&mut buf[..]), false); /// /// let options = FileOptions::default().compression_method(zip_next::CompressionMethod::Stored); /// zip.start_file("hello_world.txt", options)?; @@ -106,6 +106,7 @@ pub(crate) mod zip_writer { pub(super) writing_to_file: bool, pub(super) writing_raw: bool, pub(super) comment: Vec, + pub(super) flush_on_finish_file: bool, } } use crate::result::ZipError::InvalidArchive; @@ -372,7 +373,9 @@ impl ZipWriterStats { impl ZipWriter { /// Initializes the archive from an existing ZIP archive, making it ready for append. - pub fn new_append(mut readwriter: A) -> ZipResult> { + /// + /// See [`ZipWriter::new`] for the caveats that apply when `flush_on_finish_file` is set. + pub fn new_append(mut readwriter: A, flush_on_finish_file: bool) -> ZipResult> { let (footer, cde_start_pos) = spec::CentralDirectoryEnd::find_and_parse(&mut readwriter)?; if footer.disk_number != footer.disk_with_central_directory { @@ -409,6 +412,7 @@ impl ZipWriter { writing_to_file: false, comment: footer.zip_file_comment, writing_raw: true, // avoid recomputing the last file's header + flush_on_finish_file, }) } } @@ -472,7 +476,14 @@ impl ZipWriter { /// Before writing to this object, the [`ZipWriter::start_file`] function should be called. /// After a successful write, the file remains open for writing. After a failed write, call /// [`ZipWriter::is_writing_file`] to determine if the file remains open. - pub fn new(inner: W) -> ZipWriter { + /// + /// `flush_on_finish_file` is designed to support a streaming `inner` that may unload flushed + /// bytes. This ZipWriter will not try to seek further back than the last flushed byte unless + /// either [`ZipWriter::abort_file`] is called while [`ZipWriter::is_writing_file`] returns + /// false, or [`ZipWriter::deep_copy_file`] is called. In the latter case, it will only need to + /// read flushed bytes and not overwrite them. Do not enable this with a [BufWriter], because + /// that implicitly calls [`Writer::flush`] whenever [`Seek::seek`] is called. + pub fn new(inner: W, flush_on_finish_file: bool) -> ZipWriter { ZipWriter { inner: Storer(MaybeEncrypted::Unencrypted(inner)), files: Vec::new(), @@ -481,6 +492,7 @@ impl ZipWriter { writing_to_file: false, writing_raw: false, comment: Vec::new(), + flush_on_finish_file, } } @@ -699,6 +711,12 @@ impl ZipWriter { update_local_file_header(writer, file)?; writer.seek(SeekFrom::Start(file_end))?; } + if self.flush_on_finish_file { + if let Err(e) = writer.flush() { + self.abort_file()?; + return Err(e.into()); + } + } self.writing_to_file = false; Ok(()) @@ -1276,7 +1294,7 @@ fn write_central_directory_header(writer: &mut T, file: &ZipFileData) writer.write_u16::(version_made_by)?; // version needed to extract writer.write_u16::(file.version_needed())?; - // general puprose bit flag + // general purpose bit flag let flag = if !file.file_name.is_ascii() { 1u16 << 11 } else { @@ -1307,7 +1325,7 @@ fn write_central_directory_header(writer: &mut T, file: &ZipFileData) writer.write_u16::(0)?; // disk number start writer.write_u16::(0)?; - // internal file attribytes + // internal file attributes writer.write_u16::(0)?; // external file attributes writer.write_u32::(file.external_attributes)?; @@ -1448,7 +1466,7 @@ mod test { #[test] fn write_empty_zip() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer.set_comment("ZIP"); let result = writer.finish().unwrap(); assert_eq!(result.get_ref().len(), 25); @@ -1467,7 +1485,7 @@ mod test { #[test] fn write_zip_dir() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .add_directory( "test", @@ -1495,7 +1513,7 @@ mod test { #[test] fn write_symlink_simple() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .add_symlink( "name", @@ -1524,7 +1542,7 @@ mod test { #[test] fn write_symlink_wonky_paths() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .add_symlink( "directory\\link", @@ -1556,7 +1574,7 @@ mod test { #[test] fn write_mimetype_zip() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); let options = FileOptions { compression_method: CompressionMethod::Stored, compression_level: None, @@ -1596,7 +1614,7 @@ mod test { #[test] fn test_shallow_copy() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); let options = FileOptions { compression_method: CompressionMethod::Deflated, compression_level: Some(9), @@ -1617,7 +1635,7 @@ mod test { .shallow_copy_file(RT_TEST_FILENAME, SECOND_FILENAME) .expect_err("Duplicate filename"); let zip = writer.finish().unwrap(); - let mut writer = ZipWriter::new_append(zip).unwrap(); + let mut writer = ZipWriter::new_append(zip, false).unwrap(); writer .shallow_copy_file(SECOND_FILENAME, SECOND_FILENAME) .expect_err("Duplicate filename"); @@ -1646,7 +1664,7 @@ mod test { #[test] fn test_deep_copy() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); let options = FileOptions { compression_method: CompressionMethod::Deflated, compression_level: Some(9), @@ -1664,7 +1682,7 @@ mod test { .deep_copy_file(RT_TEST_FILENAME, SECOND_FILENAME) .unwrap(); let zip = writer.finish().unwrap(); - let mut writer = ZipWriter::new_append(zip).unwrap(); + let mut writer = ZipWriter::new_append(zip, false).unwrap(); writer .deep_copy_file(RT_TEST_FILENAME, THIRD_FILENAME) .unwrap(); @@ -1693,7 +1711,7 @@ mod test { #[test] fn duplicate_filenames() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .start_file("foo/bar/test", FileOptions::default()) .unwrap(); @@ -1707,7 +1725,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); 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", @@ -1720,7 +1738,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_2() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); 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", @@ -1734,7 +1752,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_2a() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); 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", @@ -1748,7 +1766,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_3() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .start_file("\0PK\u{6}\u{6}", FileOptions::default()) .unwrap(); @@ -1765,7 +1783,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_4() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .start_file("PK\u{6}\u{6}", FileOptions::default()) .unwrap(); @@ -1788,19 +1806,19 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_5() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .add_directory("", FileOptions::default().with_alignment(21)) .unwrap(); - let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); + let mut writer = ZipWriter::new_append(writer.finish().unwrap(), false).unwrap(); writer.shallow_copy_file("/", "").unwrap(); writer.shallow_copy_file("", "\0").unwrap(); writer.shallow_copy_file("\0", "PK\u{6}\u{6}").unwrap(); - let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); + let mut writer = ZipWriter::new_append(writer.finish().unwrap(), false).unwrap(); writer .start_file("\0\0\0\0\0\0", FileOptions::default()) .unwrap(); - let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); + let mut writer = ZipWriter::new_append(writer.finish().unwrap(), false).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", @@ -1815,7 +1833,7 @@ mod test { #[test] fn remove_shallow_copy_keeps_original() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer .start_file("original", FileOptions::default()) .unwrap(); @@ -1834,14 +1852,14 @@ mod test { #[test] fn remove_encrypted_file() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); let first_file_options = FileOptions::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); + let mut writer = ZipWriter::new(zip, false); writer.start_file("", FileOptions::default()).unwrap(); Ok(()) } @@ -1851,12 +1869,12 @@ mod test { let mut options = FileOptions::default(); options = options.with_deprecated_encryption(b"Password"); options.alignment = 65535; - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); writer.add_symlink("", "s\t\0\0ggggg\0\0", options).unwrap(); writer.abort_file().unwrap(); let zip = writer.finish().unwrap(); println!("{:0>2x?}", zip.get_ref()); - let mut writer = ZipWriter::new_append(zip).unwrap(); + let mut writer = ZipWriter::new_append(zip, false).unwrap(); writer.start_file("", FileOptions::default()).unwrap(); Ok(()) } diff --git a/tests/end_to_end.rs b/tests/end_to_end.rs index 70e59f61..30f48506 100644 --- a/tests/end_to_end.rs +++ b/tests/end_to_end.rs @@ -35,7 +35,7 @@ fn copy() { { let mut src_archive = zip_next::ZipArchive::new(src_file).unwrap(); - let mut zip = ZipWriter::new(&mut tgt_file); + let mut zip = ZipWriter::new(&mut tgt_file, false); { let file = src_archive @@ -73,7 +73,7 @@ fn append() { write_test_archive(file, method, *shallow_copy); { - let mut zip = ZipWriter::new_append(&mut file).unwrap(); + let mut zip = ZipWriter::new_append(&mut file, false).unwrap(); zip.start_file( COPY_ENTRY_NAME, FileOptions::default() @@ -95,7 +95,7 @@ fn append() { // Write a test zip archive to buffer. fn write_test_archive(file: &mut Cursor>, method: CompressionMethod, shallow_copy: bool) { - let mut zip = ZipWriter::new(file); + let mut zip = ZipWriter::new(file, false); zip.add_directory("test/", Default::default()).unwrap(); diff --git a/tests/zip_crypto.rs b/tests/zip_crypto.rs index 45612aac..4b25de8d 100644 --- a/tests/zip_crypto.rs +++ b/tests/zip_crypto.rs @@ -25,7 +25,7 @@ fn encrypting_file() { use std::io::{Read, Write}; use zip_next::unstable::write::FileOptionsExt; let mut buf = vec![0; 2048]; - let mut archive = zip_next::write::ZipWriter::new(Cursor::new(&mut buf)); + let mut archive = zip_next::write::ZipWriter::new(Cursor::new(&mut buf), false); archive .start_file( "name",