Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ time = { workspace = true, features = ["formatting", "macros"] }
anyhow = "1.0.95"
clap = { version = "=4.4.18", features = ["derive"] }
tempfile = "3.15"
rayon = "1.11.0"

[features]
aes-crypto = ["dep:aes", "dep:constant_time_eq", "dep:generic-array", "dep:getrandom", "dep:hmac", "dep:pbkdf2", "dep:sha1", "dep:zeroize"]
Expand Down
59 changes: 57 additions & 2 deletions src/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::compression::{CompressionMethod, Decompressor};
use crate::cp437::FromCp437;
use crate::crc32::Crc32Reader;
use crate::extra_fields::{ExtendedTimestamp, ExtraField, Ntfs};
use crate::read::zip_archive::{Shared, SharedBuilder};
use crate::read::zip_archive::SharedBuilder;
use crate::result::invalid;
use crate::result::{ZipError, ZipResult};
use crate::spec::{self, CentralDirectoryEndInfo, DataAndPosition, FixedSizeBlock, Pod};
Expand All @@ -31,6 +31,7 @@ use std::sync::{Arc, OnceLock};
mod config;

pub use config::*;
pub use zip_archive::Shared;

/// Provides high level API for reading from a stream.
pub(crate) mod stream;
Expand All @@ -44,7 +45,7 @@ pub(crate) mod zip_archive {

/// Extract immutable data from `ZipArchive` to make it cheap to clone
#[derive(Debug)]
pub(crate) struct Shared {
pub struct Shared {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The Shared struct is defined inside the pub(crate) mod zip_archive, which makes it private to the crate. However, it's used in the public functions metadata() and new_with_metadata(). This will cause a compilation error because a private type is exposed in a public interface.

To fix this, you can move the Shared struct definition out of the zip_archive module to make it public within the read module. You'll also need to add use super::Shared; inside zip_archive and re-export Shared from lib.rs.

Example of changes in src/read.rs:

// ... before mod zip_archive
/// Extract immutable data from `ZipArchive` to make it cheap to clone
#[derive(Debug)]
pub struct Shared {
    pub(crate) files: IndexMap<Box<str>, ZipFileData>,
    pub(crate) offset: u64,
    pub(crate) dir_start: u64,
    // This isn't yet used anywhere, but it is here for use cases in the future.
    #[allow(dead_code)]
    pub(crate) config: Config,
    pub(crate) comment: Box<[u8]>,
    pub(crate) zip64_comment: Option<Box<[u8]>>,
}

pub(crate) mod zip_archive {
    use super::Shared;
    // ... (remove Shared struct from here)
}

And in src/lib.rs, you'll need to make Shared public:

pub use crate::read::{Shared, ZipArchive, ZipReadOptions};

Note that I've also changed pub(super) fields to pub(crate) and removed super:: prefixes as they are no longer needed after moving the struct.

This is a critical issue as it prevents the code from compiling.

pub(crate) files: IndexMap<Box<str>, super::ZipFileData>,
pub(super) offset: u64,
pub(super) dir_start: u64,
Expand Down Expand Up @@ -763,6 +764,60 @@ impl<R: Read + Seek> ZipArchive<R> {
Self::with_config(Default::default(), reader)
}

/// Get the metadata associated with the ZIP archive.
///
/// This can be used with [`Self::new_with_metadata`] to create a new reader over the
/// same file without needing to reparse the metadata.
pub fn metadata(&self) -> Arc<Shared> {
self.shared.clone()
}

// UNSAFETY: Requires `unsafe` because this relies on the user to ensure
// `reader` and `metadata` are compatible.
// This is similar to [how `sguaba` uses `unsafe`](https://github.com/helsing-ai/sguaba/blob/6c82af9626d0fe761a75d023be571cebb5d7e5a0/src/lib.rs#L64).
Comment on lines +775 to +777
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment block is redundant with the # Safety section in the documentation for new_with_metadata. The doc comment is what users will see, and it already explains the safety requirements. The link to an external repository is also not ideal to have in the source code. It's better to remove this comment block to keep the code clean.

/// Read a ZIP archive using the given `metadata`.
///
/// This is useful for creating multiple readers over the same file without
/// needing to reparse the metadata.
///
/// # Safety
/// `unsafe` is used here to indicate that `reader` and `metadata` could
/// potentially be incompatible, and it is left to the user to ensure they are.
///
/// # Example
///
/// ```no_run
/// # use std::fs;
/// use rayon::prelude::*;
///
/// const FILE_NAME: &str = "my_data.zip";
///
/// let file = fs::File::open(FILE_NAME).unwrap();
/// let mut archive = zip::ZipArchive::new(file).unwrap();
///
/// let file_names = (0..archive.len())
/// .into_par_iter()
/// .map_init({
/// let metadata = archive.metadata().clone();
/// move || {
/// let file = fs::File::open(FILE_NAME).unwrap();
/// unsafe { zip::ZipArchive::new_with_metadata(file, metadata.clone()) }
/// }},
/// |archive, i| {
/// let mut file = archive.by_index(i).unwrap();
/// file.enclosed_name()
/// }
/// )
/// .filter_map(|name| name)
/// .collect::<Vec<_>>();
/// ```
pub unsafe fn unsafe_new_with_metadata(reader: R, metadata: Arc<Shared>) -> Self {
Self {
reader,
shared: metadata,
}
}

/// Read a ZIP archive providing a read configuration, collecting the files it contains.
///
/// This uses the central directory record of the ZIP file, and ignores local file headers.
Expand Down