From 2407ef95c6db06d119e424f70afd28864f9f9bf8 Mon Sep 17 00:00:00 2001 From: Chris Hennick Date: Tue, 30 May 2023 18:17:59 -0700 Subject: [PATCH] Fixes and refactors for no-features build --- .github/workflows/ci.yaml | 36 +++++- CHANGELOG.md | 14 ++- Cargo.toml | 4 +- README.md | 4 +- 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 | 6 +- src/compression.rs | 49 ++++++++ src/lib.rs | 2 +- src/read.rs | 120 ++++++++++++-------- src/spec.rs | 12 -- src/types.rs | 27 ++--- src/write.rs | 190 +++++++++++++++++++------------- tests/end_to_end.rs | 6 +- tests/zip_crypto.rs | 2 +- 17 files changed, 310 insertions(+), 170 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a676dc03..f837e6b5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -25,18 +25,24 @@ jobs: toolchain: ${{ matrix.rust }} override: true - - name: check + - name: Check uses: actions-rs/cargo@v1 with: command: check args: --all --bins --examples - - name: tests + - name: Tests uses: actions-rs/cargo@v1 with: command: test args: --all + - name: Tests (no features) + uses: actions-rs/cargo@v1 + with: + command: test + args: --all --no-default-features + clippy: runs-on: ubuntu-latest @@ -117,6 +123,32 @@ jobs: - name: run fuzz run: | cargo fuzz run fuzz_write -- -timeout=5s -jobs=100 -workers=2 -runs=10000 -max_len=5000000000 + - name: Upload any failure inputs + if: always() + uses: actions/upload-artifact@v3 + with: + name: fuzz_write_bad_inputs + path: fuzz/artifacts/fuzz_write/crash-* + if-no-files-found: ignore + + fuzz_write_with_no_features: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v3 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + override: true + + - run: cargo install cargo-fuzz + - name: compile fuzz + run: | + cargo fuzz build --no-default-features fuzz_write + - name: run fuzz + run: | + cargo fuzz run fuzz_write -- -timeout=5s -jobs=100 -workers=2 -runs=1000000 -max_len=5000000000 - name: Upload any failure inputs if: always() uses: actions/upload-artifact@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index a0d7010b..1f48ba12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -197,4 +197,16 @@ ### Added - `zlib-ng` for fast Deflate compression. This is now the default for compression levels 0-9. - - `chrono` to convert zip_next::DateTime to and from chrono::NaiveDateTime \ No newline at end of file + - `chrono` to convert zip_next::DateTime to and from chrono::NaiveDateTime + +## [0.10.0] + +### Changed + + - Replaces the `flush_on_finish_file` parameter of `ZipWriter::new` and `ZipWriter::Append` with + a `set_flush_on_finish_file` method. + +### Fixed + + - Fixes build errors that occur when all default features are disabled. + - Fixes more cases of a bug when ZIP64 magic bytes occur in filenames. \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml index 84204c3f..86256105 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "zip_next" -version = "0.9.2" +version = "0.10.0" authors = ["Mathijs van de Nes ", "Marli Frost ", "Ryan Levick ", "Chris Hennick "] license = "MIT" @@ -16,7 +16,7 @@ edition = "2021" aes = { version = "0.8.2", optional = true } byteorder = "1.4.3" bzip2 = { version = "0.4.4", optional = true } -chrono = { version = "0.4.25", optional = true } +chrono = { version = "0.4.26", optional = true } constant_time_eq = { version = "0.2.5", optional = true } crc32fast = "1.3.2" flate2 = { version = "1.0.26", default-features = false, optional = true } diff --git a/README.md b/README.md index 38b93134..12902363 100644 --- a/README.md +++ b/README.md @@ -32,14 +32,14 @@ With all default features: ```toml [dependencies] -zip_next = "0.9.2" +zip_next = "0.10.0" ``` Without the default features: ```toml [dependencies] -zip_next = { version = "0.9.2", default-features = false } +zip_next = { version = "0.10.0", default-features = false } ``` The features available are: diff --git a/benches/read_entry.rs b/benches/read_entry.rs index dd4bced7..4ee20b02 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), false); + let mut writer = ZipWriter::new(Cursor::new(data)); 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 eee2d713..f9be2ec3 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), false); + let mut writer = ZipWriter::new(Cursor::new(data)); 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 a7f14e35..6d92d161 100644 --- a/examples/write_dir.rs +++ b/examples/write_dir.rs @@ -76,7 +76,7 @@ fn zip_dir( where T: Write + Seek, { - let mut zip = zip_next::ZipWriter::new(writer, false); + let mut zip = zip_next::ZipWriter::new(writer); let options = FileOptions::default() .compression_method(method) .unix_permissions(0o755); diff --git a/examples/write_sample.rs b/examples/write_sample.rs index 0834d473..bb9739d0 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, false); + let mut zip = zip_next::ZipWriter::new(file); zip.add_directory("test/", Default::default())?; diff --git a/fuzz/fuzz_targets/fuzz_write.rs b/fuzz/fuzz_targets/fuzz_write.rs index 5aa0ab4b..b09b7ef9 100644 --- a/fuzz/fuzz_targets/fuzz_write.rs +++ b/fuzz/fuzz_targets/fuzz_write.rs @@ -51,6 +51,7 @@ 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, ..} => { @@ -86,7 +87,7 @@ 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(), flush_on_finish_file).unwrap(); + writer.borrow_mut().finish().unwrap()).unwrap(); assert_eq!(&old_comment, new_writer.get_raw_comment()); *writer = new_writer.into(); } @@ -94,8 +95,7 @@ fn do_operation(writer: &mut RefCell>, } fuzz_target!(|test_case: FuzzTestCase| { - let mut writer = RefCell::new(zip_next::ZipWriter::new(Cursor::new(Vec::new()), - test_case.flush_on_finish_file)); + 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); diff --git a/src/compression.rs b/src/compression.rs index e23d743f..9abd34af 100644 --- a/src/compression.rs +++ b/src/compression.rs @@ -152,6 +152,55 @@ impl CompressionMethod { } } +impl Default for CompressionMethod { + fn default() -> Self { + #[cfg(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-zopfli" + ))] + return CompressionMethod::Deflated; + + #[cfg(all( + not(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-zopfli" + )), + feature = "bzip2" + ))] + return CompressionMethod::Bzip2; + + #[cfg(all( + not(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-zopfli", + feature = "bzip2" + )), + feature = "zstd" + ))] + return CompressionMethod::Zstd; + + #[cfg(not(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-zopfli", + feature = "bzip2", + feature = "zstd" + )))] + return CompressionMethod::Stored; + } +} + impl fmt::Display for CompressionMethod { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { // Just duplicate what the Debug format looks like, i.e, the enum key: diff --git a/src/lib.rs b/src/lib.rs index 93b31e35..c18edfe4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,6 +49,6 @@ mod zipcrypto; /// /// ```toml /// [dependencies] -/// zip_next = "=0.9.2" +/// zip_next = "=0.10.0" /// ``` pub mod unstable; diff --git a/src/read.rs b/src/read.rs index f20d4662..5c8d818d 100644 --- a/src/read.rs +++ b/src/read.rs @@ -72,6 +72,7 @@ pub(crate) mod zip_archive { } pub use zip_archive::ZipArchive; + #[allow(clippy::large_enum_variant)] pub(crate) enum CryptoReader<'a> { Plaintext(io::Take<&'a mut dyn Read>), @@ -296,11 +297,19 @@ pub(crate) fn make_reader( } } +pub(crate) struct DirectoryCounts { + pub(crate) archive_offset: u64, + pub(crate) directory_start: u64, + pub(crate) number_of_files: usize, + pub(crate) disk_number: u32, + pub(crate) disk_with_central_directory: u32, +} + impl ZipArchive { fn get_directory_counts_zip32( footer: &spec::CentralDirectoryEnd, cde_start_pos: u64, - ) -> ZipResult<(u64, u64, usize)> { + ) -> ZipResult { // Some zip files have data prepended to them, resulting in the // offsets all being too small. Get the amount of error by comparing // the actual file position we found the CDE at with the offset @@ -314,14 +323,20 @@ impl ZipArchive { let directory_start = footer.central_directory_offset as u64 + archive_offset; let number_of_files = footer.number_of_files_on_this_disk as usize; - Ok((archive_offset, directory_start, number_of_files)) + Ok(DirectoryCounts { + archive_offset, + directory_start, + number_of_files, + disk_number: footer.disk_number as u32, + disk_with_central_directory: footer.disk_with_central_directory as u32, + }) } fn get_directory_counts_zip64( reader: &mut R, footer: &spec::CentralDirectoryEnd, cde_start_pos: u64, - ) -> ZipResult<(u64, u64, usize, ZipResult<()>)> { + ) -> ZipResult { // See if there's a ZIP64 footer. The ZIP64 locator if present will // have its signature 20 bytes in front of the standard footer. The // standard footer, in turn, is 22+N bytes large, where N is the @@ -373,21 +388,13 @@ impl ZipArchive { )); } - let supported = if (footer64.disk_number != footer64.disk_with_central_directory) - || (!footer.record_too_small() - && footer.disk_number as u32 != locator64.disk_with_central_directory) - { - unsupported_zip_error("Support for multi-disk files is not implemented") - } else { - Ok(()) - }; - - Ok(( + Ok(DirectoryCounts { archive_offset, directory_start, - footer64.number_of_files as usize, - supported, - )) + number_of_files: footer64.number_of_files as usize, + disk_number: footer64.disk_number, + disk_with_central_directory: footer64.disk_with_central_directory, + }) } /// Get the directory start offset and number of files. This is done in a @@ -396,29 +403,48 @@ impl ZipArchive { reader: &mut R, footer: &spec::CentralDirectoryEnd, cde_start_pos: u64, - ) -> ZipResult<(u64, u64, usize)> { + ) -> ZipResult { // Check if file has a zip64 footer - let (archive_offset_64, directory_start_64, number_of_files_64, supported_64) = - match Self::get_directory_counts_zip64(reader, footer, cde_start_pos) { - Ok(result) => result, - Err(_) => return Self::get_directory_counts_zip32(footer, cde_start_pos), - }; - // Check if it also has a zip32 footer - let (archive_offset_32, directory_start_32, number_of_files_32) = - match Self::get_directory_counts_zip32(footer, cde_start_pos) { - Ok(result) => result, - Err(_) => { - supported_64?; - return Ok((archive_offset_64, directory_start_64, number_of_files_64)); + let counts_64 = Self::get_directory_counts_zip64(reader, footer, cde_start_pos); + let counts_32 = Self::get_directory_counts_zip32(footer, cde_start_pos); + match counts_64 { + Err(_) => match counts_32 { + Err(e) => Err(e), + Ok(counts) => { + if counts.disk_number != counts.disk_with_central_directory { + return unsupported_zip_error( + "Support for multi-disk files is not implemented", + ); + } + Ok(counts) } - }; - // It has both, so check if the zip64 footer is valid; if not, assume zip32 - if number_of_files_64 != number_of_files_32 && number_of_files_32 != u16::MAX as usize { - return Ok((archive_offset_32, directory_start_32, number_of_files_32)); + }, + Ok(counts_64) => { + match counts_32 { + Err(_) => Ok(counts_64), + Ok(counts_32) => { + // Both zip32 and zip64 footers exist, so check if the zip64 footer is valid; if not, try zip32 + if counts_64.number_of_files != counts_32.number_of_files + && counts_32.number_of_files != u16::MAX as usize + { + return Ok(counts_32); + } + if counts_64.disk_number != counts_32.disk_number + && counts_32.disk_number != u16::MAX as u32 + { + return Ok(counts_32); + } + if counts_64.disk_with_central_directory + != counts_32.disk_with_central_directory + && counts_32.disk_with_central_directory != u16::MAX as u32 + { + return Ok(counts_32); + } + Ok(counts_64) + } + } + } } - // It is, so we assume a zip64 - supported_64?; - Ok((archive_offset_64, directory_start_64, number_of_files_64)) } /// Read a ZIP archive, collecting the files it contains @@ -427,32 +453,34 @@ impl ZipArchive { pub fn new(mut reader: R) -> ZipResult> { let (footer, cde_start_pos) = spec::CentralDirectoryEnd::find_and_parse(&mut reader)?; - if !footer.record_too_small() && footer.disk_number != footer.disk_with_central_directory { + let counts = Self::get_directory_counts(&mut reader, &footer, cde_start_pos)?; + + if counts.disk_number != counts.disk_with_central_directory { return unsupported_zip_error("Support for multi-disk files is not implemented"); } - let (archive_offset, directory_start, number_of_files) = - Self::get_directory_counts(&mut reader, &footer, cde_start_pos)?; - // If the parsed number of files is greater than the offset then // something fishy is going on and we shouldn't trust number_of_files. - let file_capacity = if number_of_files > cde_start_pos as usize { + let file_capacity = if counts.number_of_files > cde_start_pos as usize { 0 } else { - number_of_files + counts.number_of_files }; let mut files = Vec::with_capacity(file_capacity); let mut names_map = HashMap::with_capacity(file_capacity); - if reader.seek(io::SeekFrom::Start(directory_start)).is_err() { + if reader + .seek(io::SeekFrom::Start(counts.directory_start)) + .is_err() + { return Err(ZipError::InvalidArchive( "Could not seek to start of central directory", )); } - for _ in 0..number_of_files { - let file = central_header_to_zip_file(&mut reader, archive_offset)?; + for _ in 0..counts.number_of_files { + let file = central_header_to_zip_file(&mut reader, counts.archive_offset)?; names_map.insert(file.file_name.clone(), files.len()); files.push(file); } @@ -460,7 +488,7 @@ impl ZipArchive { let shared = Arc::new(zip_archive::Shared { files, names_map, - offset: archive_offset, + offset: counts.archive_offset, comment: footer.zip_file_comment, }); diff --git a/src/spec.rs b/src/spec.rs index 1ecd2cce..691f2cab 100644 --- a/src/spec.rs +++ b/src/spec.rs @@ -23,18 +23,6 @@ pub struct CentralDirectoryEnd { } impl CentralDirectoryEnd { - // Per spec 4.4.1.4 - a CentralDirectoryEnd field might be insufficient to hold the - // required data. In this case the file SHOULD contain a ZIP64 format record - // and the field of this record will be set to -1 - pub(crate) fn record_too_small(&self) -> bool { - self.disk_number == 0xFFFF - || self.disk_with_central_directory == 0xFFFF - || self.number_of_files_on_this_disk == 0xFFFF - || self.number_of_files == 0xFFFF - || self.central_directory_size == 0xFFFFFFFF - || self.central_directory_offset == 0xFFFFFFFF - } - pub fn parse(reader: &mut T) -> ZipResult { let magic = reader.read_u32::()?; if magic != CENTRAL_DIRECTORY_END_SIGNATURE { diff --git a/src/types.rs b/src/types.rs index 5ee22af3..8af87d0b 100644 --- a/src/types.rs +++ b/src/types.rs @@ -11,8 +11,6 @@ use chrono::{Datelike, NaiveDate, NaiveDateTime, NaiveTime, Timelike}; target_arch = "powerpc" )))] use std::sync::atomic; -#[cfg(not(feature = "time"))] -use std::time::SystemTime; #[cfg(doc)] use {crate::read::ZipFile, crate::write::FileOptions}; @@ -53,8 +51,6 @@ mod atomic { } } -#[cfg(feature = "time")] -use crate::result::DateTimeRangeError; #[cfg(feature = "time")] use time::{error::ComponentRange, Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; @@ -121,16 +117,16 @@ impl arbitrary::Arbitrary<'_> for DateTime { #[cfg(feature = "chrono")] #[allow(clippy::result_unit_err)] impl TryFrom for DateTime { - type Error = DateTimeRangeError; + type Error = (); fn try_from(value: NaiveDateTime) -> Result { DateTime::from_date_and_time( - value.year().try_into()?, - value.month().try_into()?, - value.day().try_into()?, - value.hour().try_into()?, - value.minute().try_into()?, - value.second().try_into()?, + value.year().try_into().map_err(|_| ())?, + value.month().try_into().map_err(|_| ())?, + value.day().try_into().map_err(|_| ())?, + value.hour().try_into().map_err(|_| ())?, + value.minute().try_into().map_err(|_| ())?, + value.second().try_into().map_err(|_| ())?, ) } } @@ -201,7 +197,7 @@ impl DateTime { hour: u8, minute: u8, second: u8, - ) -> Result { + ) -> Result { if (1980..=2107).contains(&year) && (1..=12).contains(&month) && (1..=31).contains(&day) @@ -218,7 +214,7 @@ impl DateTime { second, }) } else { - Err(DateTimeRangeError) + Err(()) } } @@ -316,8 +312,9 @@ impl DateTime { } #[cfg(feature = "time")] +#[allow(clippy::result_unit_err)] impl TryFrom for DateTime { - type Error = DateTimeRangeError; + type Error = (); fn try_from(dt: OffsetDateTime) -> Result { if dt.year() >= 1980 && dt.year() <= 2107 { @@ -330,7 +327,7 @@ impl TryFrom for DateTime { second: dt.second(), }) } else { - Err(DateTimeRangeError) + Err(()) } } } diff --git a/src/write.rs b/src/write.rs index 5ed7c78a..b882a326 100644 --- a/src/write.rs +++ b/src/write.rs @@ -8,12 +8,31 @@ use crate::types::{ffi, AtomicU64, DateTime, System, ZipFileData, DEFAULT_VERSIO use byteorder::{LittleEndian, WriteBytesExt}; use crc32fast::Hasher; use std::collections::HashMap; +#[cfg(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-zopfli", + feature = "bzip2", + feature = "zstd", + feature = "time" +))] use std::convert::TryInto; use std::default::Default; use std::io; use std::io::prelude::*; -use std::io::{BufReader, BufWriter, SeekFrom}; +use std::io::{BufReader, SeekFrom}; use std::mem; +#[cfg(any( + feature = "deflate", + feature = "deflate-miniz", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "zopfli", + feature = "bzip2", + feature = "zstd", +))] use std::num::NonZeroU8; use std::str::{from_utf8, Utf8Error}; use std::sync::Arc; @@ -24,16 +43,20 @@ use std::sync::Arc; feature = "deflate-zlib", feature = "deflate-zlib-ng" ))] -use flate2::write::DeflateEncoder; +use flate2::{write::DeflateEncoder, Compression}; #[cfg(feature = "bzip2")] use bzip2::write::BzEncoder; -use flate2::Compression; #[cfg(feature = "time")] use time::OffsetDateTime; + +#[cfg(feature = "deflate-zopfli")] use zopfli::Options; +#[cfg(feature = "deflate-zopfli")] +use std::io::BufWriter; + #[cfg(feature = "zstd")] use zstd::stream::write::Encoder as ZstdEncoder; @@ -93,7 +116,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[..]), false); + /// let mut zip = ZipWriter::new(std::io::Cursor::new(&mut buf[..])); /// /// let options = FileOptions::default().compression_method(zip_next::CompressionMethod::Stored); /// zip.start_file("hello_world.txt", options)?; @@ -121,7 +144,7 @@ pub(crate) mod zip_writer { use crate::result::ZipError::InvalidArchive; use crate::write::GenericZipWriter::{Closed, Storer}; use crate::zipcrypto::ZipCryptoKeys; -use crate::CompressionMethod::{Deflated, Stored}; +use crate::CompressionMethod::Stored; pub use zip_writer::ZipWriter; #[derive(Default)] @@ -170,11 +193,11 @@ impl arbitrary::Arbitrary<'_> for FileOptions { zopfli_buffer_size: None, }; match options.compression_method { - Deflated => { + #[cfg(feature = "deflate-zopfli")] + CompressionMethod::Deflated => { if bool::arbitrary(u)? { let level = u.int_in_range(0..=24)?; options.compression_level = Some(level); - #[cfg(feature = "deflate-zopfli")] if level > Compression::best().level().try_into().unwrap() { options.zopfli_buffer_size = Some(1 << u.int_in_range(9..=30)?); } @@ -347,22 +370,7 @@ impl Default for FileOptions { /// Construct a new FileOptions object fn default() -> Self { Self { - #[cfg(any( - feature = "deflate", - feature = "deflate-miniz", - feature = "deflate-zlib", - feature = "deflate-zlib-ng", - feature = "deflate-zopfli" - ))] - compression_method: Deflated, - #[cfg(not(any( - feature = "deflate", - feature = "deflate-miniz", - feature = "deflate-zlib", - feature = "deflate-zlib-ng", - feature = "deflate-zopfli" - )))] - compression_method: Stored, + compression_method: Default::default(), compression_level: None, #[cfg(feature = "time")] last_modified_time: OffsetDateTime::now_utc().try_into().unwrap_or_default(), @@ -435,28 +443,28 @@ impl ZipWriterStats { impl ZipWriter { /// Initializes the archive from an existing ZIP archive, making it ready for append. - /// - /// 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> { + pub fn new_append(mut readwriter: A) -> ZipResult> { let (footer, cde_start_pos) = spec::CentralDirectoryEnd::find_and_parse(&mut readwriter)?; - if footer.disk_number != footer.disk_with_central_directory { + let counts = ZipArchive::get_directory_counts(&mut readwriter, &footer, cde_start_pos)?; + + if counts.disk_number != counts.disk_with_central_directory { return Err(ZipError::UnsupportedArchive( "Support for multi-disk files is not implemented", )); } - let (archive_offset, directory_start, number_of_files) = - ZipArchive::get_directory_counts(&mut readwriter, &footer, cde_start_pos)?; - - if readwriter.seek(SeekFrom::Start(directory_start)).is_err() { + if readwriter + .seek(SeekFrom::Start(counts.directory_start)) + .is_err() + { return Err(InvalidArchive( "Could not seek to start of central directory", )); } - let files = (0..number_of_files) - .map(|_| central_header_to_zip_file(&mut readwriter, archive_offset)) + let files = (0..counts.number_of_files) + .map(|_| central_header_to_zip_file(&mut readwriter, counts.archive_offset)) .collect::, _>>()?; let mut files_by_name = HashMap::new(); @@ -464,7 +472,7 @@ impl ZipWriter { files_by_name.insert(file.file_name.to_owned(), index); } - let _ = readwriter.seek(SeekFrom::Start(directory_start)); // seek directory_start to overwrite it + let _ = readwriter.seek(SeekFrom::Start(counts.directory_start)); // seek directory_start to overwrite it Ok(ZipWriter { inner: Storer(MaybeEncrypted::Unencrypted(readwriter)), @@ -474,9 +482,26 @@ 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, + flush_on_finish_file: false, }) } + + /// `flush_on_finish_file` is designed to support a streaming `inner` that may unload flushed + /// bytes. It flushes a file's header and body once it starts writing another file. A ZipWriter + /// will not try to seek back into where a previous file was written 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 previously-written files and not overwrite them. + /// + /// Note: when using an `inner` that cannot overwrite flushed bytes, do not wrap it in a + /// [std::io::BufWriter], because that has a [seek] method that implicitly calls [flush], and + /// ZipWriter needs to seek backward to update each file's header with the size and checksum + /// after writing the body. + /// + /// This setting is false by default. + pub fn set_flush_on_finish_file(&mut self, flush_on_finish_file: bool) { + self.flush_on_finish_file = flush_on_finish_file; + } } impl ZipWriter { @@ -540,15 +565,7 @@ 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. - /// - /// `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. Likewise, when - /// using Deflate compression, set [] - pub fn new(inner: W, flush_on_finish_file: bool) -> ZipWriter { + pub fn new(inner: W) -> ZipWriter { ZipWriter { inner: Storer(MaybeEncrypted::Unencrypted(inner)), files: Vec::new(), @@ -557,7 +574,7 @@ impl ZipWriter { writing_to_file: false, writing_raw: false, comment: Vec::new(), - flush_on_finish_file, + flush_on_finish_file: false, } } @@ -754,7 +771,12 @@ impl ZipWriter { return Ok(()); } - let make_plain_writer = self.inner.prepare_next_writer(Stored, None, None)?; + let make_plain_writer = self.inner.prepare_next_writer( + Stored, + None, + #[cfg(feature = "deflate-zopfli")] + None, + )?; self.inner.switch_to(make_plain_writer)?; self.switch_to_non_encrypting_writer()?; let writer = self.inner.get_plain(); @@ -804,7 +826,12 @@ impl ZipWriter { pub fn abort_file(&mut self) -> ZipResult<()> { let last_file = self.files.pop().ok_or(ZipError::FileNotFound)?; self.files_by_name.remove(&last_file.file_name); - let make_plain_writer = self.inner.prepare_next_writer(Stored, None, None)?; + let make_plain_writer = self.inner.prepare_next_writer( + Stored, + None, + #[cfg(feature = "deflate-zopfli")] + None, + )?; self.inner.switch_to(make_plain_writer)?; self.switch_to_non_encrypting_writer()?; // Make sure this is the last file, and that no shallow copies of it remain; otherwise we'd @@ -1180,7 +1207,7 @@ impl GenericZipWriter { feature = "deflate-zlib-ng", feature = "deflate-zopfli" ))] - Deflated => { + CompressionMethod::Deflated => { let default = if cfg!(feature = "deflate") || cfg!(feature = "deflate-miniz") || cfg!(feature = "deflate-zlib") @@ -1605,6 +1632,13 @@ mod test { use crate::compression::CompressionMethod; use crate::result::ZipResult; use crate::types::DateTime; + #[cfg(any( + feature = "deflate", + feature = "deflate-zlib", + feature = "deflate-zlib-ng", + feature = "deflate-miniz", + feature = "deflate-zopfli" + ))] use crate::CompressionMethod::Deflated; use crate::ZipArchive; use std::io; @@ -1613,7 +1647,7 @@ mod test { #[test] fn write_empty_zip() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer.set_comment("ZIP"); let result = writer.finish().unwrap(); assert_eq!(result.get_ref().len(), 25); @@ -1632,7 +1666,7 @@ mod test { #[test] fn write_zip_dir() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .add_directory( "test", @@ -1660,7 +1694,7 @@ mod test { #[test] fn write_symlink_simple() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .add_symlink( "name", @@ -1689,7 +1723,7 @@ mod test { #[test] fn write_symlink_wonky_paths() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .add_symlink( "directory\\link", @@ -1721,7 +1755,7 @@ mod test { #[test] fn write_mimetype_zip() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); let options = FileOptions { compression_method: CompressionMethod::Stored, compression_level: None, @@ -1763,10 +1797,10 @@ mod test { #[test] fn test_shallow_copy() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); let options = FileOptions { - compression_method: Deflated, - compression_level: Some(9), + compression_method: CompressionMethod::default(), + compression_level: None, last_modified_time: DateTime::default(), permissions: Some(33188), large_file: false, @@ -1786,7 +1820,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, false).unwrap(); + let mut writer = ZipWriter::new_append(zip).unwrap(); writer .shallow_copy_file(SECOND_FILENAME, SECOND_FILENAME) .expect_err("Duplicate filename"); @@ -1815,10 +1849,10 @@ mod test { #[test] fn test_deep_copy() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); let options = FileOptions { - compression_method: Deflated, - compression_level: Some(9), + compression_method: CompressionMethod::default(), + compression_level: None, last_modified_time: DateTime::default(), permissions: Some(33188), large_file: false, @@ -1835,7 +1869,7 @@ mod test { .deep_copy_file(RT_TEST_FILENAME, SECOND_FILENAME) .unwrap(); let zip = writer.finish().unwrap(); - let mut writer = ZipWriter::new_append(zip, false).unwrap(); + let mut writer = ZipWriter::new_append(zip).unwrap(); writer .deep_copy_file(RT_TEST_FILENAME, THIRD_FILENAME) .unwrap(); @@ -1864,7 +1898,7 @@ mod test { #[test] fn duplicate_filenames() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .start_file("foo/bar/test", FileOptions::default()) .unwrap(); @@ -1878,7 +1912,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); 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", @@ -1891,7 +1925,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_2() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); 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", @@ -1905,7 +1939,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_2a() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); 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", @@ -1919,7 +1953,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_3() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .start_file("\0PK\u{6}\u{6}", FileOptions::default()) .unwrap(); @@ -1936,7 +1970,7 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_4() { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .start_file("PK\u{6}\u{6}", FileOptions::default()) .unwrap(); @@ -1959,19 +1993,19 @@ mod test { #[test] fn test_filename_looks_like_zip64_locator_5() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .add_directory("", FileOptions::default().with_alignment(21)) .unwrap(); - let mut writer = ZipWriter::new_append(writer.finish().unwrap(), false).unwrap(); + let mut writer = ZipWriter::new_append(writer.finish().unwrap()).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(), false).unwrap(); + let mut writer = ZipWriter::new_append(writer.finish().unwrap()).unwrap(); writer .start_file("\0\0\0\0\0\0", FileOptions::default()) .unwrap(); - let mut writer = ZipWriter::new_append(writer.finish().unwrap(), false).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", @@ -1986,7 +2020,7 @@ mod test { #[test] fn remove_shallow_copy_keeps_original() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer .start_file("original", FileOptions::default()) .unwrap(); @@ -2005,14 +2039,14 @@ mod test { #[test] fn remove_encrypted_file() -> ZipResult<()> { - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); 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, false); + let mut writer = ZipWriter::new(zip); writer.start_file("", FileOptions::default()).unwrap(); Ok(()) } @@ -2022,12 +2056,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()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); 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, false).unwrap(); + let mut writer = ZipWriter::new_append(zip).unwrap(); writer.start_file("", FileOptions::default()).unwrap(); Ok(()) } @@ -2037,9 +2071,9 @@ mod test { fn zopfli_empty_write() -> ZipResult<()> { let mut options = FileOptions::default(); options = options - .compression_method(Deflated) + .compression_method(CompressionMethod::default()) .compression_level(Some(264)); - let mut writer = ZipWriter::new(io::Cursor::new(Vec::new()), false); + let mut writer = ZipWriter::new(io::Cursor::new(Vec::new())); writer.start_file("", options).unwrap(); writer.write_all(&[]).unwrap(); writer.write_all(&[]).unwrap(); diff --git a/tests/end_to_end.rs b/tests/end_to_end.rs index 30f48506..70e59f61 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, false); + let mut zip = ZipWriter::new(&mut tgt_file); { 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, false).unwrap(); + let mut zip = ZipWriter::new_append(&mut file).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, false); + let mut zip = ZipWriter::new(file); zip.add_directory("test/", Default::default()).unwrap(); diff --git a/tests/zip_crypto.rs b/tests/zip_crypto.rs index 4b25de8d..45612aac 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), false); + let mut archive = zip_next::write::ZipWriter::new(Cursor::new(&mut buf)); archive .start_file( "name",